On Wed, Jun 15, 2016 at 03:30:02PM +0200, Miklos Szeredi wrote: > Hi Vivek, > > I've tested this to fix the regresion that Stephen reported. I think this > also is a good base for the selinux fix. > > Pushed to overlayfs-linus and overlayfs-next branches of > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git > > Please let me know if you see any problem with this. > > Thanks, > Miklos > > --- > From: Miklos Szeredi <[email protected]> > Subject: ovl: fix uid/gid when creating over whiteout > > Fix a regression when creating a file over a whiteout. The new > file/directory needs to use the current fsuid/fsgid, not the ones from the > mounter's credentials. > > The refcounting is a bit tricky: prepare_creds() sets an original refcount, > override_creds() gets one more, which revert_cred() drops. So > > 1) we need to expicitly put the mounter's credentials when overriding > with the updated one > > 2) we need to put the original ref to the updated creds (and this can > safely be done before revert_creds(), since we'll still have the ref > from override_creds()). > > Reported-by: Stephen Smalley <[email protected]> > Fixes: 3fe6e52f0626 ("ovl: override creds with the ones from the superblock > mounter") > Signed-off-by: Miklos Szeredi <[email protected]> > --- > fs/overlayfs/dir.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > --- a/fs/overlayfs/dir.c > +++ b/fs/overlayfs/dir.c > @@ -405,12 +405,21 @@ static int ovl_create_or_link(struct den > err = ovl_create_upper(dentry, inode, &stat, link, hardlink); > } else { > const struct cred *old_cred; > + struct cred *override_cred; > > old_cred = ovl_override_creds(dentry->d_sb); > > - err = ovl_create_over_whiteout(dentry, inode, &stat, link, > - hardlink); > + err = -ENOMEM; > + override_cred = prepare_creds(); > + if (override_cred) { > + override_cred->fsuid = old_cred->fsuid; > + override_cred->fsgid = old_cred->fsgid;
Hi Miklos, I am wondering if we are switching to tasks's ->fsuid and ->fsgid too early. ovl_create_over_whiteout() calls ovl_lookup_temp(workdir) and IIUC, task might not have permission to do lookup in workdir. Should we switch to this override_cred, just before ovl_create_real() so that task ->fsuid and ->fsgid are used only for creation purposes only. Thanks Vivek > + put_cred(override_creds(override_cred)); > + put_cred(override_cred); > > + err = ovl_create_over_whiteout(dentry, inode, &stat, > + link, hardlink); > + } > revert_creds(old_cred); > } >
