| Message ID | 20250409185019.238841-40-paul@paul-moore.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Rework the LSM initialization | expand |
On Wed, Apr 09, 2025 at 02:49:54PM -0400, Paul Moore wrote: > One part of a larger effort to cleanup the LSM framework initialization > code. > > Signed-off-by: Paul Moore <paul@paul-moore.com> > --- > security/inode.c | 9 ++-- > security/lsm_init.c | 110 ++++++++++++++++++++++++-------------------- > 2 files changed, 63 insertions(+), 56 deletions(-) > > diff --git a/security/inode.c b/security/inode.c > index 49bc3578bd23..f687e22e6809 100644 > --- a/security/inode.c > +++ b/security/inode.c > @@ -351,18 +351,17 @@ static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, > > for (i = 0; i < lsm_count; i++) > /* the '+ 1' accounts for either a comma or a NUL terminator */ > - len += strlen(lsm_order[i]->id->name) + 1; > + len += strlen(lsm_idlist[i]->name) + 1; > > str = kmalloc(len, GFP_KERNEL); > if (!str) > return -ENOMEM; > str[0] = '\0'; > > - i = 0; > - while (i < lsm_count) { > - strcat(str, lsm_order[i]->id->name); > - if (++i < lsm_count) > + for (i = 0; i < lsm_count; i++) { > + if (i > 0) > strcat(str, ","); > + strcat(str, lsm_idlist[i]->name); > } > > rc = simple_read_from_buffer(buf, count, ppos, str, len); This chunk needs to be folded into the lsm_names changing patch, I think. I missed this on the first pass, but lsm_order can never be used here because lsm_order is initdata -- it will be thrown away after init is done. > diff --git a/security/lsm_init.c b/security/lsm_init.c > index 978bb81b58fa..7f2bc8c22ce9 100644 > --- a/security/lsm_init.c > +++ b/security/lsm_init.c > @@ -10,6 +10,10 @@ > > #include "lsm.h" > > +/* LSM enabled constants. */ > +int lsm_enabled_true = 1; > +int lsm_enabled_false = 0; Why are these losing static and __initdata? It looks like they're staying assigned to the __init-marked lsm_info instances. > + > /* Pointers to LSM sections defined in include/asm-generic/vmlinux.lds.h */ > extern struct lsm_info __start_lsm_info[], __end_lsm_info[]; > extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[]; > @@ -72,41 +76,42 @@ static int __init lsm_debug_enable(char *str) > } > __setup("lsm.debug", lsm_debug_enable); > > -/* Mark an LSM's enabled flag. */ > -static int lsm_enabled_true __initdata = 1; > -static int lsm_enabled_false __initdata = 0; > -static void __init set_enabled(struct lsm_info *lsm, bool enabled) > +/** > + * lsm_enabled_set - Mark a LSM as enabled > + * @lsm: LSM definition > + * @enabled: enabled flag > + */ > +static void __init lsm_enabled_set(struct lsm_info *lsm, bool enabled) > { > /* > * When an LSM hasn't configured an enable variable, we can use > * a hard-coded location for storing the default enabled state. > */ > - if (!lsm->enabled) { > - if (enabled) > - lsm->enabled = &lsm_enabled_true; > - else > - lsm->enabled = &lsm_enabled_false; > - } else if (lsm->enabled == &lsm_enabled_true) { > - if (!enabled) > - lsm->enabled = &lsm_enabled_false; > - } else if (lsm->enabled == &lsm_enabled_false) { > - if (enabled) > - lsm->enabled = &lsm_enabled_true; > + if (!lsm->enabled || > + lsm->enabled == &lsm_enabled_true || > + lsm->enabled == &lsm_enabled_false) { > + lsm->enabled = enabled ? &lsm_enabled_true : &lsm_enabled_false; > } else { > *lsm->enabled = enabled; > } > } Good logic folding. > > -static inline bool is_enabled(struct lsm_info *lsm) > +/** > + * lsm_is_enabled - Determine if a LSM is enabled > + * @lsm: LSM definition > + */ > +static inline bool lsm_is_enabled(struct lsm_info *lsm) > { > if (!lsm->enabled) > return false; > - > return *lsm->enabled; > } This could be one-lined, actually: return lsm->enabled ? *lsm->enabled : false; > > -/* Is an LSM already listed in the ordered LSMs list? */ > -static bool __init exists_ordered_lsm(struct lsm_info *lsm) > +/** > + * lsm_order_exists - Determine if a LSM exists in the ordered list > + * @lsm: LSM definition > + */ > +static bool __init lsm_order_exists(struct lsm_info *lsm) > { > struct lsm_info **check; > > @@ -118,25 +123,29 @@ static bool __init exists_ordered_lsm(struct lsm_info *lsm) > return false; > } > > -/* Append an LSM to the list of ordered LSMs to initialize. */ > -static int last_lsm __initdata; > -static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) > +/** > + * lsm_order_append - Append a LSM to the ordered list > + * @lsm: LSM definition > + * @src: source of the addition > + */ > +static void __init lsm_order_append(struct lsm_info *lsm, const char *src) > { > /* Ignore duplicate selections. */ > - if (exists_ordered_lsm(lsm)) > + if (lsm_order_exists(lsm)) > return; > > - if (WARN(last_lsm == MAX_LSM_COUNT, "%s: out of LSM static calls!?\n", from)) > - return; > + /* Skip explicitly disabled LSMs. */ > + if (lsm->enabled && !lsm_is_enabled(lsm)) { > + if (WARN(lsm_count == MAX_LSM_COUNT, > + "%s: out of LSM static calls!?\n", src)) > + return; > + lsm_enabled_set(lsm, true); > + lsm_order[lsm_count] = lsm; > + lsm_idlist[lsm_count++] = lsm->id; > + } > > - /* Enable this LSM, if it is not already set. */ > - if (!lsm->enabled) > - lsm->enabled = &lsm_enabled_true; > - lsm_order[last_lsm] = lsm; > - lsm_idlist[last_lsm++] = lsm->id; I don't understand the logic change here. I may be missing something (it feels like a lot of logic changes mixed together again), but this logic: /* Enable this LSM, if it is not already set. */ if (!lsm->enabled) lsm->enabled = &lsm_enabled_true; seems like it has gone missing now? And I think the last_lsm/lsm_count changes need to be in the "lsm: rework lsm_active_cnt and lsm_idlist[]" patch? I'm really struggling to follow this patch, but maybe I am EOD. :P > - > - init_debug("%s ordered: %s (%s)\n", from, lsm->id->name, > - is_enabled(lsm) ? "enabled" : "disabled"); > + init_debug("%s ordered: %s (%s)\n", src, lsm->id->name, > + lsm_is_enabled(lsm) ? "enabled" : "disabled"); > } > > static void __init lsm_set_blob_size(int *need, int *lbs) > @@ -159,17 +168,17 @@ static void __init lsm_prep_single(struct lsm_info *lsm) > { > struct lsm_blob_sizes *blobs; > > - if (!is_enabled(lsm)) { > - set_enabled(lsm, false); > + if (!lsm_is_enabled(lsm)) { > + lsm_enabled_set(lsm, false); > return; > } else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && lsm_exclusive) { > init_debug("exclusive disabled: %s\n", lsm->id->name); > - set_enabled(lsm, false); > + lsm_enabled_set(lsm, false); > return; > } > > /* Mark the LSM as enabled. */ > - set_enabled(lsm, true); > + lsm_enabled_set(lsm, true); > if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !lsm_exclusive) { > init_debug("exclusive chosen: %s\n", lsm->id->name); > lsm_exclusive = lsm; > @@ -200,7 +209,7 @@ static void __init lsm_prep_single(struct lsm_info *lsm) > /* Initialize a given LSM, if it is enabled. */ > static void __init initialize_lsm(struct lsm_info *lsm) > { > - if (is_enabled(lsm)) { > + if (lsm_is_enabled(lsm)) { > int ret; > > init_debug("initializing %s\n", lsm->id->name); > @@ -218,7 +227,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) > /* LSM_ORDER_FIRST is always first. */ > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { > if (lsm->order == LSM_ORDER_FIRST) > - append_ordered_lsm(lsm, " first"); > + lsm_order_append(lsm, " first"); > } > > /* Process "security=", if given. */ > @@ -235,7 +244,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) > major++) { > if ((major->flags & LSM_FLAG_LEGACY_MAJOR) && > strcmp(major->id->name, lsm_order_legacy) != 0) { > - set_enabled(major, false); > + lsm_enabled_set(major, false); > init_debug("security=%s disabled: %s (only one legacy major LSM)\n", > lsm_order_legacy, major->id->name); > } > @@ -251,7 +260,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { > if (strcmp(lsm->id->name, name) == 0) { > if (lsm->order == LSM_ORDER_MUTABLE) > - append_ordered_lsm(lsm, origin); > + lsm_order_append(lsm, origin); > found = true; > } > } > @@ -264,24 +273,24 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) > /* Process "security=", if given. */ > if (lsm_order_legacy) { > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { > - if (exists_ordered_lsm(lsm)) > + if (lsm_order_exists(lsm)) > continue; > if (strcmp(lsm->id->name, lsm_order_legacy) == 0) > - append_ordered_lsm(lsm, "security="); > + lsm_order_append(lsm, "security="); > } > } > > /* LSM_ORDER_LAST is always last. */ > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { > if (lsm->order == LSM_ORDER_LAST) > - append_ordered_lsm(lsm, " last"); > + lsm_order_append(lsm, " last"); > } > > /* Disable all LSMs not in the ordered list. */ > for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { > - if (exists_ordered_lsm(lsm)) > + if (lsm_order_exists(lsm)) > continue; > - set_enabled(lsm, false); > + lsm_enabled_set(lsm, false); > init_debug("%s skipped: %s (not in requested order)\n", > origin, lsm->id->name); > } > @@ -313,13 +322,13 @@ static void __init lsm_init_ordered(void) > > pr_info("initializing lsm="); > lsm_early_for_each_raw(early) { > - if (is_enabled(early)) > + if (lsm_is_enabled(early)) > pr_cont("%s%s", > early == __start_early_lsm_info ? "" : ",", > early->id->name); > } > lsm_order_for_each(lsm) { > - if (is_enabled(*lsm)) > + if (lsm_is_enabled(*lsm)) > pr_cont("%s%s", > lsm == lsm_order ? "" : ",", (*lsm)->id->name); > } > @@ -404,8 +413,7 @@ int __init early_security_init(void) > struct lsm_info *lsm; > > lsm_early_for_each_raw(lsm) { > - if (!lsm->enabled) > - lsm->enabled = &lsm_enabled_true; > + lsm_enabled_set(lsm, true); > lsm_prep_single(lsm); > initialize_lsm(lsm); > } > @@ -432,7 +440,7 @@ int __init security_init(void) > */ > lsm_early_for_each_raw(lsm) { > init_debug(" early started: %s (%s)\n", lsm->id->name, > - is_enabled(lsm) ? "enabled" : "disabled"); > + lsm_is_enabled(lsm) ? "enabled" : "disabled"); > } > > /* Load LSMs in specified order. */ > -- > 2.49.0 The simple renamings looks fine, but would be nicer if they got split out.
On Wed, Apr 9, 2025 at 8:11 PM Kees Cook <kees@kernel.org> wrote: > On Wed, Apr 09, 2025 at 02:49:54PM -0400, Paul Moore wrote: > > One part of a larger effort to cleanup the LSM framework initialization > > code. > > > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > --- > > security/inode.c | 9 ++-- > > security/lsm_init.c | 110 ++++++++++++++++++++++++-------------------- > > 2 files changed, 63 insertions(+), 56 deletions(-) > > > > diff --git a/security/inode.c b/security/inode.c > > index 49bc3578bd23..f687e22e6809 100644 > > --- a/security/inode.c > > +++ b/security/inode.c > > @@ -351,18 +351,17 @@ static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, > > > > for (i = 0; i < lsm_count; i++) > > /* the '+ 1' accounts for either a comma or a NUL terminator */ > > - len += strlen(lsm_order[i]->id->name) + 1; > > + len += strlen(lsm_idlist[i]->name) + 1; > > > > str = kmalloc(len, GFP_KERNEL); > > if (!str) > > return -ENOMEM; > > str[0] = '\0'; > > > > - i = 0; > > - while (i < lsm_count) { > > - strcat(str, lsm_order[i]->id->name); > > - if (++i < lsm_count) > > + for (i = 0; i < lsm_count; i++) { > > + if (i > 0) > > strcat(str, ","); > > + strcat(str, lsm_idlist[i]->name); > > } > > > > rc = simple_read_from_buffer(buf, count, ppos, str, len); > > This chunk needs to be folded into the lsm_names changing patch, I > think. I missed this on the first pass, but lsm_order can never be used > here because lsm_order is initdata -- it will be thrown away after init > is done. Yeah, I noticed this when I was reverting that /lsm_active_cnt/lsm_count/ change and fixed it up to use lsm_idlist[] which should address that problem. Later patches convert over to lsm_idlist[] anyway, which is likely why I didn't catch this in the preliminary testing. > > diff --git a/security/lsm_init.c b/security/lsm_init.c > > index 978bb81b58fa..7f2bc8c22ce9 100644 > > --- a/security/lsm_init.c > > +++ b/security/lsm_init.c > > @@ -10,6 +10,10 @@ > > > > #include "lsm.h" > > > > +/* LSM enabled constants. */ > > +int lsm_enabled_true = 1; > > +int lsm_enabled_false = 0; > > Why are these losing static and __initdata? It looks like they're > staying assigned to the __init-marked lsm_info instances. Good point. I'm not sure what happened here, it may have been a victim of an earlier change which I dropped. > > +/** > > + * lsm_is_enabled - Determine if a LSM is enabled > > + * @lsm: LSM definition > > + */ > > +static inline bool lsm_is_enabled(struct lsm_info *lsm) > > { > > if (!lsm->enabled) > > return false; > > - > > return *lsm->enabled; > > } > > This could be one-lined, actually: > > return lsm->enabled ? *lsm->enabled : false; Sure. > > -/* Is an LSM already listed in the ordered LSMs list? */ > > -static bool __init exists_ordered_lsm(struct lsm_info *lsm) > > +/** > > + * lsm_order_exists - Determine if a LSM exists in the ordered list > > + * @lsm: LSM definition > > + */ > > +static bool __init lsm_order_exists(struct lsm_info *lsm) > > { > > struct lsm_info **check; > > > > @@ -118,25 +123,29 @@ static bool __init exists_ordered_lsm(struct lsm_info *lsm) > > return false; > > } > > > > -/* Append an LSM to the list of ordered LSMs to initialize. */ > > -static int last_lsm __initdata; > > -static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) > > +/** > > + * lsm_order_append - Append a LSM to the ordered list > > + * @lsm: LSM definition > > + * @src: source of the addition > > + */ > > +static void __init lsm_order_append(struct lsm_info *lsm, const char *src) > > { > > /* Ignore duplicate selections. */ > > - if (exists_ordered_lsm(lsm)) > > + if (lsm_order_exists(lsm)) > > return; > > > > - if (WARN(last_lsm == MAX_LSM_COUNT, "%s: out of LSM static calls!?\n", from)) > > - return; > > + /* Skip explicitly disabled LSMs. */ > > + if (lsm->enabled && !lsm_is_enabled(lsm)) { > > + if (WARN(lsm_count == MAX_LSM_COUNT, > > + "%s: out of LSM static calls!?\n", src)) > > + return; > > + lsm_enabled_set(lsm, true); > > + lsm_order[lsm_count] = lsm; > > + lsm_idlist[lsm_count++] = lsm->id; > > + } > > > > - /* Enable this LSM, if it is not already set. */ > > - if (!lsm->enabled) > > - lsm->enabled = &lsm_enabled_true; > > - lsm_order[last_lsm] = lsm; > > - lsm_idlist[last_lsm++] = lsm->id; > > I don't understand the logic change here. I may be missing something (it > feels like a lot of logic changes mixed together again), but this logic: > > /* Enable this LSM, if it is not already set. */ > if (!lsm->enabled) > lsm->enabled = &lsm_enabled_true; > > seems like it has gone missing now? It's a little confusing as lsm_order_append() gets heavily reworked a couple of patches later in "lsm: cleanup the LSM ordered parsing", which is essentially this function's end state from a logic perspective. I think the best thing to do might be to squash those two patches ... lemme see how ugly that ends up ... > The simple renamings looks fine, but would be nicer if they got split > out. I can look into doing that, let me try the squashing first.
On Thu, Apr 10, 2025 at 9:50 PM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Apr 9, 2025 at 8:11 PM Kees Cook <kees@kernel.org> wrote: > > On Wed, Apr 09, 2025 at 02:49:54PM -0400, Paul Moore wrote: ... > > > -/* Append an LSM to the list of ordered LSMs to initialize. */ > > > -static int last_lsm __initdata; > > > -static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) > > > +/** > > > + * lsm_order_append - Append a LSM to the ordered list > > > + * @lsm: LSM definition > > > + * @src: source of the addition > > > + */ > > > +static void __init lsm_order_append(struct lsm_info *lsm, const char *src) > > > { > > > /* Ignore duplicate selections. */ > > > - if (exists_ordered_lsm(lsm)) > > > + if (lsm_order_exists(lsm)) > > > return; > > > > > > - if (WARN(last_lsm == MAX_LSM_COUNT, "%s: out of LSM static calls!?\n", from)) > > > - return; > > > + /* Skip explicitly disabled LSMs. */ > > > + if (lsm->enabled && !lsm_is_enabled(lsm)) { > > > + if (WARN(lsm_count == MAX_LSM_COUNT, > > > + "%s: out of LSM static calls!?\n", src)) > > > + return; > > > + lsm_enabled_set(lsm, true); > > > + lsm_order[lsm_count] = lsm; > > > + lsm_idlist[lsm_count++] = lsm->id; > > > + } > > > > > > - /* Enable this LSM, if it is not already set. */ > > > - if (!lsm->enabled) > > > - lsm->enabled = &lsm_enabled_true; > > > - lsm_order[last_lsm] = lsm; > > > - lsm_idlist[last_lsm++] = lsm->id; > > > > I don't understand the logic change here. I may be missing something (it > > feels like a lot of logic changes mixed together again), but this logic: > > > > /* Enable this LSM, if it is not already set. */ > > if (!lsm->enabled) > > lsm->enabled = &lsm_enabled_true; > > > > seems like it has gone missing now? > > It's a little confusing as lsm_order_append() gets heavily reworked a > couple of patches later in "lsm: cleanup the LSM ordered parsing", > which is essentially this function's end state from a logic > perspective. I think the best thing to do might be to squash those > two patches ... lemme see how ugly that ends up ... Yeah, it looks *way* better now with those two patches squashed.
On Thu, Apr 10, 2025 at 9:50 PM Paul Moore <paul@paul-moore.com> wrote: > On Wed, Apr 9, 2025 at 8:11 PM Kees Cook <kees@kernel.org> wrote: > > On Wed, Apr 09, 2025 at 02:49:54PM -0400, Paul Moore wrote: ... > > The simple renamings looks fine, but would be nicer if they got split > > out. > > I can look into doing that, let me try the squashing first. ... and I pulled out the enabled/disable setter/getter functions into their own patch.
On Thu, Apr 10, 2025 at 10:14:52PM -0400, Paul Moore wrote: > On Thu, Apr 10, 2025 at 9:50 PM Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Apr 9, 2025 at 8:11 PM Kees Cook <kees@kernel.org> wrote: > > > On Wed, Apr 09, 2025 at 02:49:54PM -0400, Paul Moore wrote: > > ... > > > > The simple renamings looks fine, but would be nicer if they got split > > > out. > > > > I can look into doing that, let me try the squashing first. > > ... and I pulled out the enabled/disable setter/getter functions into > their own patch. Awesome. My future review eyes thank you! :)
diff --git a/security/inode.c b/security/inode.c index 49bc3578bd23..f687e22e6809 100644 --- a/security/inode.c +++ b/security/inode.c @@ -351,18 +351,17 @@ static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, for (i = 0; i < lsm_count; i++) /* the '+ 1' accounts for either a comma or a NUL terminator */ - len += strlen(lsm_order[i]->id->name) + 1; + len += strlen(lsm_idlist[i]->name) + 1; str = kmalloc(len, GFP_KERNEL); if (!str) return -ENOMEM; str[0] = '\0'; - i = 0; - while (i < lsm_count) { - strcat(str, lsm_order[i]->id->name); - if (++i < lsm_count) + for (i = 0; i < lsm_count; i++) { + if (i > 0) strcat(str, ","); + strcat(str, lsm_idlist[i]->name); } rc = simple_read_from_buffer(buf, count, ppos, str, len); diff --git a/security/lsm_init.c b/security/lsm_init.c index 978bb81b58fa..7f2bc8c22ce9 100644 --- a/security/lsm_init.c +++ b/security/lsm_init.c @@ -10,6 +10,10 @@ #include "lsm.h" +/* LSM enabled constants. */ +int lsm_enabled_true = 1; +int lsm_enabled_false = 0; + /* Pointers to LSM sections defined in include/asm-generic/vmlinux.lds.h */ extern struct lsm_info __start_lsm_info[], __end_lsm_info[]; extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[]; @@ -72,41 +76,42 @@ static int __init lsm_debug_enable(char *str) } __setup("lsm.debug", lsm_debug_enable); -/* Mark an LSM's enabled flag. */ -static int lsm_enabled_true __initdata = 1; -static int lsm_enabled_false __initdata = 0; -static void __init set_enabled(struct lsm_info *lsm, bool enabled) +/** + * lsm_enabled_set - Mark a LSM as enabled + * @lsm: LSM definition + * @enabled: enabled flag + */ +static void __init lsm_enabled_set(struct lsm_info *lsm, bool enabled) { /* * When an LSM hasn't configured an enable variable, we can use * a hard-coded location for storing the default enabled state. */ - if (!lsm->enabled) { - if (enabled) - lsm->enabled = &lsm_enabled_true; - else - lsm->enabled = &lsm_enabled_false; - } else if (lsm->enabled == &lsm_enabled_true) { - if (!enabled) - lsm->enabled = &lsm_enabled_false; - } else if (lsm->enabled == &lsm_enabled_false) { - if (enabled) - lsm->enabled = &lsm_enabled_true; + if (!lsm->enabled || + lsm->enabled == &lsm_enabled_true || + lsm->enabled == &lsm_enabled_false) { + lsm->enabled = enabled ? &lsm_enabled_true : &lsm_enabled_false; } else { *lsm->enabled = enabled; } } -static inline bool is_enabled(struct lsm_info *lsm) +/** + * lsm_is_enabled - Determine if a LSM is enabled + * @lsm: LSM definition + */ +static inline bool lsm_is_enabled(struct lsm_info *lsm) { if (!lsm->enabled) return false; - return *lsm->enabled; } -/* Is an LSM already listed in the ordered LSMs list? */ -static bool __init exists_ordered_lsm(struct lsm_info *lsm) +/** + * lsm_order_exists - Determine if a LSM exists in the ordered list + * @lsm: LSM definition + */ +static bool __init lsm_order_exists(struct lsm_info *lsm) { struct lsm_info **check; @@ -118,25 +123,29 @@ static bool __init exists_ordered_lsm(struct lsm_info *lsm) return false; } -/* Append an LSM to the list of ordered LSMs to initialize. */ -static int last_lsm __initdata; -static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from) +/** + * lsm_order_append - Append a LSM to the ordered list + * @lsm: LSM definition + * @src: source of the addition + */ +static void __init lsm_order_append(struct lsm_info *lsm, const char *src) { /* Ignore duplicate selections. */ - if (exists_ordered_lsm(lsm)) + if (lsm_order_exists(lsm)) return; - if (WARN(last_lsm == MAX_LSM_COUNT, "%s: out of LSM static calls!?\n", from)) - return; + /* Skip explicitly disabled LSMs. */ + if (lsm->enabled && !lsm_is_enabled(lsm)) { + if (WARN(lsm_count == MAX_LSM_COUNT, + "%s: out of LSM static calls!?\n", src)) + return; + lsm_enabled_set(lsm, true); + lsm_order[lsm_count] = lsm; + lsm_idlist[lsm_count++] = lsm->id; + } - /* Enable this LSM, if it is not already set. */ - if (!lsm->enabled) - lsm->enabled = &lsm_enabled_true; - lsm_order[last_lsm] = lsm; - lsm_idlist[last_lsm++] = lsm->id; - - init_debug("%s ordered: %s (%s)\n", from, lsm->id->name, - is_enabled(lsm) ? "enabled" : "disabled"); + init_debug("%s ordered: %s (%s)\n", src, lsm->id->name, + lsm_is_enabled(lsm) ? "enabled" : "disabled"); } static void __init lsm_set_blob_size(int *need, int *lbs) @@ -159,17 +168,17 @@ static void __init lsm_prep_single(struct lsm_info *lsm) { struct lsm_blob_sizes *blobs; - if (!is_enabled(lsm)) { - set_enabled(lsm, false); + if (!lsm_is_enabled(lsm)) { + lsm_enabled_set(lsm, false); return; } else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && lsm_exclusive) { init_debug("exclusive disabled: %s\n", lsm->id->name); - set_enabled(lsm, false); + lsm_enabled_set(lsm, false); return; } /* Mark the LSM as enabled. */ - set_enabled(lsm, true); + lsm_enabled_set(lsm, true); if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !lsm_exclusive) { init_debug("exclusive chosen: %s\n", lsm->id->name); lsm_exclusive = lsm; @@ -200,7 +209,7 @@ static void __init lsm_prep_single(struct lsm_info *lsm) /* Initialize a given LSM, if it is enabled. */ static void __init initialize_lsm(struct lsm_info *lsm) { - if (is_enabled(lsm)) { + if (lsm_is_enabled(lsm)) { int ret; init_debug("initializing %s\n", lsm->id->name); @@ -218,7 +227,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) /* LSM_ORDER_FIRST is always first. */ for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { if (lsm->order == LSM_ORDER_FIRST) - append_ordered_lsm(lsm, " first"); + lsm_order_append(lsm, " first"); } /* Process "security=", if given. */ @@ -235,7 +244,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) major++) { if ((major->flags & LSM_FLAG_LEGACY_MAJOR) && strcmp(major->id->name, lsm_order_legacy) != 0) { - set_enabled(major, false); + lsm_enabled_set(major, false); init_debug("security=%s disabled: %s (only one legacy major LSM)\n", lsm_order_legacy, major->id->name); } @@ -251,7 +260,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { if (strcmp(lsm->id->name, name) == 0) { if (lsm->order == LSM_ORDER_MUTABLE) - append_ordered_lsm(lsm, origin); + lsm_order_append(lsm, origin); found = true; } } @@ -264,24 +273,24 @@ static void __init ordered_lsm_parse(const char *order, const char *origin) /* Process "security=", if given. */ if (lsm_order_legacy) { for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { - if (exists_ordered_lsm(lsm)) + if (lsm_order_exists(lsm)) continue; if (strcmp(lsm->id->name, lsm_order_legacy) == 0) - append_ordered_lsm(lsm, "security="); + lsm_order_append(lsm, "security="); } } /* LSM_ORDER_LAST is always last. */ for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { if (lsm->order == LSM_ORDER_LAST) - append_ordered_lsm(lsm, " last"); + lsm_order_append(lsm, " last"); } /* Disable all LSMs not in the ordered list. */ for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) { - if (exists_ordered_lsm(lsm)) + if (lsm_order_exists(lsm)) continue; - set_enabled(lsm, false); + lsm_enabled_set(lsm, false); init_debug("%s skipped: %s (not in requested order)\n", origin, lsm->id->name); } @@ -313,13 +322,13 @@ static void __init lsm_init_ordered(void) pr_info("initializing lsm="); lsm_early_for_each_raw(early) { - if (is_enabled(early)) + if (lsm_is_enabled(early)) pr_cont("%s%s", early == __start_early_lsm_info ? "" : ",", early->id->name); } lsm_order_for_each(lsm) { - if (is_enabled(*lsm)) + if (lsm_is_enabled(*lsm)) pr_cont("%s%s", lsm == lsm_order ? "" : ",", (*lsm)->id->name); } @@ -404,8 +413,7 @@ int __init early_security_init(void) struct lsm_info *lsm; lsm_early_for_each_raw(lsm) { - if (!lsm->enabled) - lsm->enabled = &lsm_enabled_true; + lsm_enabled_set(lsm, true); lsm_prep_single(lsm); initialize_lsm(lsm); } @@ -432,7 +440,7 @@ int __init security_init(void) */ lsm_early_for_each_raw(lsm) { init_debug(" early started: %s (%s)\n", lsm->id->name, - is_enabled(lsm) ? "enabled" : "disabled"); + lsm_is_enabled(lsm) ? "enabled" : "disabled"); } /* Load LSMs in specified order. */
One part of a larger effort to cleanup the LSM framework initialization code. Signed-off-by: Paul Moore <paul@paul-moore.com> --- security/inode.c | 9 ++-- security/lsm_init.c | 110 ++++++++++++++++++++++++-------------------- 2 files changed, 63 insertions(+), 56 deletions(-)