Re: [RFC][PATCH 0/3] vfs: Detach mounts on unlink.

[Rob Landley]

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  
> just
> don'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