containers
Fix O(N^2) behavior of bind mounting.
Read /proc/self/mountinfo only once instead reading it for every "--bind" flag on the command line.
This pull request is a work in progress. The intent is to fix https://github.com/containers/bubblewrap/issues/384 I'll need some help from the bubblewrap maintainers to land this change.
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
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.
Thanks very much for taking a look.
Is it possible to simulate the effects of a mount sufficiently so that the mountinfo file is not read for every bind mount requested? Maybe it's intractable or so risky that the approach I took can't be salvaged.
I don't understand the recursive mount handling and the issues with mount propagation. Are there any tests in the bubblewrap codebase for what's required ... or maybe you could describe in more detail why the current code does what it does for recursive mounts.
As I said in one of my comments, I'll need assistance from the maintainers in order to land a change like this. I'm definitely not an expert on this stuff ...
Thanks again!
On Tue, Aug 25, 2020 at 9:18 AM Alexander Larsson [email protected] wrote:
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.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/containers/bubblewrap/pull/385#issuecomment-680019272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGVMNM5IR4JB66YVGZ6NDSCO2Y7ANCNFSM4P75MVNQ .
Having alternative for constantly reading /proc/self/mountinfo was recently requested on lkml which also means such alternative may not exist right for now.
