On Wed, Apr 09, 2025 at 02:49:49PM -0400, Paul Moore wrote:
> One part of a larger effort to cleanup the LSM framework initialization
> code.
Again, needs a better commit log.
>
> Signed-off-by: Paul Moore <p...@paul-moore.com>
> ---
> security/lsm_init.c | 94 +++++++++++++++++----------------------------
> 1 file changed, 36 insertions(+), 58 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index dffa8dc2da36..407429688f1b 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -32,6 +32,12 @@ static __initdata bool debug;
> pr_info(__VA_ARGS__); \
> } while (0)
>
> +#define lsm_order_for_each(iter) \
> + for ((iter) = ordered_lsms; *(iter); (iter)++)
> +#define lsm_early_for_each_raw(iter) \
> + for ((iter) = __start_early_lsm_info; \
> + (iter) < __end_early_lsm_info; (iter)++)
The longer I look at this patch the longer I think it needs to be broken
up into a few separate patches, but they would be relatively small, like
this one: replace iter loops with iter macros.
> +
> static int lsm_append(const char *new, char **result);
>
> /* Save user chosen LSM */
> @@ -96,9 +102,10 @@ static bool __init exists_ordered_lsm(struct lsm_info
> *lsm)
> {
> struct lsm_info **check;
>
> - for (check = ordered_lsms; *check; check++)
> + lsm_order_for_each(check) {
> if (*check == lsm)
> return true;
> + }
>
> return false;
> }
> @@ -279,56 +286,13 @@ static void __init ordered_lsm_parse(const char *order,
> const char *origin)
> kfree(sep);
> }
>
> -static void __init report_lsm_order(void)
> -{
> - struct lsm_info **lsm, *early;
> - int first = 0;
> -
> - pr_info("initializing lsm=");
> -
> - /* Report each enabled LSM name, comma separated. */
> - for (early = __start_early_lsm_info;
> - early < __end_early_lsm_info; early++)
> - if (is_enabled(early))
> - pr_cont("%s%s", first++ == 0 ? "" : ",", early->name);
> - for (lsm = ordered_lsms; *lsm; lsm++)
> - if (is_enabled(*lsm))
> - pr_cont("%s%s", first++ == 0 ? "" : ",", (*lsm)->name);
> -
> - pr_cont("\n");
> -}
> -
> /**
> - * lsm_early_cred - during initialization allocate a composite cred blob
> - * @cred: the cred that needs a blob
> - *
> - * Allocate the cred blob for all the modules
> + * lsm_init_ordered - Initialize the ordered LSMs
> */
> -static void __init lsm_early_cred(struct cred *cred)
> -{
> - int rc = lsm_cred_alloc(cred, GFP_KERNEL);
> -
> - if (rc)
> - panic("%s: Early cred alloc failed.\n", __func__);
> -}
> -
> -/**
> - * lsm_early_task - during initialization allocate a composite task blob
> - * @task: the task that needs a blob
> - *
> - * Allocate the task blob for all the modules
> - */
> -static void __init lsm_early_task(struct task_struct *task)
> -{
> - int rc = lsm_task_alloc(task);
> -
> - if (rc)
> - panic("%s: Early task alloc failed.\n", __func__);
> -}
> -
> -static void __init ordered_lsm_init(void)
> +static void __init lsm_init_ordered(void)
> {
> struct lsm_info **lsm;
> + struct lsm_info *early;
>
> if (chosen_lsm_order) {
> if (chosen_major_lsm) {
> @@ -340,10 +304,23 @@ static void __init ordered_lsm_init(void)
> } else
> ordered_lsm_parse(builtin_lsm_order, "builtin");
>
> - for (lsm = ordered_lsms; *lsm; lsm++)
> + lsm_order_for_each(lsm) {
> lsm_prep_single(*lsm);
> + }
>
> - report_lsm_order();
> + pr_info("initializing lsm=");
> + lsm_early_for_each_raw(early) {
> + if (is_enabled(early))
> + pr_cont("%s%s",
> + early == __start_early_lsm_info ? "" : ",",
> + early->name);
> + }
> + lsm_order_for_each(lsm) {
> + if (is_enabled(*lsm))
> + pr_cont("%s%s",
> + lsm == ordered_lsms ? "" : ",", (*lsm)->name);
> + }
report_lsm_order()'s use of "first" needs to stay here or you don't get
the right comma/no-comma behavior. It's not about the lsm, it's about
whether "first" got incremented. Perhaps "count" might be a better name
for "first".
> + pr_cont("\n");
>
> init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> @@ -362,9 +339,6 @@ static void __init ordered_lsm_init(void)
> init_debug("xattr slots = %d\n", blob_sizes.lbs_xattr_count);
> init_debug("bdev blob size = %d\n", blob_sizes.lbs_bdev);
>
> - /*
> - * Create any kmem_caches needed for blobs
> - */
> if (blob_sizes.lbs_file)
> lsm_file_cache = kmem_cache_create("lsm_file_cache",
> blob_sizes.lbs_file, 0,
> @@ -374,10 +348,14 @@ static void __init ordered_lsm_init(void)
> blob_sizes.lbs_inode, 0,
> SLAB_PANIC, NULL);
>
> - lsm_early_cred((struct cred *) current->cred);
> - lsm_early_task(current);
> - for (lsm = ordered_lsms; *lsm; lsm++)
> + if (lsm_cred_alloc((struct cred *)current->cred, GFP_KERNEL))
> + panic("%s: early cred alloc failed.\n", __func__);
> + if (lsm_task_alloc(current))
> + panic("%s: early task alloc failed.\n", __func__);
> +
> + lsm_order_for_each(lsm) {
> initialize_lsm(*lsm);
> + }
> }
>
> static bool match_last_lsm(const char *list, const char *lsm)
> @@ -479,7 +457,7 @@ int __init early_security_init(void)
> {
> struct lsm_info *lsm;
>
> - for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + lsm_early_for_each_raw(lsm) {
> if (!lsm->enabled)
> lsm->enabled = &lsm_enabled_true;
> lsm_prep_single(lsm);
> @@ -506,7 +484,7 @@ int __init security_init(void)
> * Append the names of the early LSM modules now that kmalloc() is
> * available
> */
> - for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + lsm_early_for_each_raw(lsm) {
> init_debug(" early started: %s (%s)\n", lsm->name,
> is_enabled(lsm) ? "enabled" : "disabled");
> if (lsm->enabled)
> @@ -514,7 +492,7 @@ int __init security_init(void)
> }
>
> /* Load LSMs in specified order. */
> - ordered_lsm_init();
> + lsm_init_ordered();
>
> return 0;
> }
> --
> 2.49.0
>
Other stuff seems good.
--
Kees Cook
