Rob Landley <rob@xxxxxxxxxxx> writes: > On 10/04/2013 07:03:23 PM, Eric W. Biederman wrote: >> >> 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". Yes. Adding the restrictions I added in 4/3 are really just about preserving peoples experience that rmdir on a directory ls shows files in fails. Just to be a little less surprising. > 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? We don't require write access to files or directories before we mount on them. The permissions we require to mount is that the directory or file is visible aka execute permissions all of the way to /, and that the mounter has CAP_SYS_ADMIN in the user namespace that created the mount namespace. Basically that the mounter is locally root. In practice this means that if someone mounts something on your directory it happens in another mount namespace and you don't care. You don't even know about it unless you call rmdir and today that returns -EBUSY. The permission checks could be extended to ensure that the uid of the file or directory is mapped in to the mounter's user namespace. However that would arguably require extending the vfs -EBUSY handling to chown or chmod. And even without that we are still left with the mess that is the current magic insertion of -EBUSY because we have to deal with mount point somehow. So what I am proposing is removing the -EBUSY handling making it up to whoever creates a mount to put the mount on path they trust if they don't want the path to go away. So for the owner of the directory who doesn't see the mount it is nothing like a user scribbling into the directory. With these patches it becomes almost exactly like scribling in someone elses directory for the creator of the mount. Further even without user namespaces and with root creating all of the mounts we remove a current challenge in the VFS that requires filesystems to lie about their directory structure. Unless there is someone crazy enough to put mount points in directories that are world writable I think my current patchset is a very nice cleanup. I have meandered all over the field. Did I answer your question? I wasn't exactly certain what you were asking. FYI: As for chroot you can not create a user namespace if you are chrooted, so I suggest you file chroot under that old thing that was an early poor approximation of mount namespaces that we don't need anymore. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
