On Thu, Jul 25, 2013 at 11:52 PM, Casey Schaufler <ca...@schaufler-ca.com> wrote: > The /proc/*/attr interfaces are given to one LSM. This can be > done by setting CONFIG_SECURITY_PRESENT. Additional interfaces > have been created in /proc/*/attr so that each LSM has its own > named interfaces. The name of the presenting LSM can be read from
For me, this is one problem that was bothering me, but it was a cosmetic one that I'd mentioned before: I really disliked the /proc/$pid/attr interface being named "$lsm.$file". I feel it's important to build directories in attr/ for each LSM. So, I spent time to figure out a way to do this. This patch changes the interface to /proc/$pid/attr/$lsm/$file instead, which I feel has a much more appealing organizational structure. -Kees --- Subject: [PATCH] LSM: use subdirectories for LSM attr files Instead of filling the /proc/$pid/attr/ directory with every LSM's needed attr files, insert a directory entry for each LSM which contains the needed files. Signed-off-by: Kees Cook <keesc...@chromium.org> --- fs/proc/base.c | 95 ++++++++++++++++++++++++++++++++++++---------- fs/proc/internal.h | 1 + include/linux/security.h | 11 +++--- security/security.c | 67 ++++++++++++++------------------ 4 files changed, 112 insertions(+), 62 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 941fe83..4c80ffd 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -138,6 +138,10 @@ struct pid_entry { NOD(NAME, (S_IFREG|(MODE)), \ NULL, &proc_single_file_operations, \ { .proc_show = show } ) +#define ATTR(LSM, NAME, MODE) \ + NOD(NAME, (S_IFREG|(MODE)), \ + NULL, &proc_pid_attr_operations, \ + { .lsm = LSM } ) /* * Count the number of hardlinks for the pid_entry table, excluding the . @@ -2292,7 +2296,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf, if (!task) return -ESRCH; - length = security_getprocattr(task, + length = security_getprocattr(task, PROC_I(inode)->op.lsm, (char*)file->f_path.dentry->d_name.name, &p); put_task_struct(task); @@ -2335,7 +2339,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf, if (length < 0) goto out_free; - length = security_setprocattr(task, + length = security_setprocattr(task, PROC_I(inode)->op.lsm, (char*)file->f_path.dentry->d_name.name, (void*)page, count); mutex_unlock(&task->signal->cred_guard_mutex); @@ -2353,29 +2357,82 @@ static const struct file_operations proc_pid_attr_operations = { .llseek = generic_file_llseek, }; +#define LSM_DIR_OPS(LSM) \ +static int proc_##LSM##_attr_dir_readdir(struct file * filp, \ + void * dirent, filldir_t filldir) \ +{ \ + return proc_pident_readdir(filp, dirent, filldir, \ + LSM##_attr_dir_stuff, \ + ARRAY_SIZE(LSM##_attr_dir_stuff)); \ +} \ +\ +static const struct file_operations proc_##LSM##_attr_dir_ops = { \ + .read = generic_read_dir, \ + .readdir = proc_##LSM##_attr_dir_readdir, \ + .llseek = default_llseek, \ +}; \ +\ +static struct dentry *proc_##LSM##_attr_dir_lookup(struct inode *dir, \ + struct dentry *dentry, unsigned int flags) \ +{ \ + return proc_pident_lookup(dir, dentry, \ + LSM##_attr_dir_stuff, \ + ARRAY_SIZE(LSM##_attr_dir_stuff)); \ +} \ +\ +static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \ + .lookup = proc_##LSM##_attr_dir_lookup, \ + .getattr = pid_getattr, \ + .setattr = proc_setattr, \ +}; + +#ifdef CONFIG_SECURITY_SELINUX +static const struct pid_entry selinux_attr_dir_stuff[] = { + ATTR("selinux", "current", S_IRUGO|S_IWUGO), + ATTR("selinux", "prev", S_IRUGO), + ATTR("selinux", "exec", S_IRUGO|S_IWUGO), + ATTR("selinux", "fscreate", S_IRUGO|S_IWUGO), + ATTR("selinux", "keycreate", S_IRUGO|S_IWUGO), + ATTR("selinux", "sockcreate", S_IRUGO|S_IWUGO), +}; +LSM_DIR_OPS(selinux); +#endif + +#ifdef CONFIG_SECURITY_SMACK +static const struct pid_entry smack_attr_dir_stuff[] = { + ATTR("smack", "current", S_IRUGO|S_IWUGO), +}; +LSM_DIR_OPS(smack); +#endif + +#ifdef CONFIG_SECURITY_APPARMOR +static const struct pid_entry apparmor_attr_dir_stuff[] = { + ATTR("apparmor", "current", S_IRUGO|S_IWUGO), + ATTR("apparmor", "prev", S_IRUGO), + ATTR("apparmor", "exec", S_IRUGO|S_IWUGO), +}; +LSM_DIR_OPS(apparmor); +#endif + static const struct pid_entry attr_dir_stuff[] = { - REG("current", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("prev", S_IRUGO, proc_pid_attr_operations), - REG("exec", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("fscreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("keycreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("sockcreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("context", S_IRUGO|S_IWUGO, proc_pid_attr_operations), + ATTR(NULL, "current", S_IRUGO|S_IWUGO), + ATTR(NULL, "prev", S_IRUGO), + ATTR(NULL, "exec", S_IRUGO|S_IWUGO), + ATTR(NULL, "fscreate", S_IRUGO|S_IWUGO), + ATTR(NULL, "keycreate", S_IRUGO|S_IWUGO), + ATTR(NULL, "sockcreate", S_IRUGO|S_IWUGO), + ATTR(NULL, "context", S_IRUGO|S_IWUGO), #ifdef CONFIG_SECURITY_SELINUX - REG("selinux.current", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("selinux.prev", S_IRUGO, proc_pid_attr_operations), - REG("selinux.exec", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("selinux.fscreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("selinux.keycreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("selinux.sockcreate", S_IRUGO|S_IWUGO, proc_pid_attr_operations), + DIR("selinux", S_IRUGO|S_IXUGO, + proc_selinux_attr_dir_inode_ops, proc_selinux_attr_dir_ops), #endif #ifdef CONFIG_SECURITY_SMACK - REG("smack.current", S_IRUGO|S_IWUGO, proc_pid_attr_operations), + DIR("smack", S_IRUGO|S_IXUGO, + proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops), #endif #ifdef CONFIG_SECURITY_APPARMOR - REG("apparmor.current", S_IRUGO|S_IWUGO, proc_pid_attr_operations), - REG("apparmor.prev", S_IRUGO, proc_pid_attr_operations), - REG("apparmor.exec", S_IRUGO|S_IWUGO, proc_pid_attr_operations), + DIR("apparmor", S_IRUGO|S_IXUGO, + proc_apparmor_attr_dir_inode_ops, proc_apparmor_attr_dir_ops), #endif }; diff --git a/fs/proc/internal.h b/fs/proc/internal.h index d600fb0..795f649 100644 --- a/fs/proc/internal.h +++ b/fs/proc/internal.h @@ -56,6 +56,7 @@ union proc_op { int (*proc_show)(struct seq_file *m, struct pid_namespace *ns, struct pid *pid, struct task_struct *task); + const char *lsm; }; struct proc_inode { diff --git a/include/linux/security.h b/include/linux/security.h index d60e21c..fa89618 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2115,9 +2115,10 @@ int security_sem_semctl(struct sem_array *sma, int cmd); int security_sem_semop(struct sem_array *sma, struct sembuf *sops, unsigned nsops, int alter); void security_d_instantiate(struct dentry *dentry, struct inode *inode); -int security_getprocattr(struct task_struct *p, char *name, char **value); -int security_setprocattr(struct task_struct *p, char *name, void *value, - size_t size); +int security_getprocattr(struct task_struct *p, const char *lsm, char *name, + char **value); +int security_setprocattr(struct task_struct *p, const char *lsm, char *name, + void *value, size_t size); int security_netlink_send(struct sock *sk, struct sk_buff *skb); int security_secid_to_secctx(struct secids *secid, char **secdata, u32 *seclen, struct security_operations **secops); @@ -2801,8 +2802,8 @@ static inline void security_d_instantiate(struct dentry *dentry, struct inode *inode) { } -static inline int security_getprocattr(struct task_struct *p, char *name, - char **value) +static inline int security_getprocattr(struct task_struct *p, char *lsm, + char *name, char **value) { return -EINVAL; } diff --git a/security/security.c b/security/security.c index 119a377..499af30 100644 --- a/security/security.c +++ b/security/security.c @@ -1897,74 +1897,65 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode) } EXPORT_SYMBOL(security_d_instantiate); -int security_getprocattr(struct task_struct *p, char *name, char **value) +int security_getprocattr(struct task_struct *p, const char *lsm, char *name, + char **value) { struct security_operations *sop = NULL; struct secids secid; - char *lsm; - int lsmlen; int ret; /* - * Names will either be in the legacy form containing - * no periods (".") or they will be the LSM name followed - * by the legacy suffix. "current" or "selinux.current" - * The exception is "context", which gets all of the LSMs. - * - * Legacy names are handled by the presenting LSM. - * Suffixed names are handled by the named LSM. + * Target LSM will be either NULL or looked up by name. Names with + * a NULL LSM (legacy) are handled by the presenting LSM. The + * exception is "context", which gets all of the LSMs. */ if (strcmp(name, "context") == 0) { + char *lsmname; + int lsmlen; + security_task_getsecid(p, &secid); - ret = security_secid_to_secctx(&secid, &lsm, &lsmlen, &sop); + ret = security_secid_to_secctx(&secid, &lsmname, &lsmlen, &sop); if (ret == 0) { - *value = kstrdup(lsm, GFP_KERNEL); + *value = kstrdup(lsmname, GFP_KERNEL); if (*value == NULL) ret = -ENOMEM; else ret = strlen(*value); - security_release_secctx(lsm, lsmlen, sop); + security_release_secctx(lsmname, lsmlen, sop); } return ret; } - if (present_ops && !strchr(name, '.')) - return present_getprocattr(p, name, value); - - for_each_hook(sop, getprocattr) { - lsm = sop->name; - lsmlen = strlen(lsm); - if (!strncmp(name, lsm, lsmlen) && name[lsmlen] == '.') - return sop->getprocattr(p, name + lsmlen + 1, value); + if (!lsm) { + if (present_ops) + return present_getprocattr(p, name, value); + } else { + for_each_hook(sop, getprocattr) { + if (!strcmp(lsm, sop->name)) + return sop->getprocattr(p, name, value); + } } return -EINVAL; } -int security_setprocattr(struct task_struct *p, char *name, void *value, - size_t size) +int security_setprocattr(struct task_struct *p, const char *lsm, char *name, + void *value, size_t size) { struct security_operations *sop; - char *lsm; - int lsmlen; /* - * Names will either be in the legacy form containing - * no periods (".") or they will be the LSM name followed - * by the legacy suffix. - * "current" or "selinux.current" - * - * Legacy names are handled by the presenting LSM. - * Suffixed names are handled by the named LSM. + * Target LSM will be either NULL or looked up by name. Names with + * a NULL LSM (legacy) are handled by the presenting LSM. The */ if (present_ops && !strchr(name, '.')) return present_setprocattr(p, name, value, size); - for_each_hook(sop, setprocattr) { - lsm = sop->name; - lsmlen = strlen(lsm); - if (!strncmp(name, lsm, lsmlen) && name[lsmlen] == '.') - return sop->setprocattr(p, name + lsmlen + 1, value, - size); + if (lsm) { + for_each_hook(sop, setprocattr) { + if (!strcmp(lsm, sop->name)) + return sop->setprocattr(p, name, value, + size); + } } return -EINVAL; } -- 1.7.9.5 -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/