Re: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm

From: Casey Schaufler
Date: Tue Jun 14 2016 - 15:39:28 EST

Next message: Peter Zijlstra: "Re: [PATCH] sched: Do not release current rq lock on non contended double_lock_balance()"
Previous message: Bjorn Andersson: "Re: [PATCH] mfd: qcom_rpm: fix offset error for msm8660"
In reply to: Kees Cook: "Re: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm"
Next in thread: David A. Wheeler: "Re: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 6/14/2016 11:41 AM, Kees Cook wrote:
> On Fri, Jun 10, 2016 at 2:25 PM, Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>> Subject: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm
>>
>> I got tired of having to find indirect ways to
>> determine what security modules are active on a system.
>> I have added /sys/kernel/security/lsm, which contains a
>> comma separated list of the active security modules. No
>> more groping around in /proc/filesystems, which won't
>> help if the module doesn't support its own filesystem.
>>
>> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> Looks good to me. I might tweak the copy code below, but otherwise:
>
> Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
>
>> ---
>> include/linux/lsm_hooks.h | 12 ++++--------
>> security/apparmor/lsm.c | 3 ++-
>> security/commoncap.c | 3 ++-
>> security/inode.c | 26 ++++++++++++++++++++++++--
>> security/loadpin/loadpin.c | 2 +-
>> security/security.c | 39 +++++++++++++++++++++++++++++++++++++++
>> security/selinux/hooks.c | 2 +-
>> security/smack/smack_lsm.c | 2 +-
>> security/tomoyo/tomoyo.c | 2 +-
>> security/yama/yama_lsm.c | 2 +-
>> 10 files changed, 76 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 7ae3976..0bfd441 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1841,6 +1841,7 @@ struct security_hook_list {
>> struct list_head list;
>> struct list_head *head;
>> union security_list_options hook;
>> + char *lsm;
>> };
>>
>> /*
>> @@ -1853,15 +1854,10 @@ struct security_hook_list {
>> { .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } }
>>
>> extern struct security_hook_heads security_hook_heads;
>> +extern char *lsm_names;
>>
>> -static inline void security_add_hooks(struct security_hook_list *hooks,
>> - int count)
>> -{
>> - int i;
>> -
>> - for (i = 0; i < count; i++)
>> - list_add_tail_rcu(&hooks[i].list, hooks[i].head);
>> -}
>> +extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> + char *lsm);
>>
>> #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>> /*
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 2660fbc..fb0fb03 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -880,7 +880,8 @@ static int __init apparmor_init(void)
>> aa_free_root_ns();
>> goto alloc_out;
>> }
>> - security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks));
>> + security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
>> + "apparmor");
>>
>> /* Report that AppArmor successfully initialized */
>> apparmor_initialized = 1;
>> diff --git a/security/commoncap.c b/security/commoncap.c
>> index e7fadde..d101734 100644
>> --- a/security/commoncap.c
>> +++ b/security/commoncap.c
>> @@ -1090,7 +1090,8 @@ struct security_hook_list capability_hooks[] = {
>>
>> void __init capability_add_hooks(void)
>> {
>> - security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks));
>> + security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
>> + "capability");
>> }
>>
>> #endif /* CONFIG_SECURITY */
>> diff --git a/security/inode.c b/security/inode.c
>> index 28414b0..235ad82 100644
>> --- a/security/inode.c
>> +++ b/security/inode.c
>> @@ -20,6 +20,7 @@
>> #include <linux/init.h>
>> #include <linux/namei.h>
>> #include <linux/security.h>
>> +#include <linux/lsm_hooks.h>
>> #include <linux/magic.h>
>>
>> static struct vfsmount *mount;
>> @@ -208,6 +209,21 @@ void securityfs_remove(struct dentry *dentry)
>> }
>> EXPORT_SYMBOL_GPL(securityfs_remove);
>>
>> +#ifdef CONFIG_SECURITY
>> +static struct dentry *lsm_dentry;
>> +static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>> + loff_t *ppos)
>> +{
>> + return simple_read_from_buffer(buf, count, ppos, lsm_names,
>> + strlen(lsm_names));
>> +}
>> +
>> +static const struct file_operations lsm_ops = {
>> + .read = lsm_read,
>> + .llseek = generic_file_llseek,
>> +};
>> +#endif
>> +
>> static int __init securityfs_init(void)
>> {
>> int retval;
>> @@ -217,9 +233,15 @@ static int __init securityfs_init(void)
>> return retval;
>>
>> retval = register_filesystem(&fs_type);
>> - if (retval)
>> + if (retval) {
>> sysfs_remove_mount_point(kernel_kobj, "security");
>> - return retval;
>> + return retval;
>> + }
>> +#ifdef CONFIG_SECURITY
>> + lsm_dentry = securityfs_create_file("lsm", S_IRUGO, NULL, NULL,
>> + &lsm_ops);
>> +#endif
>> + return 0;
>> }
>>
>> core_initcall(securityfs_init);
>> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
>> index 89a46f1..1d82eae 100644
>> --- a/security/loadpin/loadpin.c
>> +++ b/security/loadpin/loadpin.c
>> @@ -182,7 +182,7 @@ static struct security_hook_list loadpin_hooks[] = {
>> void __init loadpin_add_hooks(void)
>> {
>> pr_info("ready to pin (currently %sabled)", enabled ? "en" : "dis");
>> - security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks));
>> + security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
>> }
>>
>> /* Should not be mutable after boot, so not listed in sysfs (perm == 0). */
>> diff --git a/security/security.c b/security/security.c
>> index c4bb47d..dd792d5 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -32,6 +32,7 @@
>> /* Maximum number of letters for an LSM name string */
>> #define SECURITY_NAME_MAX 10
>>
>> +char *lsm_names;
>> /* Boot-time LSM user choice */
>> static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
>> CONFIG_DEFAULT_SECURITY;
>> @@ -78,6 +79,23 @@ static int __init choose_lsm(char *str)
>> }
>> __setup("security=", choose_lsm);
>>
>> +static int lsm_append(char *new, char **result)
>> +{
>> + char *cp;
>> +
>> + if (*result == NULL) {
>> + *result = kstrdup(new, GFP_KERNEL);
>> + } else {
>> + cp = kzalloc(strlen(*result) + strlen(new) + 2, GFP_KERNEL);
>> + if (cp == NULL)
>> + return -ENOMEM;
>> + sprintf(cp, "%s,%s", *result, new);
> Instead of the kzalloc+sprintf, this could be:
>
> cp = kasprintf(GFP_KERNEL, "%s,%s", *result, new);
> if (!cp)
> return -ENOMEM;

Great, I'll do it.

>
>> + kfree(*result);
>> + *result = cp;
>> + }
>> + return 0;
>> +}
>> +
>> /**
>> * security_module_enable - Load given security module on boot ?
>> * @module: the name of the module
>> @@ -97,6 +115,27 @@ int __init security_module_enable(const char *module)
>> return !strcmp(module, chosen_lsm);
>> }
>>
>> +/**
>> + * security_add_hooks - Add a modules hooks to the hook lists.
>> + * @hooks: the hooks to add
>> + * @count: the number of hooks to add
>> + * @lsm: the name of the security module
>> + *
>> + * Each LSM has to register its hooks with the infrastructure.
>> + */
>> +void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> + char *lsm)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < count; i++) {
>> + hooks[i].lsm = lsm;
>> + list_add_tail_rcu(&hooks[i].list, hooks[i].head);
>> + }
>> + if (lsm_append(lsm, &lsm_names) < 0)
>> + panic("%s - Cannot get early memory.\n", __func__);
>> +}
>> +
>> /*
>> * Hook list operation macros.
>> *
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index a86d537..ed3a757 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6233,7 +6233,7 @@ static __init int selinux_init(void)
>> 0, SLAB_PANIC, NULL);
>> avc_init();
>>
>> - security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks));
>> + security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
>>
>> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>> panic("SELinux: Unable to register AVC netcache callback\n");
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index 6777295..3577009 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -4801,7 +4801,7 @@ static __init int smack_init(void)
>> /*
>> * Register with LSM
>> */
>> - security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks));
>> + security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
>>
>> return 0;
>> }
>> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
>> index 75c9987..edc52d6 100644
>> --- a/security/tomoyo/tomoyo.c
>> +++ b/security/tomoyo/tomoyo.c
>> @@ -542,7 +542,7 @@ static int __init tomoyo_init(void)
>> if (!security_module_enable("tomoyo"))
>> return 0;
>> /* register ourselves with the security framework */
>> - security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks));
>> + security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
>> printk(KERN_INFO "TOMOYO Linux initialized\n");
>> cred->security = &tomoyo_kernel_domain;
>> tomoyo_mm_init();
>> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
>> index 0309f21..f8ee60e 100644
>> --- a/security/yama/yama_lsm.c
>> +++ b/security/yama/yama_lsm.c
>> @@ -471,6 +471,6 @@ static inline void yama_init_sysctl(void) { }
>> void __init yama_add_hooks(void)
>> {
>> pr_info("Yama: becoming mindful.\n");
>> - security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks));
>> + security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
>> yama_init_sysctl();
>> }
>>
> -Kees
>

Next message: Peter Zijlstra: "Re: [PATCH] sched: Do not release current rq lock on non contended double_lock_balance()"
Previous message: Bjorn Andersson: "Re: [PATCH] mfd: qcom_rpm: fix offset error for msm8660"
In reply to: Kees Cook: "Re: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm"
Next in thread: David A. Wheeler: "Re: [PATCH v3 1/3] LSM: Add /sys/kernel/security/lsm"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]