Comment 16 for bug 1531747

Revision history for this message
Seth Forshee (sforshee) wrote : Re: [Bug 1531747] Re: overlay: mkdir fails if directory exists in lowerdir in a user namespace

On Tue, Jan 12, 2016 at 05:16:28PM -0000, Serge Hallyn wrote:
> I'm still looking, but it may be safe to say that all needed inode
> checks are already done before we call ovl_set_opaque() so that we
> can indeed just use prepare_kernel_cred(NULL) instead of prepare_cred().

I've been looking too, only at ovl_create_over_whiteout so far. Here's
my take, though I'm still not an overlayfs expert so I may have missed
something.

Obviously the vfs is going to validate permissions to do the requested
operation in the overlayfs superblock. This will result in
ovl_permission getting called, which (for any write operation) is going
to validate that the caller has the necessary permissions towards the
relevant inode in uppderdir. That's good.

So by the time we raise the caps and call ovl_create_over_whiteout we
know that the user is allowed to do the operation, and
ovl_create_or_link is only changing upperdir in the ways needed to
satisfy this operation. Thus having the kernel assume full-on
CAP_SYS_ADMIN before calling ovl_create_or_link seems fine wrt to
upperdir.

But nothing is being checked wrt workdir before elevating the
capabilities. So maybe there are some checks at mount time to make sure
the user has permissions needed towards workdir? ovl_fill_super doesn't
appear to do so explicitly. However it does call ovl_workdir_create, and
this expects that $WORKDIR/work doesn't already exist (where $WORKDIR is
the string passed to mount via workdir=). If it does exist it tries to
remove it with either vfs_rmdir or vfs_unlink (both of which call
may_delete), then it tries to create $WORKDIR/work using vfs_mkdir
(which calls may_create). If either of these fails the mount is
read-only. This directory is what is used as the workdir for the mount,
so I think this effectively validates that the kernel can safely modify
the workdir in the same ways that it's modifying the upperdir.