Re: [RFC PATCH 17/29] lsm: introduce an initcall mechanism into the LSM framework

[Kees Cook <kees@kernel.org>]

On Wed, Apr 09, 2025 at 02:50:02PM -0400, Paul Moore wrote:
> Currently the individual LSMs register their own initcalls, and while
> this should be harmless, it can be wasteful in the case where a LSM
> is disabled at boot as the initcall will still be executed.  This
> patch introduces support for managing the initcalls in the LSM
> framework, and future patches will convert the existing LSMs over to
> this new mechanism.
> 
> Only initcall types which are used by the current in-tree LSMs are
> supported, additional initcall types can easily be added in the future
> if needed.
> 
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  include/linux/lsm_hooks.h | 33 ++++++++++++---
>  security/lsm_init.c       | 89 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a7ecb0791a0f..0d2c2a017ffc 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -148,13 +148,36 @@ enum lsm_order {
>  	LSM_ORDER_LAST = 1,	/* This is only for integrity. */
>  };
>  
> +/**
> + * struct lsm_info - Define an individual LSM for the LSM framework.
> + * @id: LSM name/ID info
> + * @order: ordering with respect to other LSMs, optional
> + * @flags: descriptive flags, optional
> + * @blobs: LSM blob sharing, optional
> + * @enabled: controlled by CONFIG_LSM, optional
> + * @init: LSM specific initialization routine
> + * @initcall_pure: LSM callback for initcall_pure() setup, optional
> + * @initcall_early: LSM callback for early_initcall setup, optional
> + * @initcall_core: LSM callback for core_initcall() setup, optional
> + * @initcall_subsys: LSM callback for subsys_initcall() setup, optional
> + * @initcall_fs: LSM callback for fs_initcall setup, optional
> + * @nitcall_device: LSM callback for device_initcall() setup, optional
> + * @initcall_late: LSM callback for late_initcall() setup, optional
> + */

Yay! Proper kerndoc. :)

>  struct lsm_info {
>  	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 */
> -	int (*init)(void);	/* Required. */
> -	struct lsm_blob_sizes *blobs; /* Optional: for blob sharing. */
> +	enum lsm_order order;
> +	unsigned long flags;
> +	struct lsm_blob_sizes *blobs;
> +	int *enabled;
> +	int (*init)(void);
> +	int (*initcall_pure)(void);
> +	int (*initcall_early)(void);
> +	int (*initcall_core)(void);
> +	int (*initcall_subsys)(void);
> +	int (*initcall_fs)(void);
> +	int (*initcall_device)(void);
> +	int (*initcall_late)(void);
>  };
>  
>  #define DEFINE_LSM(lsm)							\
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 8e00afeb84cf..75eb0cc82869 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -39,6 +39,27 @@ static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
>  	for ((iter) = __start_early_lsm_info;				\
>  	     (iter) < __end_early_lsm_info; (iter)++)
>  
> +#define lsm_initcall(level)						\
> +	({ 								\
> +		int _r, _rc = 0;					\
> +		struct lsm_info **_lp, *_l; 				\
> +		lsm_order_for_each(_lp) { 				\
> +			_l = *_lp; 					\
> +			if (!_l->initcall_##level) 			\
> +				continue;				\
> +			lsm_pr_dbg("running %s %s initcall",		\
> +				   _l->id->name, #level);		\
> +			_r = _l->initcall_##level();			\
> +			if (_r) {					\
> +				pr_warn("failed LSM %s %s initcall with errno %d\n", \
> +					_l->id->name, #level, _r);	\
> +				if (!_rc)				\
> +					_rc = _r;			\
> +			}						\
> +		}							\
> +		_rc;							\
> +	})
> +
>  /**
>   * lsm_choose_security - Legacy "major" LSM selection
>   * @str: kernel command line parameter
> @@ -458,3 +479,71 @@ int __init security_init(void)
>  
>  	return 0;
>  }
> +
> +/**
> + * security_initcall_pure - Run the LSM pure initcalls
> + */
> +static int __init security_initcall_pure(void)
> +{
> +	return lsm_initcall(pure);
> +}
> +pure_initcall(security_initcall_pure);
> +
> +/**
> + * security_initcall_early - Run the LSM early initcalls
> + */
> +static int __init security_initcall_early(void)
> +{
> +	return lsm_initcall(early);
> +}
> +early_initcall(security_initcall_early);
> +
> +/**
> + * security_initcall_core - Run the LSM core initcalls
> + */
> +static int __init security_initcall_core(void)
> +{
> +	return lsm_initcall(core);
> +}
> +core_initcall(security_initcall_core);
> +
> +/**
> + * security_initcall_subsys - Run the LSM subsys initcalls
> + */
> +static int __init security_initcall_subsys(void)
> +{
> +	return lsm_initcall(subsys);
> +}
> +subsys_initcall(security_initcall_subsys);
> +
> +/**
> + * security_initcall_fs - Run the LSM fs initcalls
> + */
> +static int __init security_initcall_fs(void)
> +{
> +	return lsm_initcall(fs);
> +}
> +fs_initcall(security_initcall_fs);
> +
> +/**
> + * security_initcall_device - Run the LSM device initcalls
> + */
> +static int __init security_initcall_device(void)
> +{
> +	return lsm_initcall(device);
> +}
> +device_initcall(security_initcall_device);
> +
> +/**
> + * security_initcall_late - Run the LSM late initcalls
> + */
> +static int __init security_initcall_late(void)
> +{
> +	int rc;
> +
> +	rc = lsm_initcall(late);
> +	lsm_pr_dbg("all enabled LSMs fully activated\n");
> +
> +	return rc;
> +}
> +late_initcall(security_initcall_late);

You'd need a new place for the lsm_pr_dbg, but these are all just
copy/paste. These could be macro-ified too?

#define define_lsm_initcall(level)			\
static int __init security_initcall_##level(void)	\
{							\
	return lsm_initcall(level);			\
}							\
level##_initcall(security_initcall_##level)

define_lsm_initcall(pure);
define_lsm_initcall(early);
define_lsm_initcall(core);
define_lsm_initcall(subsys);
define_lsm_initcall(fs);
define_lsm_initcall(device);
define_lsm_initcall(late);

I'm not sure exposing the kerndoc for them is worth open-coding them?

And, actually, it's just a macro calling a macro. You could just combine
them? i.e. turn lsm_initcall() into:

#define define_lsm_initcall(level)				\
static int __init security_initcall_##level(void)		\
{	 							\
	int _r, _rc = 0;					\
	struct lsm_info **_lp, *_l; 				\
	...
	return _rc;						\
}								\
level##_initcall(security_initcall_##level)


But, I like it either way.

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook