linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* out of tree lsm's
@ 2017-03-20 18:54 Peter Moody
  2017-03-20 19:30 ` Paul Moore
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Moody @ 2017-03-20 18:54 UTC (permalink / raw)
  To: linux-security-module

with the success of stackable lsm's, it occurs to me that
site-specific, out-of-tree modules could be extremely worthwhile.

I realize that it doesn't make a lot of sense to have something that I
can insmod/rmmod well post-boot, but being able to at least stuff an
lsm in an initrd that's loaded during boot could be very helpful.

Without having any code to pick apart just now, is the idea of this
functionality amenable to folks?

Cheers,
peter
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-20 18:54 out of tree lsm's Peter Moody
@ 2017-03-20 19:30 ` Paul Moore
  2017-03-20 19:45   ` Peter Moody
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Moore @ 2017-03-20 19:30 UTC (permalink / raw)
  To: linux-security-module

On Mon, Mar 20, 2017 at 2:54 PM, Peter Moody <peter.moody@gmail.com> wrote:
> with the success of stackable lsm's, it occurs to me that
> site-specific, out-of-tree modules could be extremely worthwhile.

Keep in mind we don't have a general purpose solution ... yet.  Casey
continues to work on it, and I'm sure he'll have something at some
point, but right now you are limited to a single "big" LSMs (e.g.
SELinux) and some combination of "small" LSMs (e.g. Yama).

> I realize that it doesn't make a lot of sense to have something that I
> can insmod/rmmod well post-boot, but being able to at least stuff an
> lsm in an initrd that's loaded during boot could be very helpful.
>
> Without having any code to pick apart just now, is the idea of this
> functionality amenable to folks?

I think the usual comments about out-of-tree modules apply here;
you're free to do what you like, but upstream is only going to offer
limited help/support if/until the code starts its way upstream.

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-20 19:30 ` Paul Moore
@ 2017-03-20 19:45   ` Peter Moody
  2017-03-20 20:17     ` Casey Schaufler
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Moody @ 2017-03-20 19:45 UTC (permalink / raw)
  To: linux-security-module

On Mon, Mar 20, 2017 at 12:30 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Mon, Mar 20, 2017 at 2:54 PM, Peter Moody <peter.moody@gmail.com> wrote:
>> with the success of stackable lsm's, it occurs to me that
>> site-specific, out-of-tree modules could be extremely worthwhile.
>
> Keep in mind we don't have a general purpose solution ... yet.  Casey
> continues to work on it, and I'm sure he'll have something at some
> point, but right now you are limited to a single "big" LSMs (e.g.
> SELinux) and some combination of "small" LSMs (e.g. Yama).

right. sorry for the imprecise language; by site-specific I meant a "small" lsm.

I would love to have the ability write a small lsm that I can build as
a module and load at boot eg. via initrd.

AIUI, adding even a new "small" lsm requires kconfig patches, building
a new kernel, etc. I know there are objections to dynamically loadable
lsms and I was trying to find a compromise that made them easier to
work with.

Cheers,
peter

>> I realize that it doesn't make a lot of sense to have something that I
>> can insmod/rmmod well post-boot, but being able to at least stuff an
>> lsm in an initrd that's loaded during boot could be very helpful.
>>
>> Without having any code to pick apart just now, is the idea of this
>> functionality amenable to folks?
>
> I think the usual comments about out-of-tree modules apply here;
> you're free to do what you like, but upstream is only going to offer
> limited help/support if/until the code starts its way upstream.
>
> --
> paul moore
> www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-20 19:45   ` Peter Moody
@ 2017-03-20 20:17     ` Casey Schaufler
  2017-03-20 22:18       ` Tetsuo Handa
  0 siblings, 1 reply; 12+ messages in thread
From: Casey Schaufler @ 2017-03-20 20:17 UTC (permalink / raw)
  To: linux-security-module

On 3/20/2017 12:45 PM, Peter Moody wrote:
> On Mon, Mar 20, 2017 at 12:30 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Mon, Mar 20, 2017 at 2:54 PM, Peter Moody <peter.moody@gmail.com> wrote:
>>> with the success of stackable lsm's, it occurs to me that
>>> site-specific, out-of-tree modules could be extremely worthwhile.
>> Keep in mind we don't have a general purpose solution ... yet.  Casey
>> continues to work on it, and I'm sure he'll have something at some
>> point, but right now you are limited to a single "big" LSMs (e.g.
>> SELinux) and some combination of "small" LSMs (e.g. Yama).

Yes, work is in progress.

> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
>
> I would love to have the ability write a small lsm that I can build as
> a module and load at boot eg. via initrd.
>
> AIUI, adding even a new "small" lsm requires kconfig patches, building
> a new kernel, etc. I know there are objections to dynamically loadable
> lsms and I was trying to find a compromise that made them easier to
> work with.

The stacking design criteria I'm working with
include not doing anything that would prevent
dynamic module loading. I do not plan to implement
dynamic loading. Tetsuo has been a strong
advocate of loadable modules. I would expect to
see a proposal from him shortly after the
general stacking lands, assuming it does.

>
> Cheers,
> peter
>
>>> I realize that it doesn't make a lot of sense to have something that I
>>> can insmod/rmmod well post-boot, but being able to at least stuff an
>>> lsm in an initrd that's loaded during boot could be very helpful.
>>>
>>> Without having any code to pick apart just now, is the idea of this
>>> functionality amenable to folks?
>> I think the usual comments about out-of-tree modules apply here;
>> you're free to do what you like, but upstream is only going to offer
>> limited help/support if/until the code starts its way upstream.
>>
>> --
>> paul moore
>> www.paul-moore.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-20 20:17     ` Casey Schaufler
@ 2017-03-20 22:18       ` Tetsuo Handa
  2017-03-21 10:41         ` Tetsuo Handa
  0 siblings, 1 reply; 12+ messages in thread
From: Tetsuo Handa @ 2017-03-20 22:18 UTC (permalink / raw)
  To: linux-security-module

Casey Schaufler wrote:
> > right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
> >
> > I would love to have the ability write a small lsm that I can build as
> > a module and load at boot eg. via initrd.
> >
> > AIUI, adding even a new "small" lsm requires kconfig patches, building
> > a new kernel, etc. I know there are objections to dynamically loadable
> > lsms and I was trying to find a compromise that made them easier to
> > work with.
> 
> The stacking design criteria I'm working with
> include not doing anything that would prevent
> dynamic module loading. I do not plan to implement
> dynamic loading. Tetsuo has been a strong
> advocate of loadable modules. I would expect to
> see a proposal from him shortly after the
> general stacking lands, assuming it does.

