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 <mszeredi@xxxxxxxxxx>
> 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 <sds@xxxxxxxxxxxxx>
> Fixes: 3fe6e52f0626 ("ovl: override creds with the ones from the superblock mounter")
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> ---
> 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);
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
