Re: [PATCH v4 08/34] lsm: replace the name field with a pointer to the lsm_id struct

[Mimi Zohar <zohar@linux.ibm.com>]

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> Reduce the duplication between the lsm_id struct and the DEFINE_LSM()
> definition by linking the lsm_id struct directly into the individual
> LSM's DEFINE_LSM() instance.
> 
> Linking the lsm_id into the LSM definition also allows us to simplify
> the security_add_hooks() function by removing the code which populates
> the lsm_idlist[] array and moving it into the normal LSM startup code
> where the LSM list is parsed and the individual LSMs are enabled,
> making for a cleaner implementation with less overhead at boot.
> 
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  include/linux/lsm_hooks.h         |  2 +-
>  security/apparmor/lsm.c           |  2 +-
>  security/bpf/hooks.c              |  2 +-
>  security/commoncap.c              |  2 +-
>  security/integrity/evm/evm_main.c |  2 +-
>  security/integrity/ima/ima_main.c |  2 +-
>  security/ipe/ipe.c                |  2 +-
>  security/landlock/setup.c         |  2 +-
>  security/loadpin/loadpin.c        |  2 +-
>  security/lockdown/lockdown.c      |  2 +-
>  security/lsm_init.c               | 45 +++++++++++++------------------
>  security/safesetid/lsm.c          |  2 +-
>  security/selinux/hooks.c          |  2 +-
>  security/smack/smack_lsm.c        |  2 +-
>  security/tomoyo/tomoyo.c          |  2 +-
>  security/yama/yama_lsm.c          |  2 +-
>  16 files changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 0112926ed923..7343dd60b1d5 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -152,7 +152,7 @@ enum lsm_order {
>  };
>  
>  struct lsm_info {
> -	const char *name;	/* Required. */
> +	const struct lsm_id *id;
>  	enum lsm_order order;	/* Optional: default is LSM_ORDER_MUTABLE */
>  	unsigned long flags;	/* Optional: flags describing LSM */
>  	int *enabled;		/* Optional: controlled by CONFIG_LSM */
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 8e1cc229b41b..45b3a304d525 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -2552,7 +2552,7 @@ static int __init apparmor_init(void)
>  }
>  
>  DEFINE_LSM(apparmor) = {
> -	.name = "apparmor",
> +	.id = &apparmor_lsmid,
>  	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
>  	.enabled = &apparmor_enabled,
>  	.blobs = &apparmor_blob_sizes,
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> index db759025abe1..40efde233f3a 100644
> --- a/security/bpf/hooks.c
> +++ b/security/bpf/hooks.c
> @@ -33,7 +33,7 @@ struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = {
>  };
>  
>  DEFINE_LSM(bpf) = {
> -	.name = "bpf",
> +	.id = &bpf_lsmid,
>  	.init = bpf_lsm_init,
>  	.blobs = &bpf_lsm_blob_sizes
>  };
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6bd4adeb4795..b50479bd0286 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1505,7 +1505,7 @@ static int __init capability_init(void)
>  }
>  
>  DEFINE_LSM(capability) = {
> -	.name = "capability",
> +	.id = &capability_lsmid,
>  	.order = LSM_ORDER_FIRST,
>  	.init = capability_init,
>  };
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 0add782e73ba..db8e324ed4e6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -1175,7 +1175,7 @@ struct lsm_blob_sizes evm_blob_sizes __ro_after_init = {
>  };
>  
>  DEFINE_LSM(evm) = {
> -	.name = "evm",
> +	.id = &evm_lsmid,
>  	.init = init_evm_lsm,
>  	.order = LSM_ORDER_LAST,
>  	.blobs = &evm_blob_sizes,
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index cdd225f65a62..eade8e1e3cb1 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1279,7 +1279,7 @@ struct lsm_blob_sizes ima_blob_sizes __ro_after_init = {
>  };
>  
>  DEFINE_LSM(ima) = {
> -	.name = "ima",
> +	.id = &ima_lsmid,
>  	.init = init_ima_lsm,
>  	.order = LSM_ORDER_LAST,
>  	.blobs = &ima_blob_sizes,
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 4317134cb0da..2426441181dc 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -92,7 +92,7 @@ static int __init ipe_init(void)
>  }
>  
>  DEFINE_LSM(ipe) = {
> -	.name = "ipe",
> +	.id = &ipe_lsmid,
>  	.init = ipe_init,
>  	.blobs = &ipe_blobs,
>  };
> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
> index bd53c7a56ab9..47dac1736f10 100644
> --- a/security/landlock/setup.c
> +++ b/security/landlock/setup.c
> @@ -75,7 +75,7 @@ static int __init landlock_init(void)
>  }
>  
>  DEFINE_LSM(LANDLOCK_NAME) = {
> -	.name = LANDLOCK_NAME,
> +	.id = &landlock_lsmid,
>  	.init = landlock_init,
>  	.blobs = &landlock_blob_sizes,
>  };
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 68252452b66c..b9ddf05c5c16 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -271,7 +271,7 @@ static int __init loadpin_init(void)
>  }
>  
>  DEFINE_LSM(loadpin) = {
> -	.name = "loadpin",
> +	.id = &loadpin_lsmid,
>  	.init = loadpin_init,
>  };
>  
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index cf83afa1d879..4813f168ff93 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -168,6 +168,6 @@ DEFINE_EARLY_LSM(lockdown) = {
>  #else
>  DEFINE_LSM(lockdown) = {
>  #endif
> -	.name = "lockdown",
> +	.id = &lockdown_lsmid,
>  	.init = lockdown_lsm_init,
>  };
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 4a108b03c23d..628f54790360 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -127,9 +127,10 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)
>  	/* Enable this LSM, if it is not already set. */
>  	if (!lsm->enabled)
>  		lsm->enabled = &lsm_enabled_true;
> -	ordered_lsms[last_lsm++] = lsm;
> +	ordered_lsms[last_lsm] = lsm;
> +	lsm_idlist[last_lsm++] = lsm->id;
>  
> -	init_debug("%s ordered: %s (%s)\n", from, lsm->name,
> +	init_debug("%s ordered: %s (%s)\n", from, lsm->id->name,
>  		   is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
> @@ -157,7 +158,7 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  		set_enabled(lsm, false);
>  		return;
>  	} else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && exclusive) {
> -		init_debug("exclusive disabled: %s\n", lsm->name);
> +		init_debug("exclusive disabled: %s\n", lsm->id->name);
>  		set_enabled(lsm, false);
>  		return;
>  	}
> @@ -165,7 +166,7 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  	/* Mark the LSM as enabled. */
>  	set_enabled(lsm, true);
>  	if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !exclusive) {
> -		init_debug("exclusive chosen:   %s\n", lsm->name);
> +		init_debug("exclusive chosen:   %s\n", lsm->id->name);
>  		exclusive = lsm;
>  	}
>  
> @@ -200,9 +201,9 @@ static void __init initialize_lsm(struct lsm_info *lsm)
>  	if (is_enabled(lsm)) {
>  		int ret;
>  
> -		init_debug("initializing %s\n", lsm->name);
> +		init_debug("initializing %s\n", lsm->id->name);
>  		ret = lsm->init();
> -		WARN(ret, "%s failed to initialize: %d\n", lsm->name, ret);
> +		WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
>  	}
>  }
>  
> @@ -236,10 +237,10 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  		 */
>  		lsm_for_each_raw(major) {
>  			if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&
> -			    strcmp(major->name, chosen_major_lsm) != 0) {
> +			    strcmp(major->id->name, chosen_major_lsm) != 0) {
>  				set_enabled(major, false);
>  				init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> -					   chosen_major_lsm, major->name);
> +					   chosen_major_lsm, major->id->name);
>  			}
>  		}
>  	}
> @@ -251,7 +252,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  		bool found = false;
>  
>  		lsm_for_each_raw(lsm) {
> -			if (strcmp(lsm->name, name) == 0) {
> +			if (strcmp(lsm->id->name, name) == 0) {
>  				if (lsm->order == LSM_ORDER_MUTABLE)
>  					append_ordered_lsm(lsm, origin);
>  				found = true;
> @@ -268,7 +269,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  		lsm_for_each_raw(lsm) {
>  			if (exists_ordered_lsm(lsm))
>  				continue;
> -			if (strcmp(lsm->name, chosen_major_lsm) == 0)
> +			if (strcmp(lsm->id->name, chosen_major_lsm) == 0)
>  				append_ordered_lsm(lsm, "security=");
>  		}
>  	}
> @@ -285,7 +286,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  			continue;
>  		set_enabled(lsm, false);
>  		init_debug("%s skipped: %s (not in requested order)\n",
> -			   origin, lsm->name);
> +			   origin, lsm->id->name);
>  	}
>  
>  	kfree(sep);
> @@ -317,11 +318,13 @@ static void __init lsm_init_ordered(void)
>  	pr_info("initializing lsm=");
>  	lsm_early_for_each_raw(early) {
>  		if (is_enabled(early))
> -			pr_cont("%s%s", first++ == 0 ? "" : ",", early->name);
> +			pr_cont("%s%s",
> +				first++ == 0 ? "" : ",", early->id->name);
>  	}
>  	lsm_order_for_each(lsm) {
>  		if (is_enabled(*lsm))
> -			pr_cont("%s%s", first++ == 0 ? "" : ",", (*lsm)->name);
> +			pr_cont("%s%s",
> +				first++ == 0 ? "" : ",", (*lsm)->id->name);
>  	}
>  	pr_cont("\n");
>  
> @@ -432,18 +435,6 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  {
>  	int i;
>  
> -	/*
> -	 * A security module may call security_add_hooks() more
> -	 * than once during initialization, and LSM initialization
> -	 * is serialized. Landlock is one such case.
> -	 * Look at the previous entry, if there is one, for duplication.
> -	 */
> -	if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid) {
> -		if (lsm_active_cnt >= MAX_LSM_COUNT)
> -			panic("%s Too many LSMs registered.\n", __func__);
> -		lsm_idlist[lsm_active_cnt++] = lsmid;
> -	}
> -
>  	for (i = 0; i < count; i++) {
>  		hooks[i].lsmid = lsmid;
>  		lsm_static_call_init(&hooks[i]);
> @@ -491,10 +482,10 @@ int __init security_init(void)
>  	 * available
>  	 */
>  	lsm_early_for_each_raw(lsm) {
> -		init_debug("  early started: %s (%s)\n", lsm->name,
> +		init_debug("  early started: %s (%s)\n", lsm->id->name,
>  			   is_enabled(lsm) ? "enabled" : "disabled");
>  		if (lsm->enabled)
> -			lsm_append(lsm->name, &lsm_names);
> +			lsm_append(lsm->id->name, &lsm_names);
>  	}
>  
>  	/* Load LSMs in specified order. */
> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
> index 1ba564f097f5..9a7c68d4e642 100644
> --- a/security/safesetid/lsm.c
> +++ b/security/safesetid/lsm.c
> @@ -287,6 +287,6 @@ static int __init safesetid_security_init(void)
>  }
>  
>  DEFINE_LSM(safesetid_security_init) = {
> +	.id = &safesetid_lsmid,
>  	.init = safesetid_security_init,
> -	.name = "safesetid",
>  };
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4da5e792b42e..d94b1ff316ba 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7630,7 +7630,7 @@ void selinux_complete_init(void)
>  /* SELinux requires early initialization in order to label
>     all processes and objects when they are created. */
>  DEFINE_LSM(selinux) = {
> -	.name = "selinux",
> +	.id = &selinux_lsmid,
>  	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
>  	.enabled = &selinux_enabled_boot,
>  	.blobs = &selinux_blob_sizes,
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index fc340a6f0dde..e09490c75f59 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -5275,7 +5275,7 @@ static __init int smack_init(void)
>   * all processes and objects when they are created.
>   */
>  DEFINE_LSM(smack) = {
> -	.name = "smack",
> +	.id = &smack_lsmid,
>  	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
>  	.blobs = &smack_blob_sizes,
>  	.init = smack_init,
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index d6ebcd9db80a..ed0f7b052a85 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -612,7 +612,7 @@ static int __init tomoyo_init(void)
>  }
>  
>  DEFINE_LSM(tomoyo) = {
> -	.name = "tomoyo",
> +	.id = &tomoyo_lsmid,
>  	.enabled = &tomoyo_enabled,
>  	.flags = LSM_FLAG_LEGACY_MAJOR,
>  	.blobs = &tomoyo_blob_sizes,
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index 3d064dd4e03f..38b21ee0c560 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -476,6 +476,6 @@ static int __init yama_init(void)
>  }
>  
>  DEFINE_LSM(yama) = {
> -	.name = "yama",
> +	.id = &yama_lsmid,
>  	.init = yama_init,
>  };