lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sat, 05 Oct 2013 18:42:44 -0500
From:	Rob Landley <rob@...dley.net>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andy Lutomirski <luto@...capital.net>
Subject: Re: [RFC][PATCH 0/3] vfs: Detach mounts on unlink.

On 10/04/2013 07:03:23 PM, Eric W. Biederman wrote:
> Linus Torvalds <torvalds@...ux-foundation.org> writes:
> 
> > On Fri, Oct 4, 2013 at 3:41 PM, Eric W. Biederman  
> <ebiederm@...ssion.com> 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
Download attachment "zapchroot" of type "application/x-shellscript" (749 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