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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.