Re: [PATCH -v2 1/4] CAPABILITIES: add cpu endian vfs caps structure

Serge E. Hallyn
2008-11-04T16:45:29Z

Quoting Eric Paris (eparis@redhat.com):
> This patch add a generic cpu endian caps structure and externally available
> functions which retrieve fcaps information from disk. This information is
> necessary so fcaps information can be collected and recorded by the audit
> system.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>

Looks good, and seems to make the code a bit easier to read as well.

Acked-by: Serge Hallyn <serue@us.ibm.com>

thanks,
-serge

> ---
>
> include/linux/capability.h | 7 ++
> security/commoncap.c | 129 ++++++++++++++++++++++++--------------------
> 2 files changed, 78 insertions(+), 58 deletions(-)
>
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 9d1fe30..9d64a9c 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -96,6 +96,13 @@ typedef struct kernel_cap_struct {
> __u32 cap[_KERNEL_CAPABILITY_U32S];
> } kernel_cap_t;
>
> +/* exact same as vfs_cap_data but in cpu endian and always filled completely */
> +struct cpu_vfs_cap_data {
> + __u32 magic_etc;
> + kernel_cap_t permitted;
> + kernel_cap_t inheritable;
> +};
> +
> #define _USER_CAP_HEADER_SIZE (sizeof(struct __user_cap_header_struct))
> #define _KERNEL_CAP_T_SIZE (sizeof(kernel_cap_t))
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 3976613..8bb95ed 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -202,17 +202,70 @@ int cap_inode_killpriv(struct dentry *dentry)
> return inode->i_op->removexattr(dentry, XATTR_NAME_CAPS);
> }
>
> -static inline int cap_from_disk(struct vfs_cap_data *caps,
> - struct linux_binprm *bprm, unsigned size)
> +static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps,
> + struct linux_binprm *bprm)
> {
> + unsigned i;
> + int ret = 0;
> +
> + if (caps->magic_etc & VFS_CAP_FLAGS_EFFECTIVE)
> + bprm->cap_effective = true;
> + else
> + bprm->cap_effective = false;
> +
> + CAP_FOR_EACH_U32(i) {
> + __u32 permitted = caps->permitted.cap[i];
> + __u32 inheritable = caps->inheritable.cap[i];
> +
> + /*
> + * pP' = (X & fP) | (pI & fI)
> + */
> + bprm->cap_post_exec_permitted.cap[i] =
> + (current->cap_bset.cap[i] & permitted) |
> + (current->cap_inheritable.cap[i] & inheritable);
> +
> + if (permitted & ~bprm->cap_post_exec_permitted.cap[i]) {
> + /*
> + * insufficient to execute correctly
> + */
> + ret = -EPERM;
> + }
> + }
> +
> + /*
> + * For legacy apps, with no internal support for recognizing they
> + * do not have enough capabilities, we return an error if they are
> + * missing some "forced" (aka file-permitted) capabilities.
> + */
> + return bprm->cap_effective ? ret : 0;
> +}
> +
> +int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps)
> +{
> + struct inode *inode = dentry->d_inode;
> __u32 magic_etc;
> unsigned tocopy, i;
> - int ret;
> + int size;
> + struct vfs_cap_data caps;
> +
> + memset(cpu_caps, 0, sizeof(struct cpu_vfs_cap_data));
> +
> + if (!inode || !inode->i_op || !inode->i_op->getxattr)
> + return -ENODATA;
> +
> + size = inode->i_op->getxattr((struct dentry *)dentry, XATTR_NAME_CAPS, &caps,
> + XATTR_CAPS_SZ);
> + if (size == -ENODATA || size == -EOPNOTSUPP) {
> + /* no data, that's ok */
> + return -ENODATA;
> + }
> + if (size < 0)
> + return size;
>
> if (size < sizeof(magic_etc))
> return -EINVAL;
>
> - magic_etc = le32_to_cpu(caps->magic_etc);
> + cpu_caps->magic_etc = magic_etc = le32_to_cpu(caps.magic_etc);
>
> switch ((magic_etc & VFS_CAP_REVISION_MASK)) {
> case VFS_CAP_REVISION_1:
> @@ -229,46 +282,13 @@ static inline int cap_from_disk(struct vfs_cap_data *caps,
> return -EINVAL;
> }
>
> - if (magic_etc & VFS_CAP_FLAGS_EFFECTIVE) {
> - bprm->cap_effective = true;
> - } else {
> - bprm->cap_effective = false;
> - }
> -
> - ret = 0;
> -
> CAP_FOR_EACH_U32(i) {
> - __u32 value_cpu;
> -
> - if (i >= tocopy) {
> - /*
> - * Legacy capability sets have no upper bits
> - */
> - bprm->cap_post_exec_permitted.cap[i] = 0;
> - continue;
> - }
> - /*
> - * pP' = (X & fP) | (pI & fI)
> - */
> - value_cpu = le32_to_cpu(caps->data[i].permitted);
> - bprm->cap_post_exec_permitted.cap[i] =
> - (current->cap_bset.cap[i] & value_cpu) |
> - (current->cap_inheritable.cap[i] &
> - le32_to_cpu(caps->data[i].inheritable));
> - if (value_cpu & ~bprm->cap_post_exec_permitted.cap[i]) {
> - /*
> - * insufficient to execute correctly
> - */
> - ret = -EPERM;
> - }
> + if (i >= tocopy)
> + break;
> + cpu_caps->permitted.cap[i] = le32_to_cpu(caps.data[i].permitted);
> + cpu_caps->inheritable.cap[i] = le32_to_cpu(caps.data[i].inheritable);
> }
> -
> - /*
> - * For legacy apps, with no internal support for recognizing they
> - * do not have enough capabilities, we return an error if they are
> - * missing some "forced" (aka file-permitted) capabilities.
> - */
> - return bprm->cap_effective ? ret : 0;
> + return 0;
> }
>
> /* Locate any VFS capabilities: */
> @@ -276,8 +296,7 @@ static int get_file_caps(struct linux_binprm *bprm)
> {
> struct dentry *dentry;
> int rc = 0;
> - struct vfs_cap_data vcaps;
> - struct inode *inode;
> + struct cpu_vfs_cap_data vcaps;
>
> bprm_clear_caps(bprm);
>
> @@ -285,24 +304,18 @@ static int get_file_caps(struct linux_binprm *bprm)
> return 0;
>
> dentry = dget(bprm->file->f_dentry);
> - inode = dentry->d_inode;
> - if (!inode->i_op || !inode->i_op->getxattr)
> - goto out;
>
> - rc = inode->i_op->getxattr(dentry, XATTR_NAME_CAPS, &vcaps,
> - XATTR_CAPS_SZ);
> - if (rc == -ENODATA || rc == -EOPNOTSUPP) {
> - /* no data, that's ok */
> - rc = 0;
> + rc = get_vfs_caps_from_disk(dentry, &vcaps);
> + if (rc < 0) {
> + if (rc == -EINVAL)
> + printk(KERN_NOTICE "%s: get_vfs_caps_from_disk returned %d for %s\n",
> + __func__, rc, bprm->filename);
> + else if (rc == -ENODATA)
> + rc = 0;
> goto out;
> }
> - if (rc < 0)
> - goto out;
>
> - rc = cap_from_disk(&vcaps, bprm, rc);
> - if (rc == -EINVAL)
> - printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n",
> - __func__, rc, bprm->filename);
> + rc = bprm_caps_from_vfs_caps(&vcaps, bprm);
>
> out:
> dput(dentry);