But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
dynamic modules from loading. We need a legitimate interface for loadable modules like
http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-20 22:18       ` Tetsuo Handa
@ 2017-03-21 10:41         ` Tetsuo Handa
  2017-03-21 15:36           ` Casey Schaufler
  0 siblings, 1 reply; 12+ messages in thread
From: Tetsuo Handa @ 2017-03-21 10:41 UTC (permalink / raw)
  To: linux-security-module

Tetsuo Handa wrote:
> Casey Schaufler wrote:
> > > right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
> > >
> > > I would love to have the ability write a small lsm that I can build as
> > > a module and load at boot eg. via initrd.
> > >
> > > AIUI, adding even a new "small" lsm requires kconfig patches, building
> > > a new kernel, etc. I know there are objections to dynamically loadable
> > > lsms and I was trying to find a compromise that made them easier to
> > > work with.
> > 
> > The stacking design criteria I'm working with
> > include not doing anything that would prevent
> > dynamic module loading. I do not plan to implement
> > dynamic loading. Tetsuo has been a strong
> > advocate of loadable modules. I would expect to
> > see a proposal from him shortly after the
> > general stacking lands, assuming it does.
> 
> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
> dynamic modules from loading. We need a legitimate interface for loadable modules like
> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
> 

I think we need something like below change when allowing loadable modules.

>From b81fbc7c9c052c92f842777428bee2d5942ce9a9 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 21 Mar 2017 14:37:52 +0900
Subject: [PATCH] security: Make dynamic module registration legitimate.

While we are waiting for stacking support of in-tree built-in LSM modules
(and therefore I am refraining from proposing changes for supporting
dynamically loadable LSM modules), commit dd0859dccbe291cf ("security:
introduce CONFIG_SECURITY_WRITABLE_HOOKS") and commit ca97d939db114c8d
("security: mark LSM hooks as __ro_after_init") came in. But these two
commits are preventing us from making it possible to legitimately support
dynamically loadable LSM modules due to lack of architecture-independent
infrastructure for temporarily making LSM hooks writable.

This patch demonstrates how we can make it possible to legitimately support
dynamically loadable LSM modules by using set_memory_ro()/set_memory_nx()/
set_memory_rw() infrastructure rather than __ro_after_init marking.

Below is an example output for registration and runtime disablement of
SELinux by this patch. pr_info() in this patch is only for testing purpose
and will be removed in the final version.

----------
[    0.014039] Security Framework initialized
[    0.015006] Allocating LSM hooks for capability at ffff88013a10a000
[    0.016003] Yama: becoming mindful.
[    0.017008] Allocating LSM hooks for yama at ffff88013a10b000
[    0.018006] LoadPin: ready to pin (currently disabled)
[    0.018009] Allocating LSM hooks for loadpin at ffff88013a10c000
[    0.020004] SELinux:  Initializing.
[    0.021016] Allocating LSM hooks for selinux at ffff88013a114000
[    0.022005] AppArmor: AppArmor disabled by boot time parameter
[    0.023002] Marking LSM hook heads at ffffffff81853000 read only and non execute.
[    0.024025] Marking LSM hooks for capability at ffff88013a10a000 read only and non execute.
[    0.025730] Marking LSM hooks for yama at ffff88013a10b000 read only and non execute.
[    0.026007] Marking LSM hooks for loadpin at ffff88013a10c000 read only and non execute.
[    0.027007] Marking LSM hooks for selinux at ffff88013a114000 read only and non execute.
[    0.028140] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes)
(...snipped...)
[    5.069577] SELinux:  Disabled at runtime.
[    5.076575] Marking LSM hooks for capability at ffff88013a10a000 read write.
[    5.087069] Marking LSM hooks for yama at ffff88013a10b000 read write.
[    5.096963] Marking LSM hooks for loadpin at ffff88013a10c000 read write.
[    5.104957] Marking LSM hooks for selinux at ffff88013a114000 read write.
[    5.109179] Marking LSM hook heads at ffffffff81853000 read write.
[    5.112881] Marking LSM hook heads at ffffffff81853000 read only and non execute.
[    5.117276] Marking LSM hooks for capability at ffff88013a10a000 read only and non execute.
[    5.122347] Marking LSM hooks for yama at ffff88013a10b000 read only and non execute.
[    5.126936] Marking LSM hooks for loadpin at ffff88013a10c000 read only and non execute.
[    5.149116] audit: type=1404 audit(1490064154.351:2): selinux=0 auid=4294967295 ses=4294967295
----------

This patch mainly does the following things.

  (1) Use set_memory_ro()/set_memory_nx() instead of __ro_after_init
      so that runtime disablement of SELinux and runtime enablement of
      dynamically loadable LSMs can use set_memory_rw().

      While x86 architecture provides lookup_address() for finding
      "struct page" from virtual address which allows us to temporarily
      make specific pages writable, other architectures which support
      CONFIG_STRICT_KERNEL_RWX feature which __ro_after_init depends on
      (e.g. arm, parisc, s390) do not provide it.
      But set_memory_ro()/set_memory_nx()/set_memory_rw() are already
      available in x86, arm and s390. I don't know whether parisc can
      implement set_memory_xx() but I assume it is possible. Therefore,
      I assume that set_memory_xx() will support wider environments than
      __ro_after_init while allowing runtime disablement of SELinux and
      runtime enablement of dynamically loadable LSMs until we get
      architecture-independent infrastructure for temporarily making LSM
      hooks writable.

  (2) Make "struct security_hook_list" variable in each LSM module
      read-only by copying the content of that variable to dynamically
      allocated (and protected via set_memory_ro()/set_memory_nx())
      memory pages upon registration.

  (3) Add EXPORT_SYMBOL_GPL(security_add_hooks) in order to legitimately
      support dynamically loadable LSMs.

  (4) 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 .

This patch applies on top of reverting the two commits mentioned above.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 include/linux/lsm_hooks.h  |  34 +++++++-------
 security/apparmor/lsm.c    |   2 +-
 security/commoncap.c       |   2 +-
 security/loadpin/loadpin.c |   2 +-
 security/security.c        | 112 ++++++++++++++++++++++++++++++++++++++++++---
 security/selinux/hooks.c   |  14 ++++--
 security/smack/smack_lsm.c |   2 +-
 security/tomoyo/tomoyo.c   |   2 +-
 security/yama/yama_lsm.c   |   2 +-
 9 files changed, 139 insertions(+), 33 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index e29d4c6..668157f 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1857,7 +1857,7 @@ struct security_hook_heads {
 	struct list_head audit_rule_match;
 	struct list_head audit_rule_free;
 #endif /* CONFIG_AUDIT */
-};
+} __aligned(PAGE_SIZE);
 
 /*
  * Security module hook list structure.
@@ -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,14 @@ 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, const unsigned int count,
+					    const char *lsm);
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 /*
  * Assuring the safety of deleting a security module is up to
@@ -1898,15 +1905,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/apparmor/lsm.c b/security/apparmor/lsm.c
index 709eacd..04cf323 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -587,7 +587,7 @@ static int apparmor_task_setrlimit(struct task_struct *task,
 	return error;
 }
 
-static struct security_hook_list apparmor_hooks[] = {
+static const struct security_hook_list apparmor_hooks[] __initconst = {
 	LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
 	LSM_HOOK_INIT(capget, apparmor_capget),
diff --git a/security/commoncap.c b/security/commoncap.c
index 78b3783..c456805 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1071,7 +1071,7 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
 
 #ifdef CONFIG_SECURITY
 
-struct security_hook_list capability_hooks[] = {
+static const struct security_hook_list capability_hooks[] __initconst = {
 	LSM_HOOK_INIT(capable, cap_capable),
 	LSM_HOOK_INIT(settime, cap_settime),
 	LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 1d82eae..b88154a 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -174,7 +174,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 	return 0;
 }
 
-static struct security_hook_list loadpin_hooks[] = {
+static const struct security_hook_list loadpin_hooks[] __initconst = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
 	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
 };
diff --git a/security/security.c b/security/security.c
index d0e07f2..9cb2b3e 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 __ro_after_init;
 char *lsm_names;
 /* Boot-time LSM user choice */
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -47,6 +48,59 @@ static void __init do_security_initcalls(void)
 	}
 }
 
+static struct security_hook_heads security_hook_heads __aligned(PAGE_SIZE);
+static LIST_HEAD(lsm_entry_list);
+#if defined(CONFIG_STRICT_KERNEL_RWX) && !defined(CONFIG_PARISC)
+static inline void mark_security_hooks_ro(void)
+{
+	struct lsm_entry *tmp;
+	void *addr;
+
+	BUILD_BUG_ON(sizeof(security_hook_heads) % PAGE_SIZE);
+	BUILD_BUG_ON(!PAGE_ALIGNED(&security_hook_heads));
+	pr_info("Marking LSM hook heads@%p read only and non execute.\n",
+		&security_hook_heads);
+	addr = page_address(virt_to_page(&security_hook_heads));
+	set_memory_ro((unsigned long) addr,
+		      sizeof(security_hook_heads) >> PAGE_SHIFT);
+	set_memory_nx((unsigned long) addr,
+		      sizeof(security_hook_heads) >> PAGE_SHIFT);
+	list_for_each_entry(tmp, &lsm_entry_list, head) {
+		pr_info("Marking LSM hooks for %s at %p read only and non execute.\n",
+			tmp->hooks[0].lsm, tmp);
+		addr = page_address(virt_to_head_page(tmp));
+		set_memory_ro((unsigned long) addr, 1 << tmp->order);
+		set_memory_nx((unsigned long) addr, 1 << tmp->order);
+	}
+}
+
+static inline void mark_security_hooks_rw(void)
+{
+	struct lsm_entry *tmp;
+	void *addr;
+
+	list_for_each_entry(tmp, &lsm_entry_list, head) {
+		pr_info("Marking LSM hooks for %s at %p read write.\n",
+			tmp->hooks[0].lsm, tmp);
+		addr = page_address(virt_to_head_page(tmp));
+		set_memory_rw((unsigned long) addr, 1 << tmp->order);
+	}
+	pr_info("Marking LSM hook heads at %p read write.\n",
+		&security_hook_heads);
+	addr = page_address(virt_to_page(&security_hook_heads));
+	set_memory_rw((unsigned long) addr,
+		      sizeof(security_hook_heads) >> PAGE_SHIFT);
+}
+#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 +122,8 @@ int __init security_init(void)
 	 */
 	do_security_initcalls();
 
+	lsm_initialized = true;
+	mark_security_hooks_ro();
 	return 0;
 }
 
@@ -79,7 +135,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 +178,60 @@ 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,
+				     const unsigned 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_nx()/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();
+	pr_info("Allocating LSM hooks for %s@%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;
+}
+/* Enable this when we start supporting loadable LSM modules.*/
+EXPORT_SYMBOL_GPL(security_add_hooks);
+
+#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 +1720,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 0c2ac31..6fa68d9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6108,7 +6108,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
 
 #endif
 
-static struct security_hook_list selinux_hooks[] = {
+static const struct security_hook_list selinux_hooks[] __initconst = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
 	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),
@@ -6323,6 +6323,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")) {
@@ -6350,7 +6354,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");
@@ -6479,7 +6487,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();
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fc8fb31..fbc86a3 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4633,7 +4633,7 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 	return 0;
 }
 
-static struct security_hook_list smack_hooks[] = {
+static const struct security_hook_list smack_hooks[] __initconst = {
 	LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
 	LSM_HOOK_INIT(syslog, smack_syslog),
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index edc52d6..74ef461 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -496,7 +496,7 @@ static int tomoyo_socket_sendmsg(struct socket *sock, struct msghdr *msg,
  * tomoyo_security_ops is a "struct security_operations" which is used for
  * registering TOMOYO.
  */
-static struct security_hook_list tomoyo_hooks[] = {
+static const struct security_hook_list tomoyo_hooks[] __initconst = {
 	LSM_HOOK_INIT(cred_alloc_blank, tomoyo_cred_alloc_blank),
 	LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, tomoyo_cred_transfer),
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 88271a3..6e1cb40 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -428,7 +428,7 @@ int yama_ptrace_traceme(struct task_struct *parent)
 	return rc;
 }
 
-static struct security_hook_list yama_hooks[] = {
+static const struct security_hook_list yama_hooks[] __initconst = {
 	LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
 	LSM_HOOK_INIT(task_prctl, yama_task_prctl),
-- 
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 10:41         ` Tetsuo Handa
@ 2017-03-21 15:36           ` Casey Schaufler
  2017-03-21 16:06             ` Peter Moody
  0 siblings, 1 reply; 12+ messages in thread
From: Casey Schaufler @ 2017-03-21 15:36 UTC (permalink / raw)
  To: linux-security-module

On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
> Tetsuo Handa wrote:
>> Casey Schaufler wrote:
>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
>>>>
>>>> I would love to have the ability write a small lsm that I can build as
>>>> a module and load at boot eg. via initrd.
>>>>
>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
>>>> a new kernel, etc. I know there are objections to dynamically loadable
>>>> lsms and I was trying to find a compromise that made them easier to
>>>> work with.
>>> The stacking design criteria I'm working with
>>> include not doing anything that would prevent
>>> dynamic module loading. I do not plan to implement
>>> dynamic loading. Tetsuo has been a strong
>>> advocate of loadable modules. I would expect to
>>> see a proposal from him shortly after the
>>> general stacking lands, assuming it does.
>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
>> dynamic modules from loading. We need a legitimate interface for loadable modules like
>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
>>
> I think we need something like below change when allowing loadable modules.

I believe that a simpler approach would be to
add a separate list of dynamic hooks to supliment
the list of static hooks. If SELinux unloading is
desired the SELinux hooks would be put on the
dynamic list which would not be "hardened" with
_ro_after_init, where the rest of the static modules
would be.


> >From b81fbc7c9c052c92f842777428bee2d5942ce9a9 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Tue, 21 Mar 2017 14:37:52 +0900
> Subject: [PATCH] security: Make dynamic module registration legitimate.
>
> While we are waiting for stacking support of in-tree built-in LSM modules
> (and therefore I am refraining from proposing changes for supporting
> dynamically loadable LSM modules), commit dd0859dccbe291cf ("security:
> introduce CONFIG_SECURITY_WRITABLE_HOOKS") and commit ca97d939db114c8d
> ("security: mark LSM hooks as __ro_after_init") came in. But these two
> commits are preventing us from making it possible to legitimately support
> dynamically loadable LSM modules due to lack of architecture-independent
> infrastructure for temporarily making LSM hooks writable.
>
> This patch demonstrates how we can make it possible to legitimately support
> dynamically loadable LSM modules by using set_memory_ro()/set_memory_nx()/
> set_memory_rw() infrastructure rather than __ro_after_init marking.
>
> Below is an example output for registration and runtime disablement of
> SELinux by this patch. pr_info() in this patch is only for testing purpose
> and will be removed in the final version.
>
> ----------
> [    0.014039] Security Framework initialized
> [    0.015006] Allocating LSM hooks for capability at ffff88013a10a000
> [    0.016003] Yama: becoming mindful.
> [    0.017008] Allocating LSM hooks for yama at ffff88013a10b000
> [    0.018006] LoadPin: ready to pin (currently disabled)
> [    0.018009] Allocating LSM hooks for loadpin at ffff88013a10c000
> [    0.020004] SELinux:  Initializing.
> [    0.021016] Allocating LSM hooks for selinux at ffff88013a114000
> [    0.022005] AppArmor: AppArmor disabled by boot time parameter
> [    0.023002] Marking LSM hook heads at ffffffff81853000 read only and non execute.
> [    0.024025] Marking LSM hooks for capability at ffff88013a10a000 read only and non execute.
> [    0.025730] Marking LSM hooks for yama at ffff88013a10b000 read only and non execute.
> [    0.026007] Marking LSM hooks for loadpin at ffff88013a10c000 read only and non execute.
> [    0.027007] Marking LSM hooks for selinux at ffff88013a114000 read only and non execute.
> [    0.028140] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes)
> (...snipped...)
> [    5.069577] SELinux:  Disabled at runtime.
> [    5.076575] Marking LSM hooks for capability at ffff88013a10a000 read write.
> [    5.087069] Marking LSM hooks for yama at ffff88013a10b000 read write.
> [    5.096963] Marking LSM hooks for loadpin at ffff88013a10c000 read write.
> [    5.104957] Marking LSM hooks for selinux at ffff88013a114000 read write.
> [    5.109179] Marking LSM hook heads at ffffffff81853000 read write.
> [    5.112881] Marking LSM hook heads at ffffffff81853000 read only and non execute.
> [    5.117276] Marking LSM hooks for capability at ffff88013a10a000 read only and non execute.
> [    5.122347] Marking LSM hooks for yama at ffff88013a10b000 read only and non execute.
> [    5.126936] Marking LSM hooks for loadpin at ffff88013a10c000 read only and non execute.
> [    5.149116] audit: type=1404 audit(1490064154.351:2): selinux=0 auid=4294967295 ses=4294967295
> ----------
>
> This patch mainly does the following things.
>
>   (1) Use set_memory_ro()/set_memory_nx() instead of __ro_after_init
>       so that runtime disablement of SELinux and runtime enablement of
>       dynamically loadable LSMs can use set_memory_rw().
>
>       While x86 architecture provides lookup_address() for finding
>       "struct page" from virtual address which allows us to temporarily
>       make specific pages writable, other architectures which support
>       CONFIG_STRICT_KERNEL_RWX feature which __ro_after_init depends on
>       (e.g. arm, parisc, s390) do not provide it.
>       But set_memory_ro()/set_memory_nx()/set_memory_rw() are already
>       available in x86, arm and s390. I don't know whether parisc can
>       implement set_memory_xx() but I assume it is possible. Therefore,
>       I assume that set_memory_xx() will support wider environments than
>       __ro_after_init while allowing runtime disablement of SELinux and
>       runtime enablement of dynamically loadable LSMs until we get
>       architecture-independent infrastructure for temporarily making LSM
>       hooks writable.
>
>   (2) Make "struct security_hook_list" variable in each LSM module
>       read-only by copying the content of that variable to dynamically
>       allocated (and protected via set_memory_ro()/set_memory_nx())
>       memory pages upon registration.
>
>   (3) Add EXPORT_SYMBOL_GPL(security_add_hooks) in order to legitimately
>       support dynamically loadable LSMs.
>
>   (4) 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 .
>
> This patch applies on top of reverting the two commits mentioned above.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  include/linux/lsm_hooks.h  |  34 +++++++-------
>  security/apparmor/lsm.c    |   2 +-
>  security/commoncap.c       |   2 +-
>  security/loadpin/loadpin.c |   2 +-
>  security/security.c        | 112 ++++++++++++++++++++++++++++++++++++++++++---
>  security/selinux/hooks.c   |  14 ++++--
>  security/smack/smack_lsm.c |   2 +-
>  security/tomoyo/tomoyo.c   |   2 +-
>  security/yama/yama_lsm.c   |   2 +-
>  9 files changed, 139 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index e29d4c6..668157f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1857,7 +1857,7 @@ struct security_hook_heads {
>  	struct list_head audit_rule_match;
>  	struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> -};
> +} __aligned(PAGE_SIZE);
>  
>  /*
>   * Security module hook list structure.
> @@ -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,14 @@ 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, const unsigned int count,
> +					    const char *lsm);
>  #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>  /*
>   * Assuring the safety of deleting a security module is up to
> @@ -1898,15 +1905,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/apparmor/lsm.c b/security/apparmor/lsm.c
> index 709eacd..04cf323 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -587,7 +587,7 @@ static int apparmor_task_setrlimit(struct task_struct *task,
>  	return error;
>  }
>  
> -static struct security_hook_list apparmor_hooks[] = {
> +static const struct security_hook_list apparmor_hooks[] __initconst = {
>  	LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
>  	LSM_HOOK_INIT(capget, apparmor_capget),
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 78b3783..c456805 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1071,7 +1071,7 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
>  
>  #ifdef CONFIG_SECURITY
>  
> -struct security_hook_list capability_hooks[] = {
> +static const struct security_hook_list capability_hooks[] __initconst = {
>  	LSM_HOOK_INIT(capable, cap_capable),
>  	LSM_HOOK_INIT(settime, cap_settime),
>  	LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 1d82eae..b88154a 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -174,7 +174,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
>  	return 0;
>  }
>  
> -static struct security_hook_list loadpin_hooks[] = {
> +static const struct security_hook_list loadpin_hooks[] __initconst = {
>  	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
>  	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> diff --git a/security/security.c b/security/security.c
> index d0e07f2..9cb2b3e 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 __ro_after_init;
>  char *lsm_names;
>  /* Boot-time LSM user choice */
>  static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
> @@ -47,6 +48,59 @@ static void __init do_security_initcalls(void)
>  	}
>  }
>  
> +static struct security_hook_heads security_hook_heads __aligned(PAGE_SIZE);
> +static LIST_HEAD(lsm_entry_list);
> +#if defined(CONFIG_STRICT_KERNEL_RWX) && !defined(CONFIG_PARISC)
> +static inline void mark_security_hooks_ro(void)
> +{
> +	struct lsm_entry *tmp;
> +	void *addr;
> +
> +	BUILD_BUG_ON(sizeof(security_hook_heads) % PAGE_SIZE);
> +	BUILD_BUG_ON(!PAGE_ALIGNED(&security_hook_heads));
> +	pr_info("Marking LSM hook heads at %p read only and non execute.\n",
> +		&security_hook_heads);
> +	addr = page_address(virt_to_page(&security_hook_heads));
> +	set_memory_ro((unsigned long) addr,
> +		      sizeof(security_hook_heads) >> PAGE_SHIFT);
> +	set_memory_nx((unsigned long) addr,
> +		      sizeof(security_hook_heads) >> PAGE_SHIFT);
> +	list_for_each_entry(tmp, &lsm_entry_list, head) {
> +		pr_info("Marking LSM hooks for %s at %p read only and non execute.\n",
> +			tmp->hooks[0].lsm, tmp);
> +		addr = page_address(virt_to_head_page(tmp));
> +		set_memory_ro((unsigned long) addr, 1 << tmp->order);
> +		set_memory_nx((unsigned long) addr, 1 << tmp->order);
> +	}
> +}
> +
> +static inline void mark_security_hooks_rw(void)
> +{
> +	struct lsm_entry *tmp;
> +	void *addr;
> +
> +	list_for_each_entry(tmp, &lsm_entry_list, head) {
> +		pr_info("Marking LSM hooks for %s at %p read write.\n",
> +			tmp->hooks[0].lsm, tmp);
> +		addr = page_address(virt_to_head_page(tmp));
> +		set_memory_rw((unsigned long) addr, 1 << tmp->order);
> +	}
> +	pr_info("Marking LSM hook heads at %p read write.\n",
> +		&security_hook_heads);
> +	addr = page_address(virt_to_page(&security_hook_heads));
> +	set_memory_rw((unsigned long) addr,
> +		      sizeof(security_hook_heads) >> PAGE_SHIFT);
> +}
> +#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 +122,8 @@ int __init security_init(void)
>  	 */
>  	do_security_initcalls();
>  
> +	lsm_initialized = true;
> +	mark_security_hooks_ro();
>  	return 0;
>  }
>  
> @@ -79,7 +135,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 +178,60 @@ 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,
> +				     const unsigned 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_nx()/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();
> +	pr_info("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;
> +}
> +/* Enable this when we start supporting loadable LSM modules.*/
> +EXPORT_SYMBOL_GPL(security_add_hooks);
> +
> +#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 +1720,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 0c2ac31..6fa68d9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6108,7 +6108,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
>  
>  #endif
>  
> -static struct security_hook_list selinux_hooks[] = {
> +static const struct security_hook_list selinux_hooks[] __initconst = {
>  	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
>  	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
>  	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),
> @@ -6323,6 +6323,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")) {
> @@ -6350,7 +6354,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");
> @@ -6479,7 +6487,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();
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index fc8fb31..fbc86a3 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4633,7 +4633,7 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
>  	return 0;
>  }
>  
> -static struct security_hook_list smack_hooks[] = {
> +static const struct security_hook_list smack_hooks[] __initconst = {
>  	LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
>  	LSM_HOOK_INIT(syslog, smack_syslog),
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index edc52d6..74ef461 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -496,7 +496,7 @@ static int tomoyo_socket_sendmsg(struct socket *sock, struct msghdr *msg,
>   * tomoyo_security_ops is a "struct security_operations" which is used for
>   * registering TOMOYO.
>   */
> -static struct security_hook_list tomoyo_hooks[] = {
> +static const struct security_hook_list tomoyo_hooks[] __initconst = {
>  	LSM_HOOK_INIT(cred_alloc_blank, tomoyo_cred_alloc_blank),
>  	LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare),
>  	LSM_HOOK_INIT(cred_transfer, tomoyo_cred_transfer),
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index 88271a3..6e1cb40 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -428,7 +428,7 @@ int yama_ptrace_traceme(struct task_struct *parent)
>  	return rc;
>  }
>  
> -static struct security_hook_list yama_hooks[] = {
> +static const struct security_hook_list yama_hooks[] __initconst = {
>  	LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>  	LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>  	LSM_HOOK_INIT(task_prctl, yama_task_prctl),

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 15:36           ` Casey Schaufler
@ 2017-03-21 16:06             ` Peter Moody
  2017-03-21 16:21               ` Casey Schaufler
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Moody @ 2017-03-21 16:06 UTC (permalink / raw)
  To: linux-security-module

On Tue, Mar 21, 2017 at 8:36 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
>> Tetsuo Handa wrote:
>>> Casey Schaufler wrote:
>>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
>>>>>
>>>>> I would love to have the ability write a small lsm that I can build as
>>>>> a module and load at boot eg. via initrd.
>>>>>
>>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
>>>>> a new kernel, etc. I know there are objections to dynamically loadable
>>>>> lsms and I was trying to find a compromise that made them easier to
>>>>> work with.
>>>> The stacking design criteria I'm working with
>>>> include not doing anything that would prevent
>>>> dynamic module loading. I do not plan to implement
>>>> dynamic loading. Tetsuo has been a strong
>>>> advocate of loadable modules. I would expect to
>>>> see a proposal from him shortly after the
>>>> general stacking lands, assuming it does.
>>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
>>> dynamic modules from loading. We need a legitimate interface for loadable modules like
>>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
>>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
>>>
>> I think we need something like below change when allowing loadable modules.
>
> I believe that a simpler approach would be to
> add a separate list of dynamic hooks to supliment
> the list of static hooks. If SELinux unloading is
> desired the SELinux hooks would be put on the
> dynamic list which would not be "hardened" with
> _ro_after_init, where the rest of the static modules
> would be.

FWIW, I don't know if that would solve the case I was initially asking
about since the out-of-tree lsm I was hoping to be able to access all
of the standard security hooks with an out-of-tree module.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 16:06             ` Peter Moody
@ 2017-03-21 16:21               ` Casey Schaufler
  2017-03-21 21:53                 ` Tetsuo Handa
  0 siblings, 1 reply; 12+ messages in thread
From: Casey Schaufler @ 2017-03-21 16:21 UTC (permalink / raw)
  To: linux-security-module

On 3/21/2017 9:06 AM, Peter Moody wrote:
> On Tue, Mar 21, 2017 at 8:36 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
>>> Tetsuo Handa wrote:
>>>> Casey Schaufler wrote:
>>>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
>>>>>>
>>>>>> I would love to have the ability write a small lsm that I can build as
>>>>>> a module and load at boot eg. via initrd.
>>>>>>
>>>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
>>>>>> a new kernel, etc. I know there are objections to dynamically loadable
>>>>>> lsms and I was trying to find a compromise that made them easier to
>>>>>> work with.
>>>>> The stacking design criteria I'm working with
>>>>> include not doing anything that would prevent
>>>>> dynamic module loading. I do not plan to implement
>>>>> dynamic loading. Tetsuo has been a strong
>>>>> advocate of loadable modules. I would expect to
>>>>> see a proposal from him shortly after the
>>>>> general stacking lands, assuming it does.
>>>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
>>>> dynamic modules from loading. We need a legitimate interface for loadable modules like
>>>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
>>>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
>>>>
>>> I think we need something like below change when allowing loadable modules.
>> I believe that a simpler approach would be to
>> add a separate list of dynamic hooks to supliment
>> the list of static hooks. If SELinux unloading is
>> desired the SELinux hooks would be put on the
>> dynamic list which would not be "hardened" with
>> _ro_after_init, where the rest of the static modules
>> would be.
> FWIW, I don't know if that would solve the case I was initially asking
> about since the out-of-tree lsm I was hoping to be able to access all
> of the standard security hooks with an out-of-tree module.

It would work fine. All I'm suggesting is that in addition
to security_hook_heads there would be a
security_hooks_heads_dynamic. The code in security.c would
be stretched to loop through both lists. Any locking or
other complexity associated with being dynamic would be
limited to the dynamic list.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 16:21               ` Casey Schaufler
@ 2017-03-21 21:53                 ` Tetsuo Handa
  2017-03-21 22:10                   ` Casey Schaufler
  0 siblings, 1 reply; 12+ messages in thread
From: Tetsuo Handa @ 2017-03-21 21:53 UTC (permalink / raw)
  To: linux-security-module

Casey Schaufler wrote:
> On 3/21/2017 9:06 AM, Peter Moody wrote:
> > On Tue, Mar 21, 2017 at 8:36 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
> >>> Tetsuo Handa wrote:
> >>>> Casey Schaufler wrote:
> >>>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
> >>>>>>
> >>>>>> I would love to have the ability write a small lsm that I can build as
> >>>>>> a module and load at boot eg. via initrd.
> >>>>>>
> >>>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
> >>>>>> a new kernel, etc. I know there are objections to dynamically loadable
> >>>>>> lsms and I was trying to find a compromise that made them easier to
> >>>>>> work with.
> >>>>> The stacking design criteria I'm working with
> >>>>> include not doing anything that would prevent
> >>>>> dynamic module loading. I do not plan to implement
> >>>>> dynamic loading. Tetsuo has been a strong
> >>>>> advocate of loadable modules. I would expect to
> >>>>> see a proposal from him shortly after the
> >>>>> general stacking lands, assuming it does.
> >>>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
> >>>> dynamic modules from loading. We need a legitimate interface for loadable modules like
> >>>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
> >>>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
> >>>>
> >>> I think we need something like below change when allowing loadable modules.
> >> I believe that a simpler approach would be to
> >> add a separate list of dynamic hooks to supliment
> >> the list of static hooks. If SELinux unloading is
> >> desired the SELinux hooks would be put on the
> >> dynamic list which would not be "hardened" with
> >> _ro_after_init, where the rest of the static modules
> >> would be.
> > FWIW, I don't know if that would solve the case I was initially asking
> > about since the out-of-tree lsm I was hoping to be able to access all
> > of the standard security hooks with an out-of-tree module.
> 
> It would work fine. All I'm suggesting is that in addition
> to security_hook_heads there would be a
> security_hooks_heads_dynamic. The code in security.c would
> be stretched to loop through both lists. Any locking or
> other complexity associated with being dynamic would be
> limited to the dynamic list.
> 
Yes, adding security_hooks_heads_dynamic would work about calling hooks.
But why not to protect security_hooks_heads_dynamic with mostly-read-only
protection when security_hooks_heads is protected with __ro_after_init?
I don't think SELinux wants to give up read-only protection only for
allowing runtime disable.

And if protecting security_hooks_heads_dynamic, why to use separate lists?
Is keeping security_hooks_heads __ro_after_init a worthwhile protection
when we add a dynamic module to security_hooks_heads_dynamic? A malicious
dynamic module can after all tamper security_hooks_heads by making it
read-write.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 21:53                 ` Tetsuo Handa
@ 2017-03-21 22:10                   ` Casey Schaufler
  2017-03-22 12:13                     ` Tetsuo Handa
  0 siblings, 1 reply; 12+ messages in thread
From: Casey Schaufler @ 2017-03-21 22:10 UTC (permalink / raw)
  To: linux-security-module

On 3/21/2017 2:53 PM, Tetsuo Handa wrote:
> Casey Schaufler wrote:
>> On 3/21/2017 9:06 AM, Peter Moody wrote:
>>> On Tue, Mar 21, 2017 at 8:36 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
>>>>> Tetsuo Handa wrote:
>>>>>> Casey Schaufler wrote:
>>>>>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
>>>>>>>>
>>>>>>>> I would love to have the ability write a small lsm that I can build as
>>>>>>>> a module and load at boot eg. via initrd.
>>>>>>>>
>>>>>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
>>>>>>>> a new kernel, etc. I know there are objections to dynamically loadable
>>>>>>>> lsms and I was trying to find a compromise that made them easier to
>>>>>>>> work with.
>>>>>>> The stacking design criteria I'm working with
>>>>>>> include not doing anything that would prevent
>>>>>>> dynamic module loading. I do not plan to implement
>>>>>>> dynamic loading. Tetsuo has been a strong
>>>>>>> advocate of loadable modules. I would expect to
>>>>>>> see a proposal from him shortly after the
>>>>>>> general stacking lands, assuming it does.
>>>>>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
>>>>>> dynamic modules from loading. We need a legitimate interface for loadable modules like
>>>>>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
>>>>>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
>>>>>>
>>>>> I think we need something like below change when allowing loadable modules.
>>>> I believe that a simpler approach would be to
>>>> add a separate list of dynamic hooks to supliment
>>>> the list of static hooks. If SELinux unloading is
>>>> desired the SELinux hooks would be put on the
>>>> dynamic list which would not be "hardened" with
>>>> _ro_after_init, where the rest of the static modules
>>>> would be.
>>> FWIW, I don't know if that would solve the case I was initially asking
>>> about since the out-of-tree lsm I was hoping to be able to access all
>>> of the standard security hooks with an out-of-tree module.
>> It would work fine. All I'm suggesting is that in addition
>> to security_hook_heads there would be a
>> security_hooks_heads_dynamic. The code in security.c would
>> be stretched to loop through both lists. Any locking or
>> other complexity associated with being dynamic would be
>> limited to the dynamic list.
>>
> Yes, adding security_hooks_heads_dynamic would work about calling hooks.
> But why not to protect security_hooks_heads_dynamic with mostly-read-only
> protection when security_hooks_heads is protected with __ro_after_init?

I'd be fine with that. What I don't care for
is adding the complexity of mostly-read-only
to the complied-in-load-at-init case.

> I don't think SELinux wants to give up read-only protection only for
> allowing runtime disable.

The read-only protection is very new, and wasn't missed
greatly before it was added. I also understand that SELinux
is looking to remove the runtime disable feature.

> And if protecting security_hooks_heads_dynamic, why to use separate lists?
> Is keeping security_hooks_heads __ro_after_init a worthwhile protection
> when we add a dynamic module to security_hooks_heads_dynamic? A malicious
> dynamic module can after all tamper security_hooks_heads by making it
> read-write.

This is where I always get a headache. You want
the list of hooks to be mutable, but you don't want
to loose the __ro_after_init protection. The two
list approach allows the modules that are not dynamic
to be protected, and those that want to change to
change. It addresses the concern of those who want
"static" module lists to be hard while allowing
loadable modules.

I also assume that if we allow loadable modules at
some point it will be optional.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 12+ messages in thread

* out of tree lsm's
  2017-03-21 22:10                   ` Casey Schaufler
@ 2017-03-22 12:13                     ` Tetsuo Handa
  0 siblings, 0 replies; 12+ messages in thread
From: Tetsuo Handa @ 2017-03-22 12:13 UTC (permalink / raw)
  To: linux-security-module

Casey Schaufler wrote:
> On 3/21/2017 2:53 PM, Tetsuo Handa wrote:
> > Casey Schaufler wrote:
> >> On 3/21/2017 9:06 AM, Peter Moody wrote:
> >>> On Tue, Mar 21, 2017 at 8:36 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>> On 3/21/2017 3:41 AM, Tetsuo Handa wrote:
> >>>>> Tetsuo Handa wrote:
> >>>>>> Casey Schaufler wrote:
> >>>>>>>> right. sorry for the imprecise language; by site-specific I meant a "small" lsm.
> >>>>>>>>
> >>>>>>>> I would love to have the ability write a small lsm that I can build as
> >>>>>>>> a module and load at boot eg. via initrd.
> >>>>>>>>
> >>>>>>>> AIUI, adding even a new "small" lsm requires kconfig patches, building
> >>>>>>>> a new kernel, etc. I know there are objections to dynamically loadable
> >>>>>>>> lsms and I was trying to find a compromise that made them easier to
> >>>>>>>> work with.
> >>>>>>> The stacking design criteria I'm working with
> >>>>>>> include not doing anything that would prevent
> >>>>>>> dynamic module loading. I do not plan to implement
> >>>>>>> dynamic loading. Tetsuo has been a strong
> >>>>>>> advocate of loadable modules. I would expect to
> >>>>>>> see a proposal from him shortly after the
> >>>>>>> general stacking lands, assuming it does.
> >>>>>> But currently __lsm_ro_after_init which is planned to go to 4.12 is preventing
> >>>>>> dynamic modules from loading. We need a legitimate interface for loadable modules like
> >>>>>> http://lkml.kernel.org/r/201702152342.GBH04183.FOFJFHQOLMOtVS at I-love.SAKURA.ne.jp .
> >>>>>> Requiring rodata=0 kernel command line option to allow dynamic modules is silly.
> >>>>>>
> >>>>> I think we need something like below change when allowing loadable modules.
> >>>> I believe that a simpler approach would be to
> >>>> add a separate list of dynamic hooks to supliment
> >>>> the list of static hooks. If SELinux unloading is
> >>>> desired the SELinux hooks would be put on the
> >>>> dynamic list which would not be "hardened" with
> >>>> _ro_after_init, where the rest of the static modules
> >>>> would be.
> >>> FWIW, I don't know if that would solve the case I was initially asking
> >>> about since the out-of-tree lsm I was hoping to be able to access all
> >>> of the standard security hooks with an out-of-tree module.
> >> It would work fine. All I'm suggesting is that in addition
> >> to security_hook_heads there would be a
> >> security_hooks_heads_dynamic. The code in security.c would
> >> be stretched to loop through both lists. Any locking or
> >> other complexity associated with being dynamic would be
> >> limited to the dynamic list.
> >>
> > Yes, adding security_hooks_heads_dynamic would work about calling hooks.
> > But why not to protect security_hooks_heads_dynamic with mostly-read-only
> > protection when security_hooks_heads is protected with __ro_after_init?
> 
> I'd be fine with that. What I don't care for
> is adding the complexity of mostly-read-only
> to the complied-in-load-at-init case.
> 
> > I don't think SELinux wants to give up read-only protection only for
> > allowing runtime disable.
> 
> The read-only protection is very new, and wasn't missed
> greatly before it was added. I also understand that SELinux
> is looking to remove the runtime disable feature.
> 
> > And if protecting security_hooks_heads_dynamic, why to use separate lists?
> > Is keeping security_hooks_heads __ro_after_init a worthwhile protection
> > when we add a dynamic module to security_hooks_heads_dynamic? A malicious
> > dynamic module can after all tamper security_hooks_heads by making it
> > read-write.
> 
> This is where I always get a headache. You want
> the list of hooks to be mutable, but you don't want
> to loose the __ro_after_init protection. The two
> list approach allows the modules that are not dynamic
> to be protected, and those that want to change to
> change. It addresses the concern of those who want
> "static" module lists to be hard while allowing
> loadable modules.
> 
> I also assume that if we allow loadable modules at
> some point it will be optional.

So, roughly speaking, below patch is what you mean, isn't it?
Yes, it would work. 

I wonder whether the complication by splitting into security_hooks_heads
and security_hooks_heads_dynamic (because it is impossible to make only
security_hooks_heads[0] read-only when declared as security_hooks_heads[2]
and use security_hooks_heads[1] read-write instead of declaring
security_hooks_heads and security_hooks_heads_dynamic) is worthwhile
when we add mostly-read-only protection to security_hooks_heads_dynamic.


>From 08ac4ea012f91e640f895df2978b40f2feb89550 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 22 Mar 2017 20:50:47 +0900
Subject: [PATCH] LSM: Make dynamic module registration legitimate.

This patch applies on top of below two patches.

  [PATCH 1/2] LSM: Initialize security_hook_heads upon registration.
  [PATCH 2/2] LSM: Make security_hook_heads a local variable.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 include/linux/lsm_hooks.h  | 12 +++---
 security/Kconfig           |  6 ++-
 security/apparmor/lsm.c    |  2 +-
 security/commoncap.c       |  2 +-
 security/loadpin/loadpin.c |  2 +-
 security/security.c        | 93 ++++++++++++++++++++++++++++++++++++++++++++--
 security/selinux/Kconfig   |  4 +-
 security/selinux/hooks.c   |  7 +++-
 security/smack/smack_lsm.c |  2 +-
 security/tomoyo/tomoyo.c   |  2 +-
 security/yama/yama_lsm.c   |  2 +-
 11 files changed, 113 insertions(+), 21 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 54191cf..712b2d3 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1897,6 +1897,11 @@ struct security_hook_list {
 extern void security_add_hooks(struct security_hook_list *hooks, int count,
 				char *lsm);
 
+#ifdef CONFIG_SECURITY_DYNAMIC_LOADING
+extern void security_add_hooks_dynamic(struct security_hook_list *hooks,
+				       int count, char *lsm);
+#endif
+
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 /*
  * Assuring the safety of deleting a security module is up to
@@ -1920,13 +1925,6 @@ static inline void security_delete_hooks(struct security_hook_list *hooks,
 }
 #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
 
-/* Currently required to handle SELinux runtime hook disable. */
-#ifdef CONFIG_SECURITY_WRITABLE_HOOKS
-#define __lsm_ro_after_init
-#else
-#define __lsm_ro_after_init	__ro_after_init
-#endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
-
 extern int __init security_module_enable(const char *module);
 extern void __init capability_add_hooks(void);
 #ifdef CONFIG_SECURITY_YAMA
diff --git a/security/Kconfig b/security/Kconfig
index 3ff1bf9..e852cb0 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -31,10 +31,12 @@ config SECURITY
 
 	  If you are unsure how to answer this question, answer N.
 
-config SECURITY_WRITABLE_HOOKS
+config SECURITY_DYNAMIC_LOADING
 	depends on SECURITY
-	bool
+	bool "Allow dynamic registration of LSM modules"
 	default n
+	help
+	  This option allows loadable LSM modules.
 
 config SECURITYFS
 	bool "Enable the securityfs filesystem"
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index e287b69..73aad35 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -587,7 +587,7 @@ static int apparmor_task_setrlimit(struct task_struct *task,
 	return error;
 }
 
-static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list apparmor_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
 	LSM_HOOK_INIT(capget, apparmor_capget),
diff --git a/security/commoncap.c b/security/commoncap.c
index 7abebd7..9704ed0 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1071,7 +1071,7 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
 
 #ifdef CONFIG_SECURITY
 
-struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
+struct security_hook_list capability_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(capable, cap_capable),
 	LSM_HOOK_INIT(settime, cap_settime),
 	LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index dbe6efd..aaef8d9 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -174,7 +174,7 @@ static int loadpin_read_file(struct file *file, enum kernel_read_file_id id)
 	return 0;
 }
 
-static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list loadpin_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
 	LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
 };
diff --git a/security/security.c b/security/security.c
index a5868ed..493f260 100644
--- a/security/security.c
+++ b/security/security.c
@@ -32,7 +32,13 @@
 /* Maximum number of letters for an LSM name string */
 #define SECURITY_NAME_MAX	10
 
-static struct security_hook_heads security_hook_heads __lsm_ro_after_init;
+static struct security_hook_heads security_hook_heads __ro_after_init;
+#ifdef CONFIG_SECURITY_DYNAMIC_LOADING
+static struct security_hook_heads security_hook_heads_dynamic;
+#else
+/* Dummy for hiding build errors. */
+#define security_hook_heads_dynamic security_hook_heads
+#endif
 char *lsm_names;
 /* Boot-time LSM user choice */
 static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1] =
@@ -61,6 +67,12 @@ int __init security_init(void)
 	for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct list_head);
 	     i++)
 		INIT_LIST_HEAD(&list[i]);
+#ifdef CONFIG_SECURITY_DYNAMIC_LOADING
+	list = (struct list_head *) &security_hook_heads_dynamic;
+	for (i = 0; i < sizeof(security_hook_heads_dynamic) /
+		     sizeof(struct list_head); i++)
+		INIT_LIST_HEAD(&list[i]);
+#endif
 	pr_info("Security Framework initialized\n");
 
 	/*
@@ -143,6 +155,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 		panic("%s - Cannot get early memory.\n", __func__);
 }
 
+#ifdef CONFIG_SECURITY_DYNAMIC_LOADING
+void security_add_hooks_dynamic(struct security_hook_list *hooks, int count,
+				char *lsm)
+{
+	int i;
+	struct list_head *list =
+		(struct list_head *) &security_hook_heads_dynamic;
+
+	for (i = 0; i < count; i++) {
+		hooks[i].lsm = lsm;
+		list_add_tail_rcu(&hooks[i].list, &list[hooks[i].idx]);
+	}
+	if (lsm_append(lsm, &lsm_names) < 0)
+		panic("%s - Cannot get early memory.\n", __func__);
+}
+EXPORT_SYMBOL_GPL(security_add_hooks_dynamic);
+#endif
+
 /*
  * Hook list operation macros.
  *
@@ -159,6 +189,11 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 								\
 		list_for_each_entry(P, &security_hook_heads.FUNC, list)	\
 			P->hook.FUNC(__VA_ARGS__);		\
+		if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING))	\
+			list_for_each_entry(P,				\
+				    &security_hook_heads_dynamic.FUNC,  \
+					    list)			\
+				P->hook.FUNC(__VA_ARGS__);		\
 	} while (0)
 
 #define call_int_hook(FUNC, IRC, ...) ({			\
@@ -171,6 +206,14 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 			if (RC != 0)				\
 				break;				\
 		}						\
+		if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING))	\
+			list_for_each_entry(P,				\
+				    &security_hook_heads_dynamic.FUNC,  \
+					    list) {			\
+				RC = P->hook.FUNC(__VA_ARGS__);		\
+				if (RC != 0)				\
+					break;				\
+			}						\
 	} while (0);						\
 	RC;							\
 })
@@ -280,6 +323,16 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
 			break;
 		}
 	}
+	if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING) && cap_sys_admin)
+		list_for_each_entry(hp,
+			    &security_hook_heads_dynamic.vm_enough_memory,
+				    list) {
+			rc = hp->hook.vm_enough_memory(mm, pages);
+			if (rc <= 0) {
+				cap_sys_admin = 0;
+				break;
+			}
+		}
 	return __vm_enough_memory(mm, pages, cap_sys_admin);
 }
 
@@ -768,6 +821,15 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
 		if (rc != -EOPNOTSUPP)
 			return rc;
 	}
+	if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING))
+		list_for_each_entry(hp,
+			    &security_hook_heads_dynamic.inode_getsecurity,
+				    list) {
+			rc = hp->hook.inode_getsecurity(inode, name, buffer,
+							alloc);
+			if (rc != -EOPNOTSUPP)
+				return rc;
+		}
 	return -EOPNOTSUPP;
 }
 
@@ -787,6 +849,15 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
 		if (rc != -EOPNOTSUPP)
 			return rc;
 	}
+	if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING))
+		list_for_each_entry(hp,
+			    &security_hook_heads_dynamic.inode_setsecurity,
+				    list) {
+			rc = hp->hook.inode_setsecurity(inode, name, value,
+							size, flags);
+			if (rc != -EOPNOTSUPP)
+				return rc;
+		}
 	return -EOPNOTSUPP;
 }
 
@@ -1092,6 +1163,17 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 				break;
 		}
 	}
+	if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING) && rc == -ENOSYS)
+		list_for_each_entry(hp, &security_hook_heads_dynamic.task_prctl,
+				    list) {
+			thisrc = hp->hook.task_prctl(option, arg2, arg3, arg4,
+						     arg5);
+			if (thisrc != -ENOSYS) {
+				rc = thisrc;
+				if (thisrc != 0)
+					break;
+			}
+		}
 	return rc;
 }
 
@@ -1562,9 +1644,14 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
 	 */
 	list_for_each_entry(hp, &security_hook_heads.xfrm_state_pol_flow_match,
 				list) {
-		rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
-		break;
+		return hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
 	}
+	if (IS_ENABLED(CONFIG_SECURITY_DYNAMIC_LOADING))
+		list_for_each_entry(hp,
+			&security_hook_heads_dynamic.xfrm_state_pol_flow_match,
+				    list) {
+			return hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
+		}
 	return rc;
 }
 
diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
index 8af7a69..3b9ce3d 100644
--- a/security/selinux/Kconfig
+++ b/security/selinux/Kconfig
@@ -40,7 +40,7 @@ config SECURITY_SELINUX_BOOTPARAM_VALUE
 config SECURITY_SELINUX_DISABLE
 	bool "NSA SELinux runtime disable"
 	depends on SECURITY_SELINUX
-	select SECURITY_WRITABLE_HOOKS
+	select SECURITY_DYNAMIC_LOADING
 	default n
 	help
 	  This option enables writing to a selinuxfs node 'disable', which
@@ -52,7 +52,7 @@ config SECURITY_SELINUX_DISABLE
 	  to employ.
 
 	  NOTE: selecting this option will disable the '__ro_after_init'
-	  kernel hardening feature for security hooks.   Please consider
+	  kernel hardening feature for SELinux hooks.   Please consider
 	  using the selinux=0 boot parameter instead of enabling this
 	  option.
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d37a723..f5f348e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6123,7 +6123,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
 
 #endif
 
-static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list selinux_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
 	LSM_HOOK_INIT(binder_transfer_binder, selinux_binder_transfer_binder),
@@ -6366,7 +6366,12 @@ static __init int selinux_init(void)
 					    0, SLAB_PANIC, NULL);
 	avc_init();
 
+#ifndef CONFIG_SECURITY_SELINUX_DISABLE
 	security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+#else
+	security_add_hooks_dynamic(selinux_hooks, ARRAY_SIZE(selinux_hooks),
+				   "selinux");
+#endif
 
 	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 927e60e..cfaa78c 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4633,7 +4633,7 @@ static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 	return 0;
 }
 
-static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list smack_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
 	LSM_HOOK_INIT(syslog, smack_syslog),
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index b5fb930..2677427 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -496,7 +496,7 @@ static int tomoyo_socket_sendmsg(struct socket *sock, struct msghdr *msg,
  * tomoyo_security_ops is a "struct security_operations" which is used for
  * registering TOMOYO.
  */
-static struct security_hook_list tomoyo_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list tomoyo_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(cred_alloc_blank, tomoyo_cred_alloc_blank),
 	LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, tomoyo_cred_transfer),
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 8298e09..fae3cd9 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -428,7 +428,7 @@ int yama_ptrace_traceme(struct task_struct *parent)
 	return rc;
 }
 
-static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
+static struct security_hook_list yama_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
 	LSM_HOOK_INIT(task_prctl, yama_task_prctl),
-- 
1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-03-22 12:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20 18:54 out of tree lsm's Peter Moody
2017-03-20 19:30 ` Paul Moore
2017-03-20 19:45   ` Peter Moody
2017-03-20 20:17     ` Casey Schaufler
2017-03-20 22:18       ` Tetsuo Handa
2017-03-21 10:41         ` Tetsuo Handa
2017-03-21 15:36           ` Casey Schaufler
2017-03-21 16:06             ` Peter Moody
2017-03-21 16:21               ` Casey Schaufler
2017-03-21 21:53                 ` Tetsuo Handa
2017-03-21 22:10                   ` Casey Schaufler
2017-03-22 12:13                     ` Tetsuo Handa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).