Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx): > "Serge E. Hallyn" <serge@xxxxxxxxxx> writes: > > > Quoting Serge E. Hallyn (serge@xxxxxxxxxx): > >> Quoting Gao feng (gaofeng@xxxxxxxxxxxxxx): > >> > On 11/18/2013 11:19 AM, Serge E. Hallyn wrote: > >> > > Quoting Serge E. Hallyn (serge@xxxxxxxxxx): > >> > >> 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 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
