Re: [PATCH] LSM: Allow syzbot to ignore security= parameter.
From: Casey Schaufler <casey@schaufler-ca.com>
Date: 2019-02-08 16:23:44
Also in:
lkml, selinux
On 2/8/2019 2:52 AM, Tetsuo Handa wrote:
On 2019/02/08 1:24, Casey Schaufler wrote:quoted
quoted
quoted
quoted
Then, I think that it is straightforward (and easier to manage) to ignore security= parameter when lsm= parameter is specified.That reduces flexibility somewhat. If I am debugging security modules I may want to use lsm= to specify the order while using security= to identify a specific exclusive module. I could do that using lsm= by itself, but habits die hard."lsm=" can be used for identifying a specific exclusive module, and Ubuntu kernels would have to use CONFIG_LSM (or "lsm=") for identifying the default exclusive module (in order to allow enabling both TOMOYO and one of SELinux,Smack,AppArmor at the same time). Since "security=" can't be used for selectively enable/disable more than one of SELinux,Smack,TOMOYO,AppArmor, I think that recommending users to migrate to "lsm=" is the better direction. And ignoring "security=" when "lsm=" is specified is easier to understand.I added Kees to the CC list. Kees, what to you think about ignoring security= if lsm= is specified? I'm ambivalent.To help administrators easily understand what LSM modules are possibly enabled by default (which have to be fetched from e.g. /boot/config-`uname -r`)
$ cat /sys/kernel/security/lsm
quoted hunk ↗ jump to hunk
and specify lsm= parameter when they need, I propose changes shown below.diff --git a/security/security.c b/security/security.c index 3147785e..051d708 100644 --- a/security/security.c +++ b/security/security.c@@ -51,8 +51,6 @@ static __initdata const char *chosen_lsm_order; static __initdata const char *chosen_major_lsm; -static __initconst const char * const builtin_lsm_order = CONFIG_LSM; - /* Ordered list of LSMs to initialize. */ static __initdata struct lsm_info **ordered_lsms; static __initdata struct lsm_info *exclusive;@@ -284,14 +282,22 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) static void __init ordered_lsm_init(void) { struct lsm_info **lsm; + const char *order = CONFIG_LSM; + const char *origin = "builtin"; ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), GFP_KERNEL); - if (chosen_lsm_order) - ordered_lsm_parse(chosen_lsm_order, "cmdline"); - else - ordered_lsm_parse(builtin_lsm_order, "builtin"); + if (chosen_lsm_order) { + if (chosen_major_lsm) { + pr_info("security= is ignored because of lsm=\n"); + chosen_major_lsm = NULL; + } + order = chosen_lsm_order; + origin = "cmdline"; + } + pr_info("Security Framework initializing: %s\n", order); + ordered_lsm_parse(order, origin); for (lsm = ordered_lsms; *lsm; lsm++) prepare_lsm(*lsm);@@ -333,8 +339,6 @@ int __init security_init(void) int i; struct hlist_head *list = (struct hlist_head *) &security_hook_heads; - pr_info("Security Framework initializing\n"); - for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head); i++) INIT_HLIST_HEAD(&list[i]);
I'm not going to object to this, but I don't see it as important.