From patchwork Wed Feb 15 14:42:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tetsuo Handa X-Patchwork-Id: 9574233 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8482A601D8 for ; Wed, 15 Feb 2017 14:43:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 76095284F9 for ; Wed, 15 Feb 2017 14:43:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 67BE6284FE; Wed, 15 Feb 2017 14:43:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id CF6F9284F9 for ; Wed, 15 Feb 2017 14:42:59 +0000 (UTC) Received: (qmail 7856 invoked by uid 550); 15 Feb 2017 14:42:57 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 7810 invoked from network); 15 Feb 2017 14:42:53 -0000 X-Virus-Status: clean(F-Secure/fsigk_smtp/530/fsav407.sakura.ne.jp) To: jmorris@namei.org Cc: linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov, kernel-hardening@lists.openwall.com From: Tetsuo Handa References: <201702142324.IFB95862.MOSJLOVFQFtFHO@I-love.SAKURA.ne.jp> In-Reply-To: Message-Id: <201702152342.GBH04183.FOFJFHQOLMOtVS@I-love.SAKURA.ne.jp> X-Mailer: Winbiff [Version 2.51 PL2] X-Accept-Language: ja,en,zh Date: Wed, 15 Feb 2017 23:42:35 +0900 Mime-Version: 1.0 Subject: [kernel-hardening] Re: [RFC v2 PATCH 1/2] security: introduce CONFIG_SECURITY_WRITABLE_HOOKS X-Virus-Scanned: ClamAV using ClamSMTP James Morris wrote: > On Tue, 14 Feb 2017, Tetsuo Handa wrote: > > > > diff --git a/security/Kconfig b/security/Kconfig > > > index 118f454..f6f90c4 100644 > > > --- a/security/Kconfig > > > +++ b/security/Kconfig > > > @@ -31,6 +31,11 @@ config SECURITY > > > > > > If you are unsure how to answer this question, answer N. > > > > > > +config SECURITY_WRITABLE_HOOKS > > > + depends on SECURITY > > > + bool > > > + default n > > > + > > > > This configuration option must not be set to N without big fat explanation > > about implications of setting this option to N. > > It's not visible in the config menu, it's only there to support SELinux > runtime disablement, otherwise it wouldn't even be an option. > > > > > Honestly, I still don't like this option, regardless of whether SELinux > > needs this option or not. > > > > I agree, it would be better to just enable RO hardening without an option > to disable it. Here is my version. printk() is only for testing purpose and will be removed in the final version. (1) Use set_memory_ro() instead of __ro_after_init so that runtime disablement of SELinux and runtime enablement of dynamically loadable LSMs can use set_memory_rw(). (2) Replace "struct security_hook_list"->head with "struct security_hook_list"->offset so that "struct security_hook_heads security_hook_heads;" can become a local variable in security/security.c . (3) (Not in this patch.) The "struct security_hook_list" variable in each LSM module is considered as "__init" and "const" because the content is copied to dynamically allocated (and protected via set_memory_ro()) memory pages. (4) (Not in this patch.) If we add EXPORT_SYMBOL_GPL(security_add_hooks), dynamically loadable LSMs are legally allowed. include/linux/lsm_hooks.h | 31 +++++++--------- security/security.c | 89 ++++++++++++++++++++++++++++++++++++++++++---- security/selinux/hooks.c | 12 +++++- 3 files changed, 107 insertions(+), 25 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index e29d4c6..00050a7 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1865,9 +1865,16 @@ struct security_hook_heads { */ struct security_hook_list { struct list_head list; - struct list_head *head; union security_list_options hook; - char *lsm; + const char *lsm; + unsigned int offset; +}; + +struct lsm_entry { + struct list_head head; + unsigned int order; + unsigned int count; + struct security_hook_list hooks[0]; }; /* @@ -1877,14 +1884,13 @@ struct security_hook_list { * text involved. */ #define LSM_HOOK_INIT(HEAD, HOOK) \ - { .head = &security_hook_heads.HEAD, .hook = { .HEAD = HOOK } } + { .offset = offsetof(struct security_hook_heads, HEAD), \ + .hook = { .HEAD = HOOK } } -extern struct security_hook_heads security_hook_heads; extern char *lsm_names; -extern void security_add_hooks(struct security_hook_list *hooks, int count, - char *lsm); - +extern struct lsm_entry *security_add_hooks(const struct security_hook_list * + hooks, int count, const char *lsm); #ifdef CONFIG_SECURITY_SELINUX_DISABLE /* * Assuring the safety of deleting a security module is up to @@ -1898,15 +1904,8 @@ extern void security_add_hooks(struct security_hook_list *hooks, int count, * disabling their module is a good idea needs to be at least as * careful as the SELinux team. */ -static inline void security_delete_hooks(struct security_hook_list *hooks, - int count) -{ - int i; - - for (i = 0; i < count; i++) - list_del_rcu(&hooks[i].list); -} -#endif /* CONFIG_SECURITY_SELINUX_DISABLE */ +extern void security_delete_hooks(struct lsm_entry *entry); +#endif extern int __init security_module_enable(const char *module); extern void __init capability_add_hooks(void); diff --git a/security/security.c b/security/security.c index d0e07f2..dffe69b 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 +static bool lsm_initialized; char *lsm_names; /* Boot-time LSM user choice */ static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] = @@ -47,6 +48,38 @@ static void __init do_security_initcalls(void) } } +static struct security_hook_heads security_hook_heads; +static LIST_HEAD(lsm_entry_list); +#ifdef CONFIG_DEBUG_RODATA +static inline void mark_security_hooks_ro(void) +{ + struct lsm_entry *tmp; + + list_for_each_entry(tmp, &lsm_entry_list, head) { + printk("Marking LSM hooks at %p read only.\n", tmp); + set_memory_ro((unsigned long) tmp, 1 << tmp->order); + } +} + +static inline void mark_security_hooks_rw(void) +{ + struct lsm_entry *tmp; + + list_for_each_entry(tmp, &lsm_entry_list, head) { + printk("Marking LSM hooks at %p read write.\n", tmp); + set_memory_rw((unsigned long) tmp, 1 << tmp->order); + } +} +#else +static inline void mark_security_hooks_ro(void) +{ +} + +static inline void mark_security_hooks_rw(void) +{ +} +#endif + /** * security_init - initializes the security framework * @@ -68,6 +101,8 @@ int __init security_init(void) */ do_security_initcalls(); + lsm_initialized = true; + mark_security_hooks_ro(); return 0; } @@ -79,7 +114,7 @@ static int __init choose_lsm(char *str) } __setup("security=", choose_lsm); -static int lsm_append(char *new, char **result) +static int lsm_append(const char *new, char **result) { char *cp; @@ -122,18 +157,58 @@ int __init security_module_enable(const char *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) +struct lsm_entry *security_add_hooks(const struct security_hook_list *hooks, + int count, const char *lsm) { int i; + /* + * entry has to be an PAGE_ALIGNED multiple of PAGE_SIZE memory + * in order to pass to set_memory_ro()/set_memory_rw(). + */ + const size_t size = sizeof(*hooks) * count + sizeof(struct lsm_entry); + const unsigned int order = get_order(size); + struct lsm_entry *entry = kmalloc_order(size, GFP_KERNEL | __GFP_ZERO, + order); + if (!entry || !PAGE_ALIGNED(entry) || lsm_append(lsm, &lsm_names) < 0) { + kfree(entry); + goto out; + } + if (lsm_initialized) + mark_security_hooks_rw(); + printk("Allocating LSM hooks for %s at %p\n", lsm, entry); + entry->order = order; + entry->count = count; + memcpy(&entry->hooks[0], hooks, size - sizeof(struct lsm_entry)); for (i = 0; i < count; i++) { - hooks[i].lsm = lsm; - list_add_tail_rcu(&hooks[i].list, hooks[i].head); + entry->hooks[i].lsm = lsm; + list_add_tail_rcu(&entry->hooks[i].list, (struct list_head *) + (((char *) &security_hook_heads) + + entry->hooks[i].offset)); } - if (lsm_append(lsm, &lsm_names) < 0) + list_add_tail(&entry->head, &lsm_entry_list); + if (lsm_initialized) + mark_security_hooks_ro(); + return entry; + out: + if (!lsm_initialized) panic("%s - Cannot get early memory.\n", __func__); + return NULL; +} + +#ifdef CONFIG_SECURITY_SELINUX_DISABLE +void security_delete_hooks(struct lsm_entry *entry) +{ + int i; + + mark_security_hooks_rw(); + list_del_rcu(&entry->head); + for (i = 0; i < entry->count; i++) + list_del_rcu(&entry->hooks[i].list); + /* Not calling kfree() in order to avoid races. */ + mark_security_hooks_ro(); } +#endif /* CONFIG_SECURITY_SELINUX_DISABLE */ /* * Hook list operation macros. @@ -1622,7 +1697,7 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule, } #endif /* CONFIG_AUDIT */ -struct security_hook_heads security_hook_heads = { +static struct security_hook_heads security_hook_heads = { .binder_set_context_mgr = LIST_HEAD_INIT(security_hook_heads.binder_set_context_mgr), .binder_transaction = diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9bc12bc..4668590 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6319,6 +6319,10 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) #endif }; +#ifdef CONFIG_SECURITY_SELINUX_DISABLE +static struct lsm_entry *selinux_entry; +#endif + static __init int selinux_init(void) { if (!security_module_enable("selinux")) { @@ -6346,7 +6350,11 @@ static __init int selinux_init(void) 0, SLAB_PANIC, NULL); avc_init(); - security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux"); +#ifdef CONFIG_SECURITY_SELINUX_DISABLE + selinux_entry = +#endif + 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"); @@ -6475,7 +6483,7 @@ int selinux_disable(void) selinux_disabled = 1; selinux_enabled = 0; - security_delete_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks)); + security_delete_hooks(selinux_entry); /* Try to destroy the avc node cache */ avc_disable();