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

Fix O(N^2) behavior of bind mounting. #385

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brown
Copy link

@brown brown commented Aug 14, 2020

Read /proc/self/mountinfo only once instead reading it for every "--bind" flag
on the command line.

@brown
Copy link
Author

brown commented Aug 14, 2020

This pull request is a work in progress. The intent is to fix #384
I'll need some help from the bubblewrap maintainers to land this change.

@rh-atomic-bot
Copy link

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@brown brown force-pushed the fix-mount-speed branch 3 times, most recently from fac3f78 to 9e10887 Compare August 18, 2020 13:31
Read /proc/self/mountinfo only once instead reading it for every "--bind" flag
on the command line.
@alexlarsson
Copy link
Collaborator

I didn't really review this, but clearly its not doing recursive bind mounts correctly as it only adds one mount info for the mount.
Also, it doesn't do anything about flags on remount.

In the setuid case, any time we get a wrong value for current_flags for some reason we risk changing the mount flags of an existing mount and accidentally dropping a sysadmin-set flag (such as nosetuid, or read-only) which is a security failure.

@brown
Copy link
Author

brown commented Aug 25, 2020 via email

@Maryse47
Copy link

Having alternative for constantly reading /proc/self/mountinfo was recently requested on lkml which also means such alternative may not exist right for now.

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

Successfully merging this pull request may close these issues.

4 participants