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

fallback mechanism for /proc/1/mountinfo to /proc/self/mountinfo can produce noisy logs #1271

Closed
1 of 5 tasks
jdstrand opened this issue Mar 9, 2022 · 8 comments
Closed
1 of 5 tasks

Comments

@jdstrand
Copy link
Contributor

jdstrand commented Mar 9, 2022

Describe the bug
The changes to gopsutil for reading /proc/1/mountinfo affected applications running under restricted environments that disallow access to /proc/1/mountinfo and the recent fix for #1159 (filed for android but other restricted environments are also affected (eg, snaps)) fixes application behavior to work under confinement (thanks!). 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.

I suggest introducing an environment variable to force the old behavior. Eg, use the current behavior on master but introduce a new SELF_MOUNTINFO environment variable to use /proc/self/mountinfo without first trying /proc/1/mountinfo. This allows people to set SELF_MOUNTINFO=1 when gopsutil is running under these restricted environments.

I plan to submit a PR for this.

Expected behavior
gopsutil continues with current fallback behavior by default but allows reading only /proc/self/mountinfo so that gopsutil can function correctly without introducing noisy log entries.

Environment (please complete the following information):

  • Windows: [paste the result of ver]
  • Linux: [paste contents of /etc/os-release and the result of uname -a]
$ cat /etc/os-release 
NAME="Ubuntu"
VERSION="20.04.4 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.4 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

$ uname -a
Linux parvati 5.4.0-100-generic #113-Ubuntu SMP Thu Feb 3 18:43:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
  • Mac OS: [paste the result of sw_vers and uname -a
  • FreeBSD: [paste the result of freebsd-version -k -r -u and uname -a]
  • OpenBSD: [paste the result of uname -a]

Additional context
[Cross-compiling? Paste the command you are using to cross-compile and the result of the corresponding go env]

@Lomanic
Copy link
Collaborator

Lomanic commented Mar 10, 2022

I would suggest a HOST_PROC_MOUNTINFO env variable (settable to the path to /proc/X/mountinfo) instead, like the others already available in https://github.com/shirou/gopsutil#usage

@shirou
Copy link
Owner

shirou commented Mar 11, 2022

Could you tell us that log message and who emits? If it is not avoidable, I agree to @Lomanic regarding variable name, HOST_PROC_MOUNTINFO.

@jdstrand
Copy link
Contributor Author

jdstrand commented Mar 11, 2022

Could you tell us that log message and who emits?

In my case, I'm using this with telegraf running as a snap (experimental), and two log entries occur every 10 seconds (due to the default telegraf configuration for disks), but this could happen with other AppArmor policy:

Mar 09 16:10:16 host kernel: audit: type=1400 audit(1646863816.865:1064): apparmor="DENIED" operation="open" profile="snap.telegraf-exp.telegraf" name="/proc/1/mountinfo" pid=410166 comm="main" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
Mar 09 16:10:16 host kernel: audit: type=1400 audit(1646863816.865:1065): apparmor="DENIED" operation="open" profile="snap.telegraf-exp.telegraf" name="/proc/1/mounts" pid=410166 comm="main" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0

@jdstrand
Copy link
Contributor Author

jdstrand commented Mar 11, 2022

If it is not avoidable, I agree to @Lomanic regarding variable name, HOST_PROC_MOUNTINFO.

@shirou - I could do this easily, sure. I did think about before submitting it and chose not to because all of those seem designed for "Hey, I know I'm running in a different environment so I'd like to use this other location instead". This issue is different because:

  • HOST_PROC already covers the alternate location
  • there are only two valid values that HOST_PROC_MOUNTINFO could be set to: /proc/1/mountinfo (the default) and /proc/self/mountinfo so allowing setting to other values seemed error-prone

Again, I'm happy to adjust if you feel HOST_PROC_MOUNTINFO is still the right approach.

@jdstrand
Copy link
Contributor Author

If it is not avoidable, I agree to @Lomanic regarding variable name, HOST_PROC_MOUNTINFO.

@shirou - I could do this easily, sure. I did think about before submitting it and chose not to because all of those seem designed for "Hey, I know I'm running in a different environment so I'd like to use this other location instead". This issue is different because:

* HOST_PROC already covers the alternate location

* there are only two valid values that HOST_PROC_MOUNTINFO could be set to: `/proc/1/mountinfo` (the default) and `/proc/self/mountinfo` so allowing setting to other values seemed error-prone

Again, I'm happy to adjust if you feel HOST_PROC_MOUNTINFO is still the right approach.

@shirou - ping

@shirou
Copy link
Owner

shirou commented Mar 22, 2022

Sorry to late response. I think to align with other environment variables, HOST_PROC_MOUNTINFO is better even /proc/self/mountinfo is only valid value.

@Lomanic
Copy link
Collaborator

Lomanic commented Mar 22, 2022

A binary toggle really ties our hands and leaks the lib internals (what if we had to invert the logic, again), HOST_PROC_MOUNTINFO limits the moving parts in my opinion.

@jdstrand
Copy link
Contributor Author

@shirou and @Lomanic - thanks for the feedback. #1272 is adjusted to use HOST_PROC_MOUNTINFO instead.

@shirou shirou closed this as completed in 9e6e627 Apr 1, 2022
jdstrand pushed a commit to jdstrand/telegraf-snap that referenced this issue 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 issue Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants