From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932887AbXBTL24 (ORCPT ); Tue, 20 Feb 2007 06:28:56 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932892AbXBTL24 (ORCPT ); Tue, 20 Feb 2007 06:28:56 -0500 Received: from py-out-1112.google.com ([64.233.166.179]:55006 "EHLO py-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932887AbXBTL2y (ORCPT ); Tue, 20 Feb 2007 06:28:54 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type:content-transfer-encoding:sender; b=mstcVN63tm5y20ZSrYKypOjGI7iSfff90epMafLH6X60P0lJk938Z6C+zn5d3UVLKMMUHVzi6LC+ZLFI3kuCg1dtZyV6ajKQDj4EgeeJ+vH4fjI+ReGaHR2XbsirXh8mVHCQlOCYxTWT+IqX3L7cpPL5SUDwbUS9Q3k5eLdhwSU= Message-ID: <45DADB5F.1020101@kaigai.gr.jp> Date: Tue, 20 Feb 2007 20:28:31 +0900 From: KaiGai Kohei User-Agent: Thunderbird 1.5.0.9 (Windows/20061207) MIME-Version: 1.0 To: "Serge E. Hallyn" CC: lkml , Stephen Smalley , Andrew Morton , James Morris , Chris Wright Subject: Re: [RFC] [PATCH -mm] file caps: make on-disk capabilities future-proof References: <20070219170133.GB28412@sergelap.austin.ibm.com> In-Reply-To: <20070219170133.GB28412@sergelap.austin.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org 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 > 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 > > --- > > 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