From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Stephen Smalley <sds@tycho.nsa.gov>,
linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ovl: fix uid/gid when creating over whiteout
Date: Wed, 15 Jun 2016 10:01:51 -0400 [thread overview]
Message-ID: <20160615140151.GB19388@redhat.com> (raw)
In-Reply-To: <20160615133002.GA11993@veci.piliscsaba.szeredi.hu>
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@redhat.com>
> 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@tycho.nsa.gov>
> Fixes: 3fe6e52f0626 ("ovl: override creds with the ones from the superblock mounter")
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> 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);
> }
>
next prev parent reply other threads:[~2016-06-15 14:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 13:30 [PATCH] ovl: fix uid/gid when creating over whiteout Miklos Szeredi
2016-06-15 14:01 ` Vivek Goyal [this message]
2016-06-15 14:09 ` Miklos Szeredi
2016-06-15 14:20 ` Vivek Goyal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160615140151.GB19388@redhat.com \
--to=vgoyal@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=sds@tycho.nsa.gov \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).