Re: [PATCH v4 04/34] lsm: introduce looping macros for the initialization code

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

On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> There are three common for loop patterns in the LSM initialization code
> to loop through the ordered LSM list and the registered "early" LSMs.
> This patch implements these loop patterns as macros to help simplify the
> code and reduce the chance for errors.
> 
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

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

> ---
>  security/lsm_init.c | 42 +++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 6f40ab1d2f54..18828a65c364 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -32,6 +32,15 @@ static __initdata bool debug;
>  			pr_info(__VA_ARGS__);				\
>  	} while (0)
>  
> +#define lsm_order_for_each(iter)					\
> +	for ((iter) = ordered_lsms; *(iter); (iter)++)
> +#define lsm_for_each_raw(iter)						\
> +	for ((iter) = __start_lsm_info;					\
> +	     (iter) < __end_lsm_info; (iter)++)
> +#define lsm_early_for_each_raw(iter)					\
> +	for ((iter) = __start_early_lsm_info;				\
> +	     (iter) < __end_early_lsm_info; (iter)++)
> +
>  static int lsm_append(const char *new, char **result);
>  
>  /* Save user chosen LSM */
> @@ -96,9 +105,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;
>  }
> @@ -209,7 +219,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	char *sep, *name, *next;
>  
>  	/* LSM_ORDER_FIRST is always first. */
> -	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> +	lsm_for_each_raw(lsm) {
>  		if (lsm->order == LSM_ORDER_FIRST)
>  			append_ordered_lsm(lsm, "  first");
>  	}
> @@ -224,8 +234,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  		 * if the selected one was separately disabled: disable
>  		 * all non-matching Legacy Major LSMs.
>  		 */
> -		for (major = __start_lsm_info; major < __end_lsm_info;
> -		     major++) {
> +		lsm_for_each_raw(major) {
>  			if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&
>  			    strcmp(major->name, chosen_major_lsm) != 0) {
>  				set_enabled(major, false);
> @@ -241,7 +250,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	while ((name = strsep(&next, ",")) != NULL) {
>  		bool found = false;
>  
> -		for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> +		lsm_for_each_raw(lsm) {
>  			if (strcmp(lsm->name, name) == 0) {
>  				if (lsm->order == LSM_ORDER_MUTABLE)
>  					append_ordered_lsm(lsm, origin);
> @@ -256,7 +265,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  
>  	/* Process "security=", if given. */
>  	if (chosen_major_lsm) {
> -		for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> +		lsm_for_each_raw(lsm) {
>  			if (exists_ordered_lsm(lsm))
>  				continue;
>  			if (strcmp(lsm->name, chosen_major_lsm) == 0)
> @@ -265,13 +274,13 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	}
>  
>  	/* LSM_ORDER_LAST is always last. */
> -	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> +	lsm_for_each_raw(lsm) {
>  		if (lsm->order == LSM_ORDER_LAST)
>  			append_ordered_lsm(lsm, "   last");
>  	}
>  
>  	/* Disable all LSMs not in the ordered list. */
> -	for (lsm = __start_lsm_info; lsm < __end_lsm_info; lsm++) {
> +	lsm_for_each_raw(lsm) {
>  		if (exists_ordered_lsm(lsm))
>  			continue;
>  		set_enabled(lsm, false);
> @@ -290,13 +299,14 @@ static void __init report_lsm_order(void)
>  	pr_info("initializing lsm=");
>  
>  	/* Report each enabled LSM name, comma separated. */
> -	for (early = __start_early_lsm_info;
> -	     early < __end_early_lsm_info; early++)
> +	lsm_early_for_each_raw(early) {
>  		if (is_enabled(early))
>  			pr_cont("%s%s", first++ == 0 ? "" : ",", early->name);
> -	for (lsm = ordered_lsms; *lsm; lsm++)
> +	}
> +	lsm_order_for_each(lsm) {
>  		if (is_enabled(*lsm))
>  			pr_cont("%s%s", first++ == 0 ? "" : ",", (*lsm)->name);
> +	}
>  
>  	pr_cont("\n");
>  }
> @@ -343,8 +353,9 @@ 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_prepare(*lsm);
> +	}
>  
>  	report_lsm_order();
>  
> @@ -382,8 +393,9 @@ static void __init ordered_lsm_init(void)
>  
>  	lsm_early_cred((struct cred *) current->cred);
>  	lsm_early_task(current);
> -	for (lsm = ordered_lsms; *lsm; lsm++)
> +	lsm_order_for_each(lsm) {
>  		initialize_lsm(*lsm);
> +	}
>  }
>  
>  static bool match_last_lsm(const char *list, const char *lsm)
> @@ -485,7 +497,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_prepare(lsm);
> @@ -512,7 +524,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)