Regression wrt mounting /proc in user namespace in 3.13
Serge E. Hallyn
serge at hallyn.com
Tue Nov 19 03:47:33 UTC 2013
Quoting Eric W. Biederman (ebiederm at xmission.com):
> "Serge E. Hallyn" <serge at hallyn.com> writes:
>
> > Quoting Serge E. Hallyn (serge at hallyn.com):
> >> Quoting Gao feng (gaofeng at cn.fujitsu.com):
> >> > On 11/18/2013 11:19 AM, Serge E. Hallyn wrote:
> >> > > Quoting Serge E. Hallyn (serge at hallyn.com):
> >> > >> Low on power and no charger, but a quick test printing out if a mount is
> >> > >> !S_ISDIR or has nlink !=2 in fs_fully_visible() gives me:
> >> > >>
> >> > >> [ 92.939650] nlink is 1 for ino 8733 (0:3)
> >> > >>
> >> > >> (that's major 0 minor 3)
> >> > >
> >> > > Ok, so that is for binfmt_misc on /proc/sys/fs/binfmt_misc. The
> >> > > underlying directory is empty, and nlink is showing up as 1.
> >> > >
> >> > > Can we just get the nlink check changed to check for < 3 instead
> >> > > of ==2 ?
> >> > >
> >> >
> >> > I already reported this problem to Eric,hi is working on fix this problem.
> >> >
> >> > nlink is not the right thing to check if a directory is null. since
> >> > in all of filesystems, parent dir's nlink is increase only when we
> >> > create sub-dir.
> >>
> >> This whole thing feels very brittle. May I also point out that simply
> >> setting perms appears to work just fine instead of overmounting. If I
> >> chmod 700 /proc/swaps, unshare my pid and mount namespaces and remount
> >> /proc, then /proc/swaps is 700 in the new mount. Since our concern is
> >> with a new user namespace, which will be limited to world perms, this
> >> should suffice and allow us to skip all this nonsense.
> >>
> >> Eric?
> >>
> >> -serge
> >
> > So yeah, I think this patch should be reverted, rather than "fixed".
>
> I will get back to this shortly. The trivial fix is to kill the nlink
> test or make the nlink test look for nlink <= 2.
>
> The point of the original patch was to remove the horrible special case
> that implemented the test to see if proc was moutned, so that we could
> trivial prevent mount of proc in chroots or other legacy environments
> where root did not want proc to be mounted.
>
> The refinement I attempted and failed was to verify that people had only
> mounted on empty proc directories. Just to verify that nothing becomes
> possible when mounting a new copy of proc that wasn't possible before.
>
> My test is totally for that last case is totally braindead. Permissions
> are nice but the actual concern is setups that did silly things that are
> not namespace aware.
They don't have to be namespace aware, and can keep doing what they're
doing - long as they also chmod o-rwx the mountpoint.
> I am totally against reverting the cleanup. But I have no problem
> killing the nlink test. Because we did not have protection against
> overmounted directories earlier.
Uh, I *think* what you're suggesting sounds good :)
thanks,
-serge
More information about the Containers
mailing list