Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754541AbdLHQbx (ORCPT ); Fri, 8 Dec 2017 11:31:53 -0500 Received: from sonic312-28.consmr.mail.ne1.yahoo.com ([66.163.191.209]:34580 "EHLO sonic312-28.consmr.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753313AbdLHQbu (ORCPT ); Fri, 8 Dec 2017 11:31:50 -0500 X-YMail-OSG: DIdpK.IVM1mOMmr3Jj1XrFkI_qw8ppIoQ3EvU9ZohzZnNbIDATIHYvWdHdzQDE4 BUUJHpSJ8rO.50Vs6rO0O10h7aBUi_ZViz1fs_TrD4xiZ9A2_miLf4WL9uBMa4oaQEvIwnLOS_ie kvLzGzqoIIOdkejwY3wV7SyVjfYP_ZaaGukIxV2zwciyHOXowvl91IL2sE9ekfKVeWJRHzsueegG EZDQ1ZMqGygqFLYFeQbXLISNVIm0sEQiDs60lqYntsErzbgU7vaTjG5gEafP.wDIbG9Fff4SRCNP pBR4vlry0KICFZ75YT7IBJ1bfCPb1Vc8nlJg2qP61MhEF1FCJznlWEiCQZS5X7y3lCTR7OF7Ivpe sy3Rl0DKFaENub1P6jDxsA5dt7FIF.YNS8PPkQc5uK7UnPtWzxN2YskyjhlSLJGbHtxLPfdCEXco A0JvN3_OLsv4FtPl6ADjKf1yPCjCxyzQMqnlaoe3vQq6pvHvJ.iOVamsigIurjlzBNb8RlmTMJwg cCS56cPYVIsHD2GqAfvN3pSjE4StQE1WldJMYlaGogGA- Subject: Re: [RFC v2 2/3] LSM: Add statistics about the invocation of dynamic hooks To: Sargun Dhillon , linux-security-module@vger.kernel.org Cc: keescook@chromium.org, igor.stoppa@huawei.com, linux-kernel@vger.kernel.org References: <0d030add49ec1dfd2971e955ab7856cc536e37b1.1512704909.git.sargun@netflix.com> From: Casey Schaufler Message-ID: Date: Fri, 8 Dec 2017 08:31:46 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <0d030add49ec1dfd2971e955ab7856cc536e37b1.1512704909.git.sargun@netflix.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11999 Lines: 364 On 12/7/2017 8:24 PM, Sargun Dhillon wrote: > This patch builds on the dynamic hooks patch. With dynamic hooks, > /sys/kernel/security/lsm doesn't really make a lot of sense, because > the administrator is more likely interested in the per-hook modules. User space software (e.g. systemd) still needs a way to get the current set of modules. Looking by hook does not make sense for a program that just wants to know the high level view of what the system is running. Granularity is not always a good thing. > There is now a /sys/kernel/security/dynamic_hooks/${HOOK_NAME} which > has the currently attached LSMs for that given hook. > > Not only does it list which LSMs are hooked into each dynamic hook, > but it also lists the global accept / deny count, as well as the > per-hook accept / deny count. The earlier is useful to keep consistent > statistics across the attachment, and unattachment of LSMs. > > The purpose of this is similar to the purpose of something like > iptables -L -n. With the proliferation of LSMs, it's going to > be more important to have a way to understand what's going on. > > Signed-off-by: Sargun Dhillon > --- > include/linux/lsm_hooks.h | 4 ++ > security/Kconfig | 7 +++ > security/Makefile | 1 + > security/dynamic.c | 26 ++++++++++- > security/dynamic.h | 14 ++++++ > security/dynamicfs.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++ > security/inode.c | 2 + > security/security.c | 21 +++++++-- > 8 files changed, 179 insertions(+), 5 deletions(-) > create mode 100644 security/dynamicfs.c > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index 9c44300fc1f8..0aa9f7616f77 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -29,6 +29,8 @@ > #include > #include > #include > +#include > +#include > > /** > * union security_list_options - Linux Security Module hook function list > @@ -2212,6 +2214,8 @@ enum dynamic_security_hook_type { > }; > > struct dynamic_security_hook { > + struct percpu_counter invocation; > + struct percpu_counter deny; > struct list_head list; > union security_list_options hook; > enum dynamic_security_hook_type type; > diff --git a/security/Kconfig b/security/Kconfig > index 77841bdb5bc5..d6c579d6099f 100644 > --- a/security/Kconfig > +++ b/security/Kconfig > @@ -94,6 +94,13 @@ config SECURITY_DYNAMIC_HOOKS > They cannot circumvent the built-in LSMs. > If you are unsure how to answer this question, answer N. > > +config SECURITY_DYNAMIC_HOOKS_FS > + bool > + default y if SECURITY_DYNAMIC_HOOKS && SECURITYFS > + depends on SECURITY_DYNAMIC_HOOKS && SECURITYFS > + help > + This option enables listing of dynamically loaded LSM hooks. > + > config INTEL_TXT > bool "Enable Intel(R) Trusted Execution Technology (Intel(R) TXT)" > depends on HAVE_INTEL_TXT > diff --git a/security/Makefile b/security/Makefile > index 59e695a7e4b6..51cbec0fa49e 100644 > --- a/security/Makefile > +++ b/security/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_MMU) += min_addr.o > obj-$(CONFIG_SECURITY) += security.o > obj-$(CONFIG_SECURITYFS) += inode.o > obj-$(CONFIG_SECURITY_DYNAMIC_HOOKS) += dynamic.o > +obj-$(CONFIG_SECURITY_DYNAMIC_HOOKS_FS) += dynamicfs.o > obj-$(CONFIG_SECURITY_SELINUX) += selinux/ > obj-$(CONFIG_SECURITY_SMACK) += smack/ > obj-$(CONFIG_AUDIT) += lsm_audit.o > diff --git a/security/dynamic.c b/security/dynamic.c > index cc2f5d232e9a..d96a50a93bad 100644 > --- a/security/dynamic.c > +++ b/security/dynamic.c > @@ -250,7 +250,17 @@ struct dynamic_hook dynamic_hooks[__MAX_DYNAMIC_SECURITY_HOOK] = { > */ > int security_add_dynamic_hook(struct dynamic_security_hook *hook) > { > + int ret; > + > WARN_ON(!try_module_get(hook->owner)); > + ret = percpu_counter_init(&hook->invocation, 0, GFP_KERNEL); > + if (ret) > + return ret; > + ret = percpu_counter_init(&hook->deny, 0, GFP_KERNEL); > + if (ret) { > + percpu_counter_destroy(&hook->invocation); > + return ret; > + } > mutex_lock(&dynamic_hook_lock); > list_add_tail_rcu(&hook->list, &dynamic_hooks[hook->type].head); > mutex_unlock(&dynamic_hook_lock); > @@ -262,8 +272,20 @@ EXPORT_SYMBOL_GPL(security_add_dynamic_hook); > > void __init security_init_dynamic_hooks(void) > { > - int i; > + int i, ret; > > - for (i = 0; i < ARRAY_SIZE(dynamic_hooks); i++) > + for (i = 0; i < ARRAY_SIZE(dynamic_hooks); i++) { > INIT_LIST_HEAD(&dynamic_hooks[i].head); > + ret = percpu_counter_init(&dynamic_hooks[i].invocation, 0, > + GFP_KERNEL); > + if (ret) > + panic("%s - %d - Cannot init invocation counter.\n", > + __func__, ret); > + ret = percpu_counter_init(&dynamic_hooks[i].deny, 0, > + GFP_KERNEL); > + if (ret) > + panic("%s - %d - Cannot init deny counter.\n", > + __func__, ret); > + > + } > } > diff --git a/security/dynamic.h b/security/dynamic.h > index 575bc4e3d370..402cfc3f3ea3 100644 > --- a/security/dynamic.h > +++ b/security/dynamic.h > @@ -2,12 +2,20 @@ > #include > #include > #include > +#include > +#include > +#include > > #ifdef CONFIG_SECURITY_DYNAMIC_HOOKS > extern struct static_key_false dynamic_hooks_keys[]; > > struct dynamic_hook { > + struct percpu_counter invocation; > + struct percpu_counter deny; > const char *name; > +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS_FS > + struct dentry *dentry; > +#endif > struct list_head head; > }; > > @@ -16,3 +24,9 @@ extern void security_init_dynamic_hooks(void); > #else > static void security_init_dynamic_hooks(void) {} > #endif > + > +#ifdef CONFIG_SECURITY_DYNAMIC_HOOKS_FS > +extern void securityfs_init_dynamic_hooks(void); > +#else > +static void securityfs_init_dynamic_hooks(void) {} > +#endif > diff --git a/security/dynamicfs.c b/security/dynamicfs.c > new file mode 100644 > index 000000000000..d7079f0acd1c > --- /dev/null > +++ b/security/dynamicfs.c > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "dynamic.h" > + > +struct seq_private_data { > + struct dynamic_hook *dh; > +}; > + > +static void *dynamic_hooks_sop_start(struct seq_file *s, loff_t *pos) > +{ > + struct seq_private_data *pd = s->private; > + > + return seq_list_start_head(&pd->dh->head, *pos); > +} > + > +static void *dynamic_hooks_sop_next(struct seq_file *s, void *v, loff_t *pos) > +{ > + struct seq_private_data *pd = s->private; > + > + return seq_list_next(v, &pd->dh->head, pos); > +} > + > +static int dynamic_hooks_sop_show(struct seq_file *s, void *v) > +{ > + struct seq_private_data *pd = s->private; > + struct dynamic_security_hook *dsh; > + > + if (v == (void *)&pd->dh->head) { > + seq_puts(s, "name\tinvocations\tdenies\n"); > + seq_printf(s, "all\t%lld\t%lld\n", > + percpu_counter_sum(&pd->dh->invocation), > + percpu_counter_sum(&pd->dh->deny)); > + return 0; > + } > + > + dsh = list_entry(v, typeof(*dsh), list); > + seq_printf(s, "%s\t%lld\t%lld\n", dsh->lsm, > + percpu_counter_sum(&dsh->invocation), > + percpu_counter_sum(&dsh->deny)); > + > + return 0; > +} > + > +static void dynamic_hooks_sop_stop(struct seq_file *s, void *v) { } > + > +static const struct seq_operations dynamic_hooks_sops = { > + .start = dynamic_hooks_sop_start, > + .next = dynamic_hooks_sop_next, > + .show = dynamic_hooks_sop_show, > + .stop = dynamic_hooks_sop_stop, > +}; > + > +static int security_dynamic_hook_open(struct inode *inode, struct file *file) > +{ > + struct seq_private_data *pd; > + > + pd = (struct seq_private_data *)__seq_open_private(file, > + &dynamic_hooks_sops, > + sizeof(*pd)); > + > + if (!pd) > + return -ENOMEM; > + > + pd->dh = inode->i_private; > + > + return 0; > +} > + > +static const struct file_operations dynamic_hooks_fops = { > + .open = security_dynamic_hook_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = seq_release, > +}; > + > +static struct dentry *dynamic_hooks_dir; > +void securityfs_init_dynamic_hooks(void) > +{ > + struct dynamic_hook *dh; > + int i; > + > + dynamic_hooks_dir = securityfs_create_dir("dynamic_hooks", NULL); > + if (IS_ERR(dynamic_hooks_dir)) { > + pr_err("Unable to create dynamic hooks LSM directory - %ld\n", > + PTR_ERR(dynamic_hooks_dir)); > + return; > + } > + > + for (i = 0; i < __MAX_DYNAMIC_SECURITY_HOOK; i++) { > + > + dh = &dynamic_hooks[i]; > + dh->dentry = securityfs_create_file(dh->name, 0444, > + dynamic_hooks_dir, dh, > + &dynamic_hooks_fops); > + if (IS_ERR(dh->dentry)) > + goto err; > + } > + return; > + > +err: > + pr_err("Unable to create dynamic hook directory - %s - %ld\n", > + dh->name, PTR_ERR(dh->dentry)); > +} > diff --git a/security/inode.c b/security/inode.c > index 8dd9ca8848e4..0862be12a340 100644 > --- a/security/inode.c > +++ b/security/inode.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include "dynamic.h" > > static struct vfsmount *mount; > static int mount_count; > @@ -335,6 +336,7 @@ static int __init securityfs_init(void) > sysfs_remove_mount_point(kernel_kobj, "security"); > return retval; > } > + securityfs_init_dynamic_hooks(); > #ifdef CONFIG_SECURITY > lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, > &lsm_ops); > diff --git a/security/security.c b/security/security.c > index 278f46ac8fc3..efb9bdd42ece 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -213,8 +213,11 @@ static_branch_unlikely(&dynamic_hooks_keys[DYNAMIC_SECURITY_HOOK_##FUNC]) > struct dynamic_security_hook *dsh; \ > struct dynamic_hook *dh; \ > dh = &dynamic_hooks[DYNAMIC_SECURITY_HOOK_##FUNC]; \ > - list_for_each_entry_rcu(dsh, &dh->head, list) \ > + percpu_counter_inc(&dh->invocation); \ > + list_for_each_entry_rcu(dsh, &dh->head, list) { \ > + percpu_counter_inc(&dsh->invocation); \ > dsh->hook.FUNC(__VA_ARGS__); \ > + } \ > }) > > #define call_void_hook(FUNC, ...) \ > @@ -244,10 +247,15 @@ static_branch_unlikely(&dynamic_hooks_keys[DYNAMIC_SECURITY_HOOK_##FUNC]) > if (!continue_iteration) \ > break; \ > dh = &dynamic_hooks[DYNAMIC_SECURITY_HOOK_##FUNC]; \ > + percpu_counter_inc(&dh->invocation); \ > list_for_each_entry(dsh, &dh->head, list) { \ > + percpu_counter_inc(&dsh->invocation); \ > RC = dsh->hook.FUNC(__VA_ARGS__); \ > - if (RC != 0) \ > + if (RC != 0) { \ > + percpu_counter_inc(&dh->deny); \ > + percpu_counter_inc(&dsh->deny); \ > break; \ > + } \ > } \ > } while (0); \ > RC; \ > @@ -368,10 +376,15 @@ static int dynamic_vm_enough_memory_mm(struct mm_struct *mm, long pages) > return 1; > > dh = &dynamic_hooks[DYNAMIC_SECURITY_HOOK_vm_enough_memory]; > + percpu_counter_inc(&dh->invocation); > list_for_each_entry(dsh, &dh->head, list) { > + percpu_counter_inc(&dsh->invocation); > rc = dsh->hook.vm_enough_memory(mm, pages); > - if (rc <= 0) > + if (rc <= 0) { > + percpu_counter_inc(&dh->deny); > + percpu_counter_inc(&dsh->deny); > break; > + } > } > return rc; > } > @@ -1215,7 +1228,9 @@ static int dynamic_task_prctl(int option, unsigned long arg2, > goto out; > > dh = &dynamic_hooks[DYNAMIC_SECURITY_HOOK_task_prctl]; > + percpu_counter_inc(&dh->invocation); > list_for_each_entry(dsh, &dh->head, list) { > + percpu_counter_inc(&dsh->invocation); > thisrc = dsh->hook.task_prctl(option, arg2, arg3, arg4, arg5); > if (thisrc != -ENOSYS) { > rc = thisrc;