On 10/04/2013 07:03:23 PM, Eric W. Biederman wrote:
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:> On Fri, Oct 4, 2013 at 3:41 PM, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:>> >> After thinking about it removing the restrictions on mount points >> appears safe, because it is just plain dumb to have a mount point >> in a directory that is not restricted to root only modifications. >>>> This is a change in user visible semantics, so I want to be very careful>> about this. Are there any reasons to not make this change? > > At least one worry: people are very used to 'rmdir()' not removing > empty directories, and I've written code myself that just does an> 'rmdir()' without worrying about it. I think git has code like "remove> file, then try to remove directory file is in, and recurse until it> fails or you hit the top of tree". And it all depends on knowing that> rmdir() is harmless, and returns the appropriate error when the > directory isn't empty. > > And you're now changing that. If it's a mount-point, the rmdir just > succeeds, afaik. >> Does anybody care? I dunno. But it looks like a _big_ semantic change.Which is definitley why I am asking and being careful.> That said, I like the _concept_ of being able to remove a mount-point > and the mount just goes away. But I do think that for sanity sake, it> should have something like "if one of the mounts is in the current > namespace, return -EBUSY". IOW, the patch-series would make the VFS > layer _able_ to remove mount-points, but a normal rmdir() when > something is mounted in that namespace would fail in order to give > legacy behavior. > > Hmm? In principle I have no problems tweaking rmdir to check for that case.At the same time the real reason that this is safe is that mount points are almost always part of trusted paths to important files and you justdon't mess with those paths. So tweaking rmdir to fail would be more about making stupid mistakes like running "rm -rf /" fail than it would be about security or correctness.
If you do an rm -rf descending through a mount point, it's going to delete the contents of the mount point before _trying_ to unlink the mount point, which may be bad for the thing you mounted. Then there's the fun corner case of "the directory wasn't empty before it was mounted on, and umounting revealed the overmounted files, so the unlink fails for that reason even after magic umount".
Doing rmdir on a non-empty directory won't delete it if it _isn't_ a mount point, and presumably we require write access to directories to mount on them, so in what ways is this different than another user mucking about with my files asynchronously?
Oh, attached is a dumb "zapchroot" script I've been using for years to unlink all mount points under a given directory, taking advantage of the fact that mount points are appended to the end of the list so if you unlink from the end to the front you should get the sub-mounts before the parent mounts (modulo mount --move not reordering the list, but that's uncommon).
Recently I noticed some kernels where chroot does _not_ trim the paths so that the paths you see in /proc/mounts are relevant to the current chroot but instead have all sorts of crap you can't access with no way to know what it's talking about. That was sad, I need to go figure out if that was distro breakage or vanilla breakage...
Rob
Attachment:
zapchroot
Description: application/shellscript
