[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <45DADB5F.1020101@kaigai.gr.jp>
Date: Tue, 20 Feb 2007 20:28:31 +0900
From: KaiGai Kohei <kaigai@...gai.gr.jp>
To: "Serge E. Hallyn" <serue@...ibm.com>
CC: lkml <linux-kernel@...r.kernel.org>,
Stephen Smalley <sds@...ho.nsa.gov>,
Andrew Morton <akpm@...ux-foundation.org>,
James Morris <jmorris@...ei.org>,
Chris Wright <chrisw@...s-sol.org>
Subject: Re: [RFC] [PATCH -mm] file caps: make on-disk capabilities future-proof
Hi, Serge.
Thanks for the information.
I'll update the userspace utilities next weekend.
Please wait for a while.
Serge E. Hallyn wrote:
> Stephen Smalley has pointed out that the current file capabilities
> will eventually pose a problem.
>
> As the capability set changes and distributions start tagging
> binaries with capabilities, we would like for running an older
> kernel to not necessarily make those binaries unusable. The
> following patch tries to address that. Kaigai, if we went with
> this patch, your userspace tools would need to be updated to
> (a) insert a size parameter, and (b) update the
> _LINUX_CAPABILITY_VERSION.
>
> It would be nice to solve this before file caps hit mainline.
>
> thanks,
> -serge
>
> From: Serge E. Hallyn <serue@...ibm.com>
> Subject: [PATCH -mm] file caps: make on-disk capabilities future-proof
>
> Stephen Smalley has pointed out that the current file capabilities
> will eventually pose a problem.
>
> As the capability set changes and distributions start tagging
> binaries with capabilities, we would like for running an older
> kernel to not necessarily make those binaries unusable. To
> that end,
>
> 1. If capabilities are specified which we don't know
> about, just ignore them, do not return -EPERM as we
> were doing before.
> 2. Specify a size with the on-disk capability implementation.
> In this implementation the size is the number of __u32's
> used for each of (eff,perm,inh). For now, sz is 1.
> When we move to 64-bit capabilities, it becomes 2.
>
> Signed-off-by: Serge E. Hallyn <serue@...ibm.com>
>
> ---
>
> include/linux/capability.h | 18 ++++++--
> security/commoncap.c | 100 +++++++++++++++++++++++++-------------------
> 2 files changed, 70 insertions(+), 48 deletions(-)
>
> f4beca776d303bbb6348dc08e4d02c3bd37f3a83
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 2776886..1de7a85 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -27,7 +27,7 @@
> library since the draft standard requires the use of malloc/free
> etc.. */
>
> -#define _LINUX_CAPABILITY_VERSION 0x19980330
> +#define _LINUX_CAPABILITY_VERSION 0x20070215
>
> typedef struct __user_cap_header_struct {
> __u32 version;
> @@ -44,11 +44,21 @@ typedef struct __user_cap_data_struct {
>
> #define XATTR_CAPS_SUFFIX "capability"
> #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
> +#define XATTR_CAPS_SZ (5*sizeof(__le32))
> +/*
> + * sz is the # of __le32's in each set, 1 for now.
> + * data[] is organized as:
> + * effective[0..sz-1]
> + * permitted[0..sz-1]
> + * inheritable[0..sz-1]
> + * ...
> + * this way we can just read as much of the on-disk capability as
> + * we know should exist and know we'll get the data we'll need.
> + */
> struct vfs_cap_data_disk {
> __le32 version;
> - __le32 effective;
> - __le32 permitted;
> - __le32 inheritable;
> + __le32 sz;
> + __le32 data[]; /* effective[sz], permitted[sz], inheritable[sz] */
> };
>
> #ifdef __KERNEL__
> diff --git a/security/commoncap.c b/security/commoncap.c
> index be86acb..dc8bf4f 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -111,35 +111,32 @@ void cap_capset_set (struct task_struct
> }
>
> #ifdef CONFIG_SECURITY_FS_CAPABILITIES
> -static inline void cap_from_disk(struct vfs_cap_data_disk *dcap,
> - struct vfs_cap_data *cap)
> +static inline int cap_from_disk(struct vfs_cap_data_disk *dcap,
> + struct vfs_cap_data *cap, int size)
> {
> + __u32 sz;
> +
> cap->version = le32_to_cpu(dcap->version);
> - cap->effective = le32_to_cpu(dcap->effective);
> - cap->permitted = le32_to_cpu(dcap->permitted);
> - cap->inheritable = le32_to_cpu(dcap->inheritable);
> + sz = le32_to_cpu(dcap->sz);
> +
> + if ((sz*3+2)*sizeof(__u32) != size) {
> + printk(KERN_NOTICE "%s: sz is %d, size is %d, should be %d\n", __FUNCTION__,
> + sz, size, (sz*3+2)*sizeof(__u32));
> + return -EINVAL;
> + }
> +
> + cap->effective = le32_to_cpu(dcap->data[0]);
> + cap->permitted = le32_to_cpu(dcap->data[sz]);
> + cap->inheritable = le32_to_cpu(dcap->data[sz*2]);
> +
> + return 0;
> }
>
> static int check_cap_sanity(struct vfs_cap_data *cap)
> {
> - int i;
> -
> if (cap->version != _LINUX_CAPABILITY_VERSION)
> return -EPERM;
>
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->effective); i++) {
> - if (cap->effective & CAP_TO_MASK(i))
> - return -EPERM;
> - }
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->permitted); i++) {
> - if (cap->permitted & CAP_TO_MASK(i))
> - return -EPERM;
> - }
> - for (i = CAP_NUMCAPS; i < 8*sizeof(cap->inheritable); i++) {
> - if (cap->inheritable & CAP_TO_MASK(i))
> - return -EPERM;
> - }
> -
> return 0;
> }
>
> @@ -148,50 +145,65 @@ static int set_file_caps(struct linux_bi
> {
> struct dentry *dentry;
> ssize_t rc;
> - struct vfs_cap_data_disk dcaps;
> + struct vfs_cap_data_disk *dcaps;
> struct vfs_cap_data caps;
> struct inode *inode;
> - int err;
>
> if (bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)
> return 0;
>
> dentry = dget(bprm->file->f_dentry);
> inode = dentry->d_inode;
> - if (!inode->i_op || !inode->i_op->getxattr) {
> - dput(dentry);
> - return 0;
> + rc = 0;
> + if (!inode->i_op || !inode->i_op->getxattr)
> + goto out;
> +
> + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, NULL, 0);
> + if (rc == -ENODATA) {
> + rc = 0;
> + goto out;
> + }
> + if (rc < 0)
> + goto out;
> + if (rc < sizeof(struct vfs_cap_data_disk)) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + dcaps = kmalloc(rc, GFP_KERNEL);
> + if (!dcaps) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, dcaps,
> + XATTR_CAPS_SZ);
> + if (rc == -ENODATA) {
> + rc = 0;
> + goto out_free;
> }
>
> - rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &dcaps,
> - sizeof(dcaps));
> - dput(dentry);
> -
> - if (rc == -ENODATA)
> - return 0;
> -
> if (rc < 0) {
> printk(KERN_NOTICE "%s: Error (%zd) getting xattr\n",
> __FUNCTION__, rc);
> - return rc;
> + goto out_free;
> }
>
> - if (rc != sizeof(dcaps)) {
> - printk(KERN_NOTICE "%s: got wrong size for getxattr (%zd)\n",
> - __FUNCTION__, rc);
> - return -EPERM;
> - }
> -
> - cap_from_disk(&dcaps, &caps);
> - err = check_cap_sanity(&caps);
> - if (err)
> - return err;
> + rc = cap_from_disk(dcaps, &caps, rc);
> + if (rc)
> + goto out_free;
> + rc = check_cap_sanity(&caps);
> + if (rc)
> + goto out_free;
>
> bprm->cap_effective = caps.effective;
> bprm->cap_permitted = caps.permitted;
> bprm->cap_inheritable = caps.inheritable;
>
> - return 0;
> +out_free:
> + kfree(dcaps);
> +out:
> + dput(dentry);
> + return rc;
> }
> #else
> static inline int set_file_caps(struct linux_binprm *bprm)
--
KaiGai Kohei <kaigai@...gai.gr.jp>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists