From: Serge Hallyn <serge.hallyn@ubuntu.com>
To: Andy Whitcroft <apw@canonical.com>
Cc: "Miklos Szeredi" <miklos@szeredi.hu>,
Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
kernel-team@lists.ubuntu.com,
"Stéphane Graber" <stephane.graber@canonical.com>
Subject: Re: [RFC PATCH 1/1] overlayfs: use kernel service credentials for copy up and xattr manipulations
Date: Wed, 5 Mar 2014 14:01:44 -0600 [thread overview]
Message-ID: <20140305200144.GA10027@sergelap> (raw)
In-Reply-To: <1394041592-3772-3-git-send-email-apw@canonical.com>
Quoting Andy Whitcroft (apw@canonical.com):
> We need to be priviledged to perform operations such as copy up and
> xattr manipulations on "trusted.". Use prepare_kernel_cred to obtain
> the necessary priviledges; from its documentation:
>
> "Prepare a set of credentials for a kernel service. This can then be
> used to override a task's own credentials so that work can be done
> on behalf of that task that requires a different subjective context."
>
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
Thanks, Andy. Both of these worked, and both look sane. I prefer
this patch, personally, but my test+ack applies to either.
Tested-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> ---
> fs/overlayfs/copy_up.c | 14 +-------------
> fs/overlayfs/dir.c | 35 +++++++++++------------------------
> fs/overlayfs/readdir.c | 18 ++----------------
> fs/overlayfs/super.c | 4 +---
> 4 files changed, 15 insertions(+), 56 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 351c162..c7894de 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -276,24 +276,12 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
> }
>
> err = -ENOMEM;
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> goto out_free_link;
>
> override_cred->fsuid = stat->uid;
> override_cred->fsgid = stat->gid;
> - /*
> - * CAP_SYS_ADMIN for copying up extended attributes
> - * CAP_DAC_OVERRIDE for create
> - * CAP_FOWNER for chmod, timestamp update
> - * CAP_FSETID for chmod
> - * CAP_MKNOD for mknod
> - */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> - cap_raise(override_cred->cap_effective, CAP_DAC_OVERRIDE);
> - cap_raise(override_cred->cap_effective, CAP_FOWNER);
> - cap_raise(override_cred->cap_effective, CAP_FSETID);
> - cap_raise(override_cred->cap_effective, CAP_MKNOD);
> old_cred = override_creds(override_cred);
>
> mutex_lock_nested(&upperdir->d_inode->i_mutex, I_MUTEX_PARENT);
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index a209409..0a7eb4a 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -11,6 +11,7 @@
> #include <linux/namei.h>
> #include <linux/xattr.h>
> #include <linux/security.h>
> +#include <linux/sched.h>
> #include <linux/cred.h>
> #include "overlayfs.h"
>
> @@ -26,20 +27,16 @@ static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
> /* FIXME: recheck lower dentry to see if whiteout is really needed */
>
> err = -ENOMEM;
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> goto out;
>
> - /*
> - * CAP_SYS_ADMIN for setxattr
> - * CAP_DAC_OVERRIDE for symlink creation
> - * CAP_FOWNER for unlink in sticky directory
> - */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> - cap_raise(override_cred->cap_effective, CAP_DAC_OVERRIDE);
> - cap_raise(override_cred->cap_effective, CAP_FOWNER);
> - override_cred->fsuid = GLOBAL_ROOT_UID;
> - override_cred->fsgid = GLOBAL_ROOT_GID;
> + override_cred->fsuid = make_kuid(current_user_ns(), 0);
> + if (!uid_valid(override_cred->fsuid))
> + override_cred->fsuid = GLOBAL_ROOT_UID;
> + override_cred->fsgid = make_kgid(current_user_ns(), 0);
> + if (!gid_valid(override_cred->fsgid))
> + override_cred->fsgid = GLOBAL_ROOT_GID;
> old_cred = override_creds(override_cred);
>
> newdentry = lookup_one_len(dentry->d_name.name, upperdir,
> @@ -103,16 +100,10 @@ static struct dentry *ovl_lookup_create(struct dentry *upperdir,
> goto out_dput;
>
> err = -ENOMEM;
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> goto out_dput;
>
> - /*
> - * CAP_SYS_ADMIN for getxattr
> - * CAP_FOWNER for unlink in sticky directory
> - */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> - cap_raise(override_cred->cap_effective, CAP_FOWNER);
> old_cred = override_creds(override_cred);
>
> err = -EEXIST;
> @@ -205,12 +196,10 @@ static int ovl_set_opaque(struct dentry *upperdentry)
> const struct cred *old_cred;
> struct cred *override_cred;
>
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> return -ENOMEM;
>
> - /* CAP_SYS_ADMIN for setxattr of "trusted" namespace */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> old_cred = override_creds(override_cred);
> err = vfs_setxattr(upperdentry, ovl_opaque_xattr, "y", 1, 0);
> revert_creds(old_cred);
> @@ -225,12 +214,10 @@ static int ovl_remove_opaque(struct dentry *upperdentry)
> const struct cred *old_cred;
> struct cred *override_cred;
>
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> return -ENOMEM;
>
> - /* CAP_SYS_ADMIN for removexattr of "trusted" namespace */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> old_cred = override_creds(override_cred);
> err = vfs_removexattr(upperdentry, ovl_opaque_xattr);
> revert_creds(old_cred);
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 9c6f08f..1cc4cbd 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -218,18 +218,12 @@ static int ovl_dir_mark_whiteouts(struct ovl_readdir_data *rdd)
> const struct cred *old_cred;
> struct cred *override_cred;
>
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred) {
> ovl_cache_free(rdd->list);
> return -ENOMEM;
> }
>
> - /*
> - * CAP_SYS_ADMIN for getxattr
> - * CAP_DAC_OVERRIDE for lookup
> - */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> - cap_raise(override_cred->cap_effective, CAP_DAC_OVERRIDE);
> old_cred = override_creds(override_cred);
>
> mutex_lock(&rdd->dir->d_inode->i_mutex);
> @@ -503,18 +497,10 @@ static int ovl_remove_whiteouts(struct dentry *dir, struct list_head *list)
> ovl_path_upper(dir, &upperpath);
> upperdir = upperpath.dentry;
>
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> return -ENOMEM;
>
> - /*
> - * CAP_DAC_OVERRIDE for lookup and unlink
> - * CAP_SYS_ADMIN for setxattr of "trusted" namespace
> - * CAP_FOWNER for unlink in sticky directory
> - */
> - cap_raise(override_cred->cap_effective, CAP_DAC_OVERRIDE);
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> - cap_raise(override_cred->cap_effective, CAP_FOWNER);
> old_cred = override_creds(override_cred);
>
> err = vfs_setxattr(upperdir, ovl_opaque_xattr, "y", 1, 0);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 50890c2..79288a8 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -304,12 +304,10 @@ static int ovl_do_lookup(struct dentry *dentry)
> struct cred *override_cred;
>
> err = -ENOMEM;
> - override_cred = prepare_creds();
> + override_cred = prepare_kernel_cred(NULL);
> if (!override_cred)
> goto out_dput_upper;
>
> - /* CAP_SYS_ADMIN needed for getxattr */
> - cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
> old_cred = override_creds(override_cred);
>
> if (ovl_is_opaquedir(upperdentry)) {
> --
> 1.9.0
prev parent reply other threads:[~2014-03-05 20:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-25 17:31 [PATCH RFC] overlayfs,xattr: allow unprivileged users to whiteout Serge Hallyn
2014-02-28 14:15 ` Miklos Szeredi
2014-02-28 14:55 ` Andy Whitcroft
2014-02-28 16:23 ` Serge Hallyn
2014-03-05 17:46 ` [RFC] overlayfs priviledge escalation handling under user namespaces Andy Whitcroft
2014-03-05 17:46 ` [RFC PATCH 1/1] overlayfs: switch to the init user namespace for xattr operations Andy Whitcroft
2014-03-05 17:46 ` [RFC PATCH 1/1] overlayfs: use kernel service credentials for copy up and xattr manipulations Andy Whitcroft
2014-03-05 20:01 ` Serge Hallyn [this message]
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=20140305200144.GA10027@sergelap \
--to=serge.hallyn@ubuntu.com \
--cc=apw@canonical.com \
--cc=kernel-team@lists.ubuntu.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=stephane.graber@canonical.com \
/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).