Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[disk][linux] add HOST_PROC_MOUNTINFO #1272

Merged
merged 1 commit into from Apr 1, 2022

Conversation

jdstrand
Copy link
Contributor

@jdstrand jdstrand commented Mar 9, 2022

The changes to gopsutil for reading /proc/1/mountinfo affected applications running under restricted environments that disallows access to /proc/1/mountinfo. #1159 was filed for android but other restricted environments are also affected (eg, snaps)). The fix for #1159 addressed the application behavior to work under confinement for non-android as well. However, depending on the system, the attempt to read /proc/1/mountinfo could cause a sandbox denial in the logs which can be quite noisy if using gopsutil as part of a monitoring solution that polls often.

This introduces HOST_PROC_MOUNTINFO to force reading from the parent dir of the specified path instead of first trying /proc/1. When unset, retain the current behavior with fallback. This allows people, for example, to set HOST_PROC_MOUNTINFO=/proc/self/mountinfo when gopsutil is running under these restricted environments.

This change updates the private readMountFile() to use a root path instead of a root subpath, and adjusts PartitionsWithContext() to set the root path to /proc/1 initially and falling back to /proc/self. When HOST_PROC_MOUNTINFO is not empty, set the root path to the parent directory of HOST_PROC_MOUNTINFO.

Tested on linux with HOST_PROC_MOUNTINFO unset, set to /proc/self/mountinfo as well as other values and it behaves as expected (eg, when unset, has the current behavior with fallback, else tries the specified path without fallback). Also make build_test passed all tests.

NOTE: originally this used SELF_MOUNTINFO. Thanks to @shirou and @Lomanic for the feedback.

The changes to gopsutil for reading /proc/1/mountinfo affected
applications running under restricted environments that disallows access
to /proc/1/mountinfo. shirou#1159 was filed for android but other restricted
environments are also affected (eg, snaps)). The fix for shirou#1159 addressed
the application behavior to work under confinement for non-android as
well. However, depending on the system, the attempt to read
/proc/1/mountinfo could cause a sandbox denial in the logs which can be
quite noisy if using gopsutil as part of a monitoring solution that
polls often.

This introduces HOST_PROC_MOUNTINFO to force reading from the parent dir
of the specified path instead of first trying /proc/1. When unset,
retain the current behavior with fallback. This allows people, for
example, to set HOST_PROC_MOUNTINFO=/proc/self/mountinfo when gopsutil
is running under these restricted environments.

This change updates the private readMountFile() to use a root path
instead of a root subpath, and adjusts PartitionsWithContext() to set
the root path to /proc/1 initially and falling back to /proc/self. When
HOST_PROC_MOUNTINFO is not empty, set the root path to the parent
directory of HOST_PROC_MOUNTINFO.
@jdstrand jdstrand force-pushed the jdstrand/add-env-force-workaround branch from 514d2a8 to 9e6e627 Compare March 29, 2022 13:20
@jdstrand jdstrand changed the title [disk][linux] add SELF_MOUNTINFO to use /proc/self [disk][linux] add HOST_PROC_MOUNTINFO Mar 29, 2022
Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems great. Thank you so much!

@shirou shirou merged commit cf63093 into shirou:master Apr 1, 2022
jdstrand pushed a commit to jdstrand/telegraf-snap that referenced this pull request Jun 15, 2022
Will want to use this when the new telegraf pulls in the new gopsutil.

shirou/gopsutil#1271
shirou/gopsutil#1272
jdstrand pushed a commit to jdstrand/telegraf-snap that referenced this pull request Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants