Quoting KaiGai Kohei ([EMAIL PROTECTED]):
> Hi, Serge.
>
> Thanks for the information.
> I'll update the userspace utilities next weekend.
Ok - so this change does make sense to you?
Upping _LINUX_CAPABILITY_VERSION seems drastic, but anyone who's already
been using the current patch would end up unable to run old cap-enhanced
binaries.
Now that I think about it, I suppose my patch should handle the older
_LINUX_CAPABILITY_VERSION binaries if it runs by them. I'll send an
updated patch, though maybe not today.
> Please wait for a while.
Thanks :)
-serge
>
> 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 <[EMAIL PROTECTED]>
> >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 <[EMAIL PROTECTED]>
> >
> >---
> >
> > 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 <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
