Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [RFC PATCH v2 33/34] lsm: consolidate all of the LSM framework initcalls
From: Casey Schaufler @ 2025-07-24 23:37 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-69-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> The LSM framework itself registers a small number of initcalls, this
> patch converts these initcalls into the new initcall mechanism.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/inode.c    |  3 +--
>  security/lsm.h      |  4 ++++
>  security/lsm_init.c | 14 ++++++++++++--
>  security/min_addr.c |  5 +++--
>  4 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index 68ee6c9de833..d15a0b0f4b14 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -408,7 +408,7 @@ static const struct file_operations lsm_ops = {
>  };
>  #endif
>  
> -static int __init securityfs_init(void)
> +int __init securityfs_init(void)
>  {
>  	int retval;
>  
> @@ -427,4 +427,3 @@ static int __init securityfs_init(void)
>  #endif
>  	return 0;
>  }
> -core_initcall(securityfs_init);
> diff --git a/security/lsm.h b/security/lsm.h
> index 8dc267977ae0..436219260376 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -35,4 +35,8 @@ extern struct kmem_cache *lsm_inode_cache;
>  int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
>  int lsm_task_alloc(struct task_struct *task);
>  
> +/* LSM framework initializers */
> +int securityfs_init(void);
> +int min_addr_init(void);
> +
>  #endif /* _LSM_H_ */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index ab739f9c2244..f178a9a2f9d4 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -482,7 +482,12 @@ int __init security_init(void)
>   */
>  static int __init security_initcall_pure(void)
>  {
> -	return lsm_initcall(pure);
> +	int rc_adr, rc_lsm;
> +
> +	rc_adr = min_addr_init();
> +	rc_lsm = lsm_initcall(pure);
> +
> +	return (rc_adr ? rc_adr : rc_lsm);
>  }
>  pure_initcall(security_initcall_pure);
>  
> @@ -500,7 +505,12 @@ early_initcall(security_initcall_early);
>   */
>  static int __init security_initcall_core(void)
>  {
> -	return lsm_initcall(core);
> +	int rc_sfs, rc_lsm;
> +
> +	rc_sfs = securityfs_init();
> +	rc_lsm = lsm_initcall(core);
> +
> +	return (rc_sfs ? rc_sfs : rc_lsm);
>  }
>  core_initcall(security_initcall_core);
>  
> diff --git a/security/min_addr.c b/security/min_addr.c
> index df1bc643d886..40714bdeefbe 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -4,6 +4,8 @@
>  #include <linux/security.h>
>  #include <linux/sysctl.h>
>  
> +#include "lsm.h"
> +
>  /* amount of vm to protect from userspace access by both DAC and the LSM*/
>  unsigned long mmap_min_addr;
>  /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -54,11 +56,10 @@ static const struct ctl_table min_addr_sysctl_table[] = {
>  	},
>  };
>  
> -static int __init init_mmap_min_addr(void)
> +int __init min_addr_init(void)
>  {
>  	register_sysctl_init("vm", min_addr_sysctl_table);
>  	update_mmap_min_addr();
>  
>  	return 0;
>  }
> -pure_initcall(init_mmap_min_addr);

^ permalink raw reply

* Re: [RFC PATCH v2 21/34] lsm: output available LSMs when debugging
From: Casey Schaufler @ 2025-07-24 23:33 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-57-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> This will display all of the LSMs built into the kernel, regardless
> of if they are enabled or not.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 0a8e4c725055..7e794f4515ea 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -360,6 +360,8 @@ int __init early_security_init(void)
>  {
>  	struct lsm_info *lsm;
>  
> +	/* NOTE: lsm_pr_dbg() doesn't work here as lsm_debug is not yet set */
> +
>  	lsm_early_for_each_raw(lsm) {
>  		lsm_enabled_set(lsm, true);
>  		lsm_order_append(lsm, "early");
> @@ -382,9 +384,24 @@ int __init security_init(void)
>  	struct lsm_info **lsm;
>  
>  	if (lsm_debug) {
> -		lsm_pr("built-in LSM list: %s\n", lsm_order_builtin);
> +		struct lsm_info *i;
> +
> +		cnt = 0;
> +		lsm_pr("available LSMs: ");
> +		lsm_early_for_each_raw(i)
> +			lsm_pr_cont("%s%s(E)", (cnt++ ? "," : ""), i->id->name);
> +		lsm_for_each_raw(i)
> +			lsm_pr_cont("%s%s", (cnt++ ? "," : ""), i->id->name);
> +		lsm_pr_cont("\n");
> +
> +		lsm_pr("built-in LSM config: %s\n", lsm_order_builtin);
> +
>  		lsm_pr("legacy LSM parameter: %s\n", lsm_order_legacy);
>  		lsm_pr("boot LSM parameter: %s\n", lsm_order_cmdline);
> +
> +		/* see the note about lsm_pr_dbg() in early_security_init() */
> +		lsm_early_for_each_raw(i)
> +			lsm_pr("enabled LSM early:%s\n", i->id->name);
>  	}
>  
>  	if (lsm_order_cmdline) {

^ permalink raw reply

* Re: [RFC PATCH v2 19/34] lsm: add/tweak function header comment blocks in lsm_init.c
From: Casey Schaufler @ 2025-07-24 23:31 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-55-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Add function header comments for lsm_static_call_init() and
> early_security_init(), tweak the existing comment block for
> security_add_hooks().
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 25fe0c89e884..75e97f6b530c 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -312,6 +312,10 @@ static void __init lsm_order_parse(const char *list, const char *src)
>  	}
>  }
>  
> +/**
> + * lsm_static_call_init - Initialize a LSM's static calls
> + * @hl: LSM hook list
> + */
>  static void __init lsm_static_call_init(struct security_hook_list *hl)
>  {
>  	struct lsm_static_call *scall = hl->scalls;
> @@ -332,12 +336,12 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
>  }
>  
>  /**
> - * security_add_hooks - Add a modules hooks to the hook lists.
> - * @hooks: the hooks to add
> - * @count: the number of hooks to add
> - * @lsmid: the identification information for the security module
> + * security_add_hooks - Add a LSM's hooks to the LSM framework's hook lists
> + * @hooks: LSM hooks to add
> + * @count: number of hooks to add
> + * @lsmid: identification information for the LSM
>   *
> - * Each LSM has to register its hooks with the infrastructure.
> + * Each LSM has to register its hooks with the LSM framework.
>   */
>  void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  			       const struct lsm_id *lsmid)
> @@ -350,6 +354,9 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  	}
>  }
>  
> +/**
> + * early_security_init - Initialize the early LSMs
> + */
>  int __init early_security_init(void)
>  {
>  	struct lsm_info *lsm;

^ permalink raw reply

* Re: [RFC PATCH v2 17/34] lsm: cleanup initialize_lsm() and rename to lsm_init_single()
From: Casey Schaufler @ 2025-07-24 23:29 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-53-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Rename initialize_lsm() to be more consistent with the rest of the LSM
> initialization changes and rework the function itself to better fit
> with the "exit on fail" coding pattern.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index aad363e37140..49f93383e551 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -169,6 +169,7 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
> +
>  /**
>   * lsm_blob_size_update - Update the LSM blob size and offset information
>   * @sz_req: the requested additional blob size
> @@ -222,16 +223,20 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  	lsm_blob_size_update(&blobs->lbs_bdev, &blob_sizes.lbs_bdev);
>  }
>  
> -/* Initialize a given LSM, if it is enabled. */
> -static void __init initialize_lsm(struct lsm_info *lsm)
> +/**
> + * lsm_init_single - Initialize a given LSM
> + * @lsm: LSM definition
> + */
> +static void __init lsm_init_single(struct lsm_info *lsm)
>  {
> -	if (lsm_is_enabled(lsm)) {
> -		int ret;
> +	int ret;
>  
> -		init_debug("initializing %s\n", lsm->id->name);
> -		ret = lsm->init();
> -		WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
> -	}
> +	if (!lsm_is_enabled(lsm))
> +		return;
> +
> +	init_debug("initializing %s\n", lsm->id->name);
> +	ret = lsm->init();
> +	WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
>  }
>  
>  /**
> @@ -373,7 +378,7 @@ static void __init lsm_init_ordered(void)
>  		panic("%s: early task alloc failed.\n", __func__);
>  
>  	lsm_order_for_each(lsm) {
> -		initialize_lsm(*lsm);
> +		lsm_init_single(*lsm);
>  	}
>  }
>  
> @@ -423,7 +428,7 @@ int __init early_security_init(void)
>  		lsm_enabled_set(lsm, true);
>  		lsm_order_append(lsm, "early");
>  		lsm_prepare(lsm);
> -		initialize_lsm(lsm);
> +		lsm_init_single(lsm);
>  	}
>  
>  	return 0;

^ permalink raw reply

* Re: [RFC PATCH v2 16/34] lsm: cleanup the LSM blob size code
From: Casey Schaufler @ 2025-07-24 23:28 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-52-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Convert the lsm_blob_size fields to unsigned integers as there is no
> current need for them to be negative, change "lsm_set_blob_size()" to
> "lsm_blob_size_update()" to better reflect reality, and perform some
> other minor cleanups to the associated code.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  include/linux/lsm_hooks.h | 28 ++++++++++-----------
>  security/lsm_init.c       | 51 +++++++++++++++++++++++----------------
>  2 files changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index bc477fb20d02..a7ecb0791a0f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -102,20 +102,20 @@ struct security_hook_list {
>   * Security blob size or offset data.
>   */
>  struct lsm_blob_sizes {
> -	int lbs_cred;
> -	int lbs_file;
> -	int lbs_ib;
> -	int lbs_inode;
> -	int lbs_sock;
> -	int lbs_superblock;
> -	int lbs_ipc;
> -	int lbs_key;
> -	int lbs_msg_msg;
> -	int lbs_perf_event;
> -	int lbs_task;
> -	int lbs_xattr_count; /* number of xattr slots in new_xattrs array */
> -	int lbs_tun_dev;
> -	int lbs_bdev;
> +	unsigned int lbs_cred;
> +	unsigned int lbs_file;
> +	unsigned int lbs_ib;
> +	unsigned int lbs_inode;
> +	unsigned int lbs_sock;
> +	unsigned int lbs_superblock;
> +	unsigned int lbs_ipc;
> +	unsigned int lbs_key;
> +	unsigned int lbs_msg_msg;
> +	unsigned int lbs_perf_event;
> +	unsigned int lbs_task;
> +	unsigned int lbs_xattr_count; /* num xattr slots in new_xattrs array */
> +	unsigned int lbs_tun_dev;
> +	unsigned int lbs_bdev;
>  };
>  
>  /*
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index b1156f414491..aad363e37140 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -169,16 +169,22 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
> -static void __init lsm_set_blob_size(int *need, int *lbs)
> +/**
> + * lsm_blob_size_update - Update the LSM blob size and offset information
> + * @sz_req: the requested additional blob size
> + * @sz_cur: the existing blob size
> + */
> +static void __init lsm_blob_size_update(unsigned int *sz_req,
> +					unsigned int *sz_cur)
>  {
> -	int offset;
> +	unsigned int offset;
>  
> -	if (*need <= 0)
> +	if (*sz_req == 0)
>  		return;
>  
> -	offset = ALIGN(*lbs, sizeof(void *));
> -	*lbs = offset + *need;
> -	*need = offset;
> +	offset = ALIGN(*sz_cur, sizeof(void *));
> +	*sz_cur = offset + *sz_req;
> +	*sz_req = offset;
>  }
>  
>  /**
> @@ -193,24 +199,27 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  		return;
>  
>  	/* Register the LSM blob sizes. */
> -	lsm_set_blob_size(&blobs->lbs_cred, &blob_sizes.lbs_cred);
> -	lsm_set_blob_size(&blobs->lbs_file, &blob_sizes.lbs_file);
> -	lsm_set_blob_size(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> +	blobs = lsm->blobs;
> +	lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
> +	lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
> +	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
>  	/* inode blob gets an rcu_head in addition to LSM blobs. */
>  	if (blobs->lbs_inode && blob_sizes.lbs_inode == 0)
>  		blob_sizes.lbs_inode = sizeof(struct rcu_head);
> -	lsm_set_blob_size(&blobs->lbs_inode, &blob_sizes.lbs_inode);
> -	lsm_set_blob_size(&blobs->lbs_ipc, &blob_sizes.lbs_ipc);
> -	lsm_set_blob_size(&blobs->lbs_key, &blob_sizes.lbs_key);
> -	lsm_set_blob_size(&blobs->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> -	lsm_set_blob_size(&blobs->lbs_perf_event, &blob_sizes.lbs_perf_event);
> -	lsm_set_blob_size(&blobs->lbs_sock, &blob_sizes.lbs_sock);
> -	lsm_set_blob_size(&blobs->lbs_superblock, &blob_sizes.lbs_superblock);
> -	lsm_set_blob_size(&blobs->lbs_task, &blob_sizes.lbs_task);
> -	lsm_set_blob_size(&blobs->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
> -	lsm_set_blob_size(&blobs->lbs_xattr_count,
> -			  &blob_sizes.lbs_xattr_count);
> -	lsm_set_blob_size(&blobs->lbs_bdev, &blob_sizes.lbs_bdev);
> +	lsm_blob_size_update(&blobs->lbs_inode, &blob_sizes.lbs_inode);
> +	lsm_blob_size_update(&blobs->lbs_ipc, &blob_sizes.lbs_ipc);
> +	lsm_blob_size_update(&blobs->lbs_key, &blob_sizes.lbs_key);
> +	lsm_blob_size_update(&blobs->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> +	lsm_blob_size_update(&blobs->lbs_perf_event,
> +			     &blob_sizes.lbs_perf_event);
> +	lsm_blob_size_update(&blobs->lbs_sock, &blob_sizes.lbs_sock);
> +	lsm_blob_size_update(&blobs->lbs_superblock,
> +			     &blob_sizes.lbs_superblock);
> +	lsm_blob_size_update(&blobs->lbs_task, &blob_sizes.lbs_task);
> +	lsm_blob_size_update(&blobs->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
> +	lsm_blob_size_update(&blobs->lbs_xattr_count,
> +			     &blob_sizes.lbs_xattr_count);
> +	lsm_blob_size_update(&blobs->lbs_bdev, &blob_sizes.lbs_bdev);
>  }
>  
>  /* Initialize a given LSM, if it is enabled. */

^ permalink raw reply

* Re: [RFC PATCH v2 34/34] lsm: add a LSM_STARTED_ALL notification event
From: Casey Schaufler @ 2025-07-24 23:38 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-70-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Add a new LSM notifier event, LSM_STARTED_ALL, which is fired once at
> boot when all of the LSMs have been started.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  include/linux/security.h | 1 +
>  security/lsm_init.c      | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c57a9a7499ea..b9cace094f0f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -85,6 +85,7 @@ struct timezone;
>  
>  enum lsm_event {
>  	LSM_POLICY_CHANGE,
> +	LSM_STARTED_ALL,
>  };
>  
>  struct dm_verity_digest {
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index f178a9a2f9d4..6812a13f64c9 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -550,6 +550,7 @@ static int __init security_initcall_late(void)
>  
>  	rc = lsm_initcall(late);
>  	lsm_pr_dbg("all enabled LSMs fully activated\n");
> +	call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
>  
>  	return rc;
>  }

^ permalink raw reply

* Re: [RFC PATCH v2 26/34] smack: move initcalls to the LSM framework
From: Casey Schaufler @ 2025-07-24 23:36 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-62-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> As the LSM framework only supports one LSM initcall callback for each
> initcall type, the init_smk_fs() and smack_nf_ip_init() functions were
> wrapped with a new function, smack_initcall() that is registered with
> the LSM framework.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/smack/smack.h           | 7 +++++++
>  security/smack/smack_lsm.c       | 9 +++++++++
>  security/smack/smack_netfilter.c | 4 +---
>  security/smack/smackfs.c         | 4 +---
>  4 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index bf6a6ed3946c..885a2f2929fd 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -275,6 +275,13 @@ struct smk_audit_info {
>  #endif
>  };
>  
> +/*
> + * Initialization
> + */
> +int init_smk_fs(void);
> +int smack_nf_ip_init(void);
> +int smack_initcall(void);
> +
>  /*
>   * These functions are in smack_access.c
>   */
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index e09490c75f59..f14d536c516b 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -5270,6 +5270,14 @@ static __init int smack_init(void)
>  	return 0;
>  }
>  
> +int __init smack_initcall(void)
> +{
> +	int rc_fs = init_smk_fs();
> +	int rc_nf = smack_nf_ip_init();
> +
> +	return rc_fs ? rc_fs : rc_nf;
> +}
> +
>  /*
>   * Smack requires early initialization in order to label
>   * all processes and objects when they are created.
> @@ -5279,4 +5287,5 @@ DEFINE_LSM(smack) = {
>  	.flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
>  	.blobs = &smack_blob_sizes,
>  	.init = smack_init,
> +	.initcall_device = smack_initcall,
>  };
> diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
> index 8fd747b3653a..17ba578b1308 100644
> --- a/security/smack/smack_netfilter.c
> +++ b/security/smack/smack_netfilter.c
> @@ -68,7 +68,7 @@ static struct pernet_operations smack_net_ops = {
>  	.exit = smack_nf_unregister,
>  };
>  
> -static int __init smack_nf_ip_init(void)
> +int __init smack_nf_ip_init(void)
>  {
>  	if (smack_enabled == 0)
>  		return 0;
> @@ -76,5 +76,3 @@ static int __init smack_nf_ip_init(void)
>  	printk(KERN_DEBUG "Smack: Registering netfilter hooks\n");
>  	return register_pernet_subsys(&smack_net_ops);
>  }
> -
> -__initcall(smack_nf_ip_init);
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index b1e5e62f5cbd..405ace6db109 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -2978,7 +2978,7 @@ static struct vfsmount *smackfs_mount;
>   * Returns true if we were not chosen on boot or if
>   * we were chosen and filesystem registration succeeded.
>   */
> -static int __init init_smk_fs(void)
> +int __init init_smk_fs(void)
>  {
>  	int err;
>  	int rc;
> @@ -3021,5 +3021,3 @@ static int __init init_smk_fs(void)
>  
>  	return err;
>  }
> -
> -__initcall(init_smk_fs);

^ permalink raw reply

* Re: [RFC PATCH v2 23/34] lsm: introduce an initcall mechanism into the LSM framework
From: Casey Schaufler @ 2025-07-24 23:35 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-59-paul@paul-moore.com>

On 7/21/2025 4:21 PM, 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.
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.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
> + */
>  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 ada9b5448409..ab739f9c2244 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
> @@ -455,3 +476,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);

^ permalink raw reply

* Re: [RFC PATCH v2 22/34] lsm: group lsm_order_parse() with the other lsm_order_*() functions
From: Casey Schaufler @ 2025-07-24 23:34 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-58-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Move the lsm_order_parse() function near the other lsm_order_*()
> functions to improve readability.
>
> No code changes.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 138 ++++++++++++++++++++++----------------------
>  1 file changed, 69 insertions(+), 69 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7e794f4515ea..ada9b5448409 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -169,75 +169,6 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  	lsm_pr_dbg("enabling LSM %s:%s\n", src, lsm->id->name);
>  }
>  
> -/**
> - * lsm_blob_size_update - Update the LSM blob size and offset information
> - * @sz_req: the requested additional blob size
> - * @sz_cur: the existing blob size
> - */
> -static void __init lsm_blob_size_update(unsigned int *sz_req,
> -					unsigned int *sz_cur)
> -{
> -	unsigned int offset;
> -
> -	if (*sz_req == 0)
> -		return;
> -
> -	offset = ALIGN(*sz_cur, sizeof(void *));
> -	*sz_cur = offset + *sz_req;
> -	*sz_req = offset;
> -}
> -
> -/**
> - * lsm_prepare - Prepare the LSM framework for a new LSM
> - * @lsm: LSM definition
> - */
> -static void __init lsm_prepare(struct lsm_info *lsm)
> -{
> -	struct lsm_blob_sizes *blobs = lsm->blobs;
> -
> -	if (!blobs)
> -		return;
> -
> -	/* Register the LSM blob sizes. */
> -	blobs = lsm->blobs;
> -	lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
> -	lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
> -	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> -	/* inode blob gets an rcu_head in addition to LSM blobs. */
> -	if (blobs->lbs_inode && blob_sizes.lbs_inode == 0)
> -		blob_sizes.lbs_inode = sizeof(struct rcu_head);
> -	lsm_blob_size_update(&blobs->lbs_inode, &blob_sizes.lbs_inode);
> -	lsm_blob_size_update(&blobs->lbs_ipc, &blob_sizes.lbs_ipc);
> -	lsm_blob_size_update(&blobs->lbs_key, &blob_sizes.lbs_key);
> -	lsm_blob_size_update(&blobs->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> -	lsm_blob_size_update(&blobs->lbs_perf_event,
> -			     &blob_sizes.lbs_perf_event);
> -	lsm_blob_size_update(&blobs->lbs_sock, &blob_sizes.lbs_sock);
> -	lsm_blob_size_update(&blobs->lbs_superblock,
> -			     &blob_sizes.lbs_superblock);
> -	lsm_blob_size_update(&blobs->lbs_task, &blob_sizes.lbs_task);
> -	lsm_blob_size_update(&blobs->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
> -	lsm_blob_size_update(&blobs->lbs_xattr_count,
> -			     &blob_sizes.lbs_xattr_count);
> -	lsm_blob_size_update(&blobs->lbs_bdev, &blob_sizes.lbs_bdev);
> -}
> -
> -/**
> - * lsm_init_single - Initialize a given LSM
> - * @lsm: LSM definition
> - */
> -static void __init lsm_init_single(struct lsm_info *lsm)
> -{
> -	int ret;
> -
> -	if (!lsm_is_enabled(lsm))
> -		return;
> -
> -	lsm_pr_dbg("initializing %s\n", lsm->id->name);
> -	ret = lsm->init();
> -	WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
> -}
> -
>  /**
>   * lsm_order_parse - Parse the comma delimited LSM list
>   * @list: LSM list
> @@ -308,6 +239,75 @@ static void __init lsm_order_parse(const char *list, const char *src)
>  	}
>  }
>  
> +/**
> + * lsm_blob_size_update - Update the LSM blob size and offset information
> + * @sz_req: the requested additional blob size
> + * @sz_cur: the existing blob size
> + */
> +static void __init lsm_blob_size_update(unsigned int *sz_req,
> +					unsigned int *sz_cur)
> +{
> +	unsigned int offset;
> +
> +	if (*sz_req == 0)
> +		return;
> +
> +	offset = ALIGN(*sz_cur, sizeof(void *));
> +	*sz_cur = offset + *sz_req;
> +	*sz_req = offset;
> +}
> +
> +/**
> + * lsm_prepare - Prepare the LSM framework for a new LSM
> + * @lsm: LSM definition
> + */
> +static void __init lsm_prepare(struct lsm_info *lsm)
> +{
> +	struct lsm_blob_sizes *blobs = lsm->blobs;
> +
> +	if (!blobs)
> +		return;
> +
> +	/* Register the LSM blob sizes. */
> +	blobs = lsm->blobs;
> +	lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
> +	lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
> +	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> +	/* inode blob gets an rcu_head in addition to LSM blobs. */
> +	if (blobs->lbs_inode && blob_sizes.lbs_inode == 0)
> +		blob_sizes.lbs_inode = sizeof(struct rcu_head);
> +	lsm_blob_size_update(&blobs->lbs_inode, &blob_sizes.lbs_inode);
> +	lsm_blob_size_update(&blobs->lbs_ipc, &blob_sizes.lbs_ipc);
> +	lsm_blob_size_update(&blobs->lbs_key, &blob_sizes.lbs_key);
> +	lsm_blob_size_update(&blobs->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> +	lsm_blob_size_update(&blobs->lbs_perf_event,
> +			     &blob_sizes.lbs_perf_event);
> +	lsm_blob_size_update(&blobs->lbs_sock, &blob_sizes.lbs_sock);
> +	lsm_blob_size_update(&blobs->lbs_superblock,
> +			     &blob_sizes.lbs_superblock);
> +	lsm_blob_size_update(&blobs->lbs_task, &blob_sizes.lbs_task);
> +	lsm_blob_size_update(&blobs->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
> +	lsm_blob_size_update(&blobs->lbs_xattr_count,
> +			     &blob_sizes.lbs_xattr_count);
> +	lsm_blob_size_update(&blobs->lbs_bdev, &blob_sizes.lbs_bdev);
> +}
> +
> +/**
> + * lsm_init_single - Initialize a given LSM
> + * @lsm: LSM definition
> + */
> +static void __init lsm_init_single(struct lsm_info *lsm)
> +{
> +	int ret;
> +
> +	if (!lsm_is_enabled(lsm))
> +		return;
> +
> +	lsm_pr_dbg("initializing %s\n", lsm->id->name);
> +	ret = lsm->init();
> +	WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
> +}
> +
>  /**
>   * lsm_static_call_init - Initialize a LSM's static calls
>   * @hl: LSM hook list

^ permalink raw reply

* Re: [RFC PATCH v2 20/34] lsm: cleanup the debug and console output in lsm_init.c
From: Casey Schaufler @ 2025-07-24 23:32 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-56-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Move away from an init specific init_debug() macro to a more general
> lsm_pr()/lsm_pr_cont()/lsm_pr_dbg() set of macros that are available
> both before and after init.  In the process we do a number of minor
> changes to improve the LSM initialization output and cleanup the code
> somewhat.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm.h      |  11 +++++
>  security/lsm_init.c | 117 +++++++++++++++++++-------------------------
>  security/security.c |   2 +
>  3 files changed, 63 insertions(+), 67 deletions(-)
>
> diff --git a/security/lsm.h b/security/lsm.h
> index dbe755c45e57..8dc267977ae0 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -6,9 +6,20 @@
>  #ifndef _LSM_H_
>  #define _LSM_H_
>  
> +#include <linux/printk.h>
>  #include <linux/lsm_hooks.h>
>  #include <linux/lsm_count.h>
>  
> +/* LSM debugging */
> +extern bool lsm_debug;
> +#define lsm_pr(...)		pr_info(__VA_ARGS__)
> +#define lsm_pr_cont(...)	pr_cont(__VA_ARGS__)
> +#define lsm_pr_dbg(...)							\
> +	do {								\
> +		if (lsm_debug)						\
> +			pr_info(__VA_ARGS__);				\
> +	} while (0)
> +
>  /* List of configured LSMs */
>  extern unsigned int lsm_active_cnt;
>  extern const struct lsm_id *lsm_idlist[];
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 75e97f6b530c..0a8e4c725055 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -30,13 +30,6 @@ static __initdata const char *lsm_order_legacy;
>  static __initdata struct lsm_info *lsm_exclusive;
>  static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
>  
> -static __initdata bool debug;
> -#define init_debug(...)							\
> -	do {								\
> -		if (debug)						\
> -			pr_info(__VA_ARGS__);				\
> -	} while (0)
> -
>  #define lsm_order_for_each(iter)					\
>  	for ((iter) = lsm_order; *(iter); (iter)++)
>  #define lsm_for_each_raw(iter)						\
> @@ -77,7 +70,7 @@ __setup("lsm=", lsm_choose_lsm);
>   */
>  static int __init lsm_debug_enable(char *str)
>  {
> -	debug = true;
> +	lsm_debug = true;
>  	return 1;
>  }
>  __setup("lsm.debug", lsm_debug_enable);
> @@ -143,22 +136,28 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  		return;
>  
>  	/* Skip explicitly disabled LSMs. */
> -	if (lsm->enabled && !lsm_is_enabled(lsm))
> -		goto out;
> +	if (lsm->enabled && !lsm_is_enabled(lsm)) {
> +		lsm_pr_dbg("skip previously disabled LSM %s:%s\n",
> +			   src, lsm->id->name);
> +		return;
> +	}
>  
> -	if (WARN(lsm_active_cnt == MAX_LSM_COUNT,
> -		 "%s: out of LSM static calls!?\n", src)) {
> +	if (lsm_active_cnt == MAX_LSM_COUNT) {
> +		pr_warn("exceeded maximum LSM count on %s:%s\n",
> +			src, lsm->id->name);
>  		lsm_enabled_set(lsm, false);
> -		goto out;
> +		return;
>  	}
>  
>  	if (lsm->flags & LSM_FLAG_EXCLUSIVE) {
>  		if (lsm_exclusive) {
> -			init_debug("exclusive disabled: %s\n", lsm->id->name);
> +			lsm_pr_dbg("skip exclusive LSM conflict %s:%s\n",
> +				   src, lsm->id->name);
>  			lsm_enabled_set(lsm, false);
> -			goto out;
> +			return;
>  		} else {
> -			init_debug("exclusive chosen:   %s\n", lsm->id->name);
> +			lsm_pr_dbg("select exclusive LSM %s:%s\n",
> +				   src, lsm->id->name);
>  			lsm_exclusive = lsm;
>  		}
>  	}
> @@ -167,9 +166,7 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  	lsm_order[lsm_active_cnt] = lsm;
>  	lsm_idlist[lsm_active_cnt++] = lsm->id;
>  
> -out:
> -	init_debug("%s ordered: %s (%s)\n", src, lsm->id->name,
> -		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
> +	lsm_pr_dbg("enabling LSM %s:%s\n", src, lsm->id->name);
>  }
>  
>  /**
> @@ -236,7 +233,7 @@ static void __init lsm_init_single(struct lsm_info *lsm)
>  	if (!lsm_is_enabled(lsm))
>  		return;
>  
> -	init_debug("initializing %s\n", lsm->id->name);
> +	lsm_pr_dbg("initializing %s\n", lsm->id->name);
>  	ret = lsm->init();
>  	WARN(ret, "%s failed to initialize: %d\n", lsm->id->name, ret);
>  }
> @@ -263,8 +260,8 @@ static void __init lsm_order_parse(const char *list, const char *src)
>  			if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
>  			     strcmp(lsm->id->name, lsm_order_legacy)) {
>  				lsm_enabled_set(lsm, false);
> -				init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> -					   lsm_order_legacy, lsm->id->name);
> +				lsm_pr_dbg("skip legacy LSM conflict %s:%s\n",
> +					   src, lsm->id->name);
>  			}
>  		}
>  	}
> @@ -307,8 +304,7 @@ static void __init lsm_order_parse(const char *list, const char *src)
>  		if (lsm_order_exists(lsm))
>  			continue;
>  		lsm_enabled_set(lsm, false);
> -		init_debug("%s skipped: %s (not in requested order)\n",
> -			   src, lsm->id->name);
> +		lsm_pr_dbg("skip disabled LSM %s:%s\n", src, lsm->id->name);
>  	}
>  }
>  
> @@ -316,7 +312,7 @@ static void __init lsm_order_parse(const char *list, const char *src)
>   * lsm_static_call_init - Initialize a LSM's static calls
>   * @hl: LSM hook list
>   */
> -static void __init lsm_static_call_init(struct security_hook_list *hl)
> +static int __init lsm_static_call_init(struct security_hook_list *hl)
>  {
>  	struct lsm_static_call *scall = hl->scalls;
>  	int i;
> @@ -328,11 +324,12 @@ static void __init lsm_static_call_init(struct security_hook_list *hl)
>  					     hl->hook.lsm_func_addr);
>  			scall->hl = hl;
>  			static_branch_enable(scall->active);
> -			return;
> +			return 0;
>  		}
>  		scall++;
>  	}
> -	panic("%s - Ran out of static slots.\n", __func__);
> +
> +	return -ENOSPC;
>  }
>  
>  /**
> @@ -350,7 +347,9 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  
>  	for (i = 0; i < count; i++) {
>  		hooks[i].lsmid = lsmid;
> -		lsm_static_call_init(&hooks[i]);
> +		if (lsm_static_call_init(&hooks[i]))
> +			panic("exhausted LSM callback slots with LSM %s\n",
> +			      lsmid->name);
>  	}
>  }
>  
> @@ -381,19 +380,16 @@ int __init security_init(void)
>  {
>  	unsigned int cnt;
>  	struct lsm_info **lsm;
> -	struct lsm_info *early;
> -	unsigned int first = 0;
>  
> -	init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
> -	init_debug("  CONFIG_LSM=%s\n", lsm_order_builtin);
> -	init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
> +	if (lsm_debug) {
> +		lsm_pr("built-in LSM list: %s\n", lsm_order_builtin);
> +		lsm_pr("legacy LSM parameter: %s\n", lsm_order_legacy);
> +		lsm_pr("boot LSM parameter: %s\n", lsm_order_cmdline);
> +	}
>  
>  	if (lsm_order_cmdline) {
> -		if (lsm_order_legacy) {
> -			pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> -				lsm_order_legacy, lsm_order_cmdline);
> +		if (lsm_order_legacy)
>  			lsm_order_legacy = NULL;
> -		}
>  		lsm_order_parse(lsm_order_cmdline, "cmdline");
>  	} else
>  		lsm_order_parse(lsm_order_builtin, "builtin");
> @@ -401,35 +397,22 @@ int __init security_init(void)
>  	lsm_order_for_each(lsm)
>  		lsm_prepare(*lsm);
>  
> -	pr_info("initializing lsm=");
> -	lsm_early_for_each_raw(early) {
> -		if (lsm_is_enabled(early))
> -			pr_cont("%s%s",
> -				first++ == 0 ? "" : ",", early->id->name);
> +	if (lsm_debug) {
> +		lsm_pr("blob(cred) size %d\n", blob_sizes.lbs_cred);
> +		lsm_pr("blob(file) size %d\n", blob_sizes.lbs_file);
> +		lsm_pr("blob(ib) size %d\n", blob_sizes.lbs_ib);
> +		lsm_pr("blob(inode) size %d\n", blob_sizes.lbs_inode);
> +		lsm_pr("blob(ipc) size %d\n", blob_sizes.lbs_ipc);
> +		lsm_pr("blob(key) size %d\n", blob_sizes.lbs_key);
> +		lsm_pr("blob(msg_msg)_size %d\n", blob_sizes.lbs_msg_msg);
> +		lsm_pr("blob(sock) size %d\n", blob_sizes.lbs_sock);
> +		lsm_pr("blob(superblock) size %d\n", blob_sizes.lbs_superblock);
> +		lsm_pr("blob(perf_event) size %d\n", blob_sizes.lbs_perf_event);
> +		lsm_pr("blob(task) size %d\n", blob_sizes.lbs_task);
> +		lsm_pr("blob(tun_dev) size %d\n", blob_sizes.lbs_tun_dev);
> +		lsm_pr("blob(xattr) count %d\n", blob_sizes.lbs_xattr_count);
> +		lsm_pr("blob(bdev) size %d\n", blob_sizes.lbs_bdev);
>  	}
> -	lsm_order_for_each(lsm) {
> -		if (lsm_is_enabled(*lsm))
> -			pr_cont("%s%s",
> -				first++ == 0 ? "" : ",", (*lsm)->id->name);
> -	}
> -	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);
> -	init_debug("ib blob size         = %d\n", blob_sizes.lbs_ib);
> -	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
> -	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
> -#ifdef CONFIG_KEYS
> -	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
> -#endif /* CONFIG_KEYS */
> -	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
> -	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
> -	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> -	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
> -	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
> -	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
> -	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> -	init_debug("bdev blob size       = %d\n", blob_sizes.lbs_bdev);
>  
>  	if (blob_sizes.lbs_file)
>  		lsm_file_cache = kmem_cache_create("lsm_file_cache",
> @@ -441,9 +424,9 @@ int __init security_init(void)
>  						    SLAB_PANIC, NULL);
>  
>  	if (lsm_cred_alloc((struct cred *)current->cred, GFP_KERNEL))
> -		panic("%s: early cred alloc failed.\n", __func__);
> +		panic("early LSM cred alloc failed\n");
>  	if (lsm_task_alloc(current))
> -		panic("%s: early task alloc failed.\n", __func__);
> +		panic("early LSM task alloc failed\n");
>  
>  	cnt = 0;
>  	lsm_order_for_each(lsm) {
> diff --git a/security/security.c b/security/security.c
> index 106f225f4a80..9dd1435f9965 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -73,6 +73,8 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
>  	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>  };
>  
> +bool lsm_debug __ro_after_init;
> +
>  unsigned int lsm_active_cnt __ro_after_init;
>  const struct lsm_id *lsm_idlist[MAX_LSM_COUNT];
>  

^ permalink raw reply

* Re: [RFC PATCH v2 18/34] lsm: fold lsm_init_ordered() into security_init()
From: Casey Schaufler @ 2025-07-24 23:30 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-54-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> With only security_init() calling lsm_init_ordered, it makes little
> sense to keep lsm_init_ordered() as a standalone function.  Fold
> lsm_init_ordered() into security_init().
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 157 ++++++++++++++++++++------------------------
>  1 file changed, 72 insertions(+), 85 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 49f93383e551..25fe0c89e884 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -18,6 +18,9 @@ static __initdata int lsm_enabled_false = 0;
>  extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
>  extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>  
> +/* Number of "early" LSMs */
> +static __initdata unsigned int lsm_count_early;
> +
>  /* Build and boot-time LSM ordering. */
>  static __initconst const char *const lsm_order_builtin = CONFIG_LSM;
>  static __initdata const char *lsm_order_cmdline;
> @@ -169,7 +172,6 @@ static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
> -
>  /**
>   * lsm_blob_size_update - Update the LSM blob size and offset information
>   * @sz_req: the requested additional blob size
> @@ -310,78 +312,6 @@ static void __init lsm_order_parse(const char *list, const char *src)
>  	}
>  }
>  
> -/**
> - * lsm_init_ordered - Initialize the ordered LSMs
> - */
> -static void __init lsm_init_ordered(void)
> -{
> -	unsigned int first = 0;
> -	struct lsm_info **lsm;
> -	struct lsm_info *early;
> -
> -	if (lsm_order_cmdline) {
> -		if (lsm_order_legacy) {
> -			pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> -				lsm_order_legacy, lsm_order_cmdline);
> -			lsm_order_legacy = NULL;
> -		}
> -		lsm_order_parse(lsm_order_cmdline, "cmdline");
> -	} else
> -		lsm_order_parse(lsm_order_builtin, "builtin");
> -
> -	lsm_order_for_each(lsm) {
> -		lsm_prepare(*lsm);
> -	}
> -
> -	pr_info("initializing lsm=");
> -	lsm_early_for_each_raw(early) {
> -		if (lsm_is_enabled(early))
> -			pr_cont("%s%s",
> -				first++ == 0 ? "" : ",", early->id->name);
> -	}
> -	lsm_order_for_each(lsm) {
> -		if (lsm_is_enabled(*lsm))
> -			pr_cont("%s%s",
> -				first++ == 0 ? "" : ",", (*lsm)->id->name);
> -	}
> -	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);
> -	init_debug("ib blob size         = %d\n", blob_sizes.lbs_ib);
> -	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
> -	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
> -#ifdef CONFIG_KEYS
> -	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
> -#endif /* CONFIG_KEYS */
> -	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
> -	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
> -	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> -	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
> -	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
> -	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
> -	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> -	init_debug("bdev blob size       = %d\n", blob_sizes.lbs_bdev);
> -
> -	if (blob_sizes.lbs_file)
> -		lsm_file_cache = kmem_cache_create("lsm_file_cache",
> -						   blob_sizes.lbs_file, 0,
> -						   SLAB_PANIC, NULL);
> -	if (blob_sizes.lbs_inode)
> -		lsm_inode_cache = kmem_cache_create("lsm_inode_cache",
> -						    blob_sizes.lbs_inode, 0,
> -						    SLAB_PANIC, NULL);
> -
> -	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) {
> -		lsm_init_single(*lsm);
> -	}
> -}
> -
>  static void __init lsm_static_call_init(struct security_hook_list *hl)
>  {
>  	struct lsm_static_call *scall = hl->scalls;
> @@ -429,35 +359,92 @@ int __init early_security_init(void)
>  		lsm_order_append(lsm, "early");
>  		lsm_prepare(lsm);
>  		lsm_init_single(lsm);
> +		lsm_count_early++;
>  	}
>  
>  	return 0;
>  }
>  
>  /**
> - * security_init - initializes the security framework
> + * security_init - Initializes the LSM framework
>   *
>   * This should be called early in the kernel initialization sequence.
>   */
>  int __init security_init(void)
>  {
> -	struct lsm_info *lsm;
> +	unsigned int cnt;
> +	struct lsm_info **lsm;
> +	struct lsm_info *early;
> +	unsigned int first = 0;
>  
>  	init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
>  	init_debug("  CONFIG_LSM=%s\n", lsm_order_builtin);
>  	init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
>  
> -	/*
> -	 * Append the names of the early LSM modules now that kmalloc() is
> -	 * available
> -	 */
> -	lsm_early_for_each_raw(lsm) {
> -		init_debug("  early started: %s (%s)\n", lsm->id->name,
> -			   lsm_is_enabled(lsm) ? "enabled" : "disabled");
> -	}
> +	if (lsm_order_cmdline) {
> +		if (lsm_order_legacy) {
> +			pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> +				lsm_order_legacy, lsm_order_cmdline);
> +			lsm_order_legacy = NULL;
> +		}
> +		lsm_order_parse(lsm_order_cmdline, "cmdline");
> +	} else
> +		lsm_order_parse(lsm_order_builtin, "builtin");
>  
> -	/* Load LSMs in specified order. */
> -	lsm_init_ordered();
> +	lsm_order_for_each(lsm)
> +		lsm_prepare(*lsm);
> +
> +	pr_info("initializing lsm=");
> +	lsm_early_for_each_raw(early) {
> +		if (lsm_is_enabled(early))
> +			pr_cont("%s%s",
> +				first++ == 0 ? "" : ",", early->id->name);
> +	}
> +	lsm_order_for_each(lsm) {
> +		if (lsm_is_enabled(*lsm))
> +			pr_cont("%s%s",
> +				first++ == 0 ? "" : ",", (*lsm)->id->name);
> +	}
> +	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);
> +	init_debug("ib blob size         = %d\n", blob_sizes.lbs_ib);
> +	init_debug("inode blob size      = %d\n", blob_sizes.lbs_inode);
> +	init_debug("ipc blob size        = %d\n", blob_sizes.lbs_ipc);
> +#ifdef CONFIG_KEYS
> +	init_debug("key blob size        = %d\n", blob_sizes.lbs_key);
> +#endif /* CONFIG_KEYS */
> +	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
> +	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
> +	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> +	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
> +	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
> +	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
> +	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> +	init_debug("bdev blob size       = %d\n", blob_sizes.lbs_bdev);
> +
> +	if (blob_sizes.lbs_file)
> +		lsm_file_cache = kmem_cache_create("lsm_file_cache",
> +						   blob_sizes.lbs_file, 0,
> +						   SLAB_PANIC, NULL);
> +	if (blob_sizes.lbs_inode)
> +		lsm_inode_cache = kmem_cache_create("lsm_inode_cache",
> +						    blob_sizes.lbs_inode, 0,
> +						    SLAB_PANIC, NULL);
> +
> +	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__);
> +
> +	cnt = 0;
> +	lsm_order_for_each(lsm) {
> +		/* skip the "early" LSMs as they have already been setup */
> +		if (cnt++ < lsm_count_early)
> +			continue;
> +		lsm_init_single(*lsm);
> +	}
>  
>  	return 0;
>  }

^ permalink raw reply

* Re: [PATCH v5 bpf-next 0/5] bpf path iterator
From: Mickaël Salaün @ 2025-07-24 17:35 UTC (permalink / raw)
  To: Song Liu
  Cc: NeilBrown, Christian Brauner, Tingmao Wang, Song Liu,
	bpf@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org, Kernel Team,
	andrii@kernel.org, eddyz87@gmail.com, ast@kernel.org,
	daniel@iogearbox.net, martin.lau@linux.dev,
	viro@zeniv.linux.org.uk, jack@suse.cz, kpsingh@kernel.org,
	mattbobrowski@google.com, Günther Noack, Jann Horn
In-Reply-To: <2243B959-AA11-4D24-A6D0-0598E244BE3E@meta.com>

On Mon, Jul 14, 2025 at 09:09:42PM +0000, Song Liu wrote:
> 
> > On Jul 9, 2025, at 11:28 PM, Song Liu <songliubraving@meta.com> wrote:
> 
> [...]
> 
> >>>> It isn't clear to me that vfs_walk_ancestors() needs to return anything.
> >>>> All the communication happens through walk_cb()
> >>>> 
> >>>> walk_cb() is called with a path, the data, and a "may_sleep" flag.
> >>>> If it needs to sleep but may_sleep is not set, it returns "-ECHILD"
> >>>> which causes the walk to restart and use refcounts.
> >>>> If it wants to stop, it returns 0.
> >>>> If it wants to continue, it returns 1.
> >>>> If it wants a reference to the path then it can use (new)
> >>>> vfs_legitimize_path() which might fail.
> >>>> If it wants a reference to the path and may_sleep is true, it can use
> >>>> path_get() which won't fail.
> >>>> 
> >>>> When returning -ECHILD (either because of a need to sleep or because
> >>>> vfs_legitimize_path() fails), walk_cb() would reset_data().
> >>> 
> >>> This might actually work. 
> >>> 
> >>> My only concern is with vfs_legitimize_path. It is probably safer if 
> >>> we only allow taking references with may_sleep==true, so that path_get
> >>> won’t fail. In this case, we will not need walk_cb() to call 
> >>> vfs_legitimize_path. If the user want a reference, the walk_cb will 
> >>> first return -ECHILD, and call path_get when may_sleep is true.
> >> 
> >> What is your concern with vfs_legitimize_path() ??
> >> 
> >> I've since realised that always restarting in response to -ECHILD isn't
> >> necessary and isn't how normal path-walk works.  Restarting might be
> >> needed, but the first response to -ECHILD is to try legitimize_path().
> >> If that succeeds, then it is safe to sleep.
> >> So returning -ECHILD might just result in vfs_walk_ancestors() calling
> >> legitimize_path() and then calling walk_cb() again.  Why not have
> >> walk_cb() do the vfs_legitimize_path() call (which will almost always
> >> succeed in practice).
> > 
> > After reading the emails and the code more, I think I misunderstood 
> > why we need to call vfs_legitimize_path(). The goal of “legitimize” 
> > is to get a reference on @path, so a reference-less walk may not
> > need legitimize_path() at all. Do I get this right this time? 
> > 
> > However, I still have some concern with legitimize_path: it requires
> > m_seq and r_seq recorded at the beginning of the walk, do we want
> > to pass those to walk_cb()? IIUC, one of the reason we prefer a 
> > callback based solution is that it doesn’t expose nameidata (or a
> > subset of it). Letting walk_cb to call legitimize_path appears to 
> > defeat this benefit, no? 

Yes, walk_cb() should be very light and non-blocking/non-sleepable.  If
the caller cannot give these guarantees, then it can just pass NULL
instead of a valid walk_cb(), and continue the walk (if needed) by
calling the vfs_walk_ancentors() helper again, which would not benefit
from the RCU optimization in this case.

Before this patch series land, handling of disconnected directories
should be well defined, or at least let the caller deal with it.  How do
you plan to handle disconnected directories for the eBPF use case?  See
https://lore.kernel.org/all/20250719104204.545188-1-mic@digikod.net/
Unfortunately, this issue is not solved for Landlock yet.

> > 
> > 
> > A separate question below. 
> > 
> > I still have some question about how vfs_walk_ancestors() and the 
> > walk_cb() interact. Let’s look at the landlock use case: the user 
> > (landlock) just want to look at each ancestor, but doesn’t need to 
> > take any references. walk_cb() will check @path against @root, and 
> > return 0 when @path is the same as @root. 
> > 
> > IIUC, in this case, we will record m_seq and r_seq at the beginning
> > of vfs_walk_ancestors(), and check them against mount_lock and 
> > rename_lock at the end of the walk. (Maybe we also need to check 
> > them at some points before the end of the walk?) If either seq
> > changed during the walk, we need to restart the walk, and take
> > reference on each step. Did I get this right so far? 

I think so.  You should get some inspiration from prepend_path().

> > 
> > If the above is right, here are my questions about the 
> > reference-less walk above: 
> > 
> > 1. Which function (vfs_walk_ancestors or walk_cb) will check m_seq 
> >   and r_seq? I think vfs_walk_ancestors should check them. 

Yes, walk_cb() should be as simple as possible: the simpler version
should just return a constant.

> > 2. When either seq changes, which function will call reset_data?
> >   I think there are 3 options here:
> >  2.a: vfs_walk_ancestors calls reset_data, which will be another
> >       callback function the caller passes to vfs_walk_ancestors. 
> >  2.b: walk_cb will call reset_data(), but we need a mechanism to
> >       tell walk_cb to do it, maybe a “restart” flag?
> >  2.c: Caller of vfs_walk_ancestors will call reset_data(). In 
> >       this case, vfs_walk_ancestors will return -ECHILD to its
> >       caller. But I think this option is NACKed. 
> > 
> > I think the right solution is to have vfs_walk_ancestors check
> > m_seq and r_seq, and have walk_cb call reset_data. But this is
> > Different to the proposal above. 

I'm not sure a reset_data() would be useful if walk_cb() never sleep.

If we really need such reset_data(), a fourth option would be for
walk_cb() to return a specific value (an enum instead of a bool) to
trigger the reset.

> > 
> > Do my questions above make any sense? Or maybe I totally 
> > misunderstood something?
> 
> Hi Neil, 
> 
> Did my questions/comments above make sense? I am hoping we can 
> agree on some design soon. 
> 
> Christian and Mickaël, 
> 
> Could you please also share your thoughts on this?
> 
> Current requirements from BPF side is straightforward: we just
> need a mechanism to “walk up one level and hold reference”. So
> most of the requirement comes from LandLock side. 

Have you thought about how to handle disconnected directories?

> 
> Thanks,
> Song
> 

^ permalink raw reply

* Re: [PATCH v2 08/13] bpf: Implement signature verification for BPF programs
From: KP Singh @ 2025-07-24 17:22 UTC (permalink / raw)
  To: James Bottomley
  Cc: bpf, linux-security-module, bboscaccy, paul, kys, ast, daniel,
	andrii
In-Reply-To: <c6ed224b9fb5db2cfac2620c75a49fa22cbaf617.camel@HansenPartnership.com>

On Wed, Jul 23, 2025 at 7:11 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Mon, 2025-07-21 at 23:19 +0200, KP Singh wrote:
> [...]
>
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index fd3b895ebebf..b42c3740e053 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1607,6 +1607,16 @@ union bpf_attr {
> >                * continuous.
> >                */
> >               __u32           fd_array_cnt;
> > +             /* Pointer to a buffer containing the signature of
> > the BPF
> > +              * program.
> > +              */
> > +             __aligned_u64   signature;
> > +             /* Size of the signature buffer in bytes. */
> > +             __u32           signature_size;
> > +             /* ID of the kernel keyring to be used for signature
> > +              * verification.
> > +              */
> > +             __u32           keyring_id;
>
> This should become __s32 to match the value passed in to
> bpf_lookup_user_key().
>
> [...]
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 22fda92ab7ce..111f91a99166 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -2779,8 +2779,41 @@ static bool is_perfmon_prog_type(enum
> > bpf_prog_type prog_type)
> >       }
> >  }
> >
> > +static noinline int bpf_prog_verify_signature(struct bpf_prog *prog,
> > +                                           union bpf_attr *attr,
> > +                                           bool is_kernel)
> > +{
> > +     bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
> > +     struct bpf_dynptr_kern sig_ptr, insns_ptr;
> > +     struct bpf_key *key = NULL;
> > +     void *sig;
> > +     int err = 0;
> > +
> > +     key = bpf_lookup_user_key(attr->keyring_id, 0);
> > +     if (!key)
> > +             return -ENOKEY;
>
> This still only checks against user keyrings and not system trusted
> keyrings as was pointed out in v1.  Since user keyrings are negative
> and user key serials begin at 3 or more, there's no overlap with the
> system keyring specifiers and you can just overload attr->keyring_id,
> like the below.
>
> Regards,
>
> James
>
> ---
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 111f91a99166..10fd3ea5d91f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -13,6 +13,7 @@
>  #include <linux/slab.h>
>  #include <linux/sched/signal.h>
>  #include <linux/vmalloc.h>
> +#include <linux/verification.h>
>  #include <linux/mmzone.h>
>  #include <linux/anon_inodes.h>
>  #include <linux/fdtable.h>
> @@ -2789,7 +2790,10 @@ static noinline int bpf_prog_verify_signature(struct bpf_prog *prog,
>         void *sig;
>         int err = 0;
>
> -       key = bpf_lookup_user_key(attr->keyring_id, 0);
> +       if (system_keyring_id_check(attr->keyring_id) == 0)
> +               key = bpf_lookup_system_key(attr->keyring_id);
> +       else
> +               key = bpf_lookup_user_key(attr->keyring_id, 0);
>         if (!key)
>                 return -ENOKEY;
>

Thanks James, updated.

- KP

>

^ permalink raw reply

* Re: [PATCH v3 2/4] landlock: Fix handling of disconnected directories
From: Mickaël Salaün @ 2025-07-24 17:10 UTC (permalink / raw)
  To: Günther Noack
  Cc: Tingmao Wang, Jann Horn, John Johansen, Al Viro, Ben Scarlato,
	Christian Brauner, Daniel Burgener, Jeff Xu, NeilBrown,
	Paul Moore, Ryan Sullivan, Song Liu, linux-fsdevel,
	linux-security-module
In-Reply-To: <aIJH9CoEKWNq0HwN@google.com>

On Thu, Jul 24, 2025 at 04:49:24PM +0200, Günther Noack wrote:
> On Wed, Jul 23, 2025 at 11:01:42PM +0200, Mickaël Salaün wrote:
> > On Tue, Jul 22, 2025 at 07:04:02PM +0100, Tingmao Wang wrote:
> > > On the other hand, I'm still a bit uncertain about the domain check
> > > semantics.  While it would not cause a rename to be allowed if it is
> > > otherwise not allowed by any rules on or above the mountpoint, this gets a
> > > bit weird if we have a situation where renames are allowed on the
> > > mountpoint or everywhere, but not read/writes, however read/writes are
> > > allowed directly on a file, but the dir containing that file gets
> > > disconnected so the sandboxed application can't read or write to it.
> > > (Maybe someone would set up such a policy where renames are allowed,
> > > expecting Landlock to always prevent renames where additional permissions
> > > would be exposed?)
> > > 
> > > In the above situation, if the file is then moved to a connected
> > > directory, it will become readable/writable again.
> > 
> > We can generalize this issue to not only the end file but any component
> > of the path: disconnected directories.  In fact, the main issue is the
> > potential inconsistency of access checks over time (e.g. between two
> > renames).  This could be exploited to bypass the security checks done
> > for FS_REFER.
> > 
> > I see two solutions:
> > 
> > 1. *Always* walk down to the IS_ROOT directory, and then jump to the
> >    mount point.  This makes it possible to have consistent access checks
> >    for renames and open/use.  The first downside is that that would
> >    change the current behavior for bind mounts that could get more
> >    access rights (if the policy explicitly sets rights for the hidden
> >    directories).  The second downside is that we'll do more walk.
> > 
> > 2. Return -EACCES (or -ENOENT) for actions involving disconnected
> >    directories, or renames of disconnected opened files.  This second
> >    solution is simpler and safer but completely disables the use of
> >    disconnected directories and the rename of disconnected files for
> >    sandboxed processes.
> > 
> > It would be much better to be able to handle opened directories as
> > (object) capabilities, but that is not currently possible because of the
> > way paths are handled by the VFS and LSM hooks.
> > 
> > Tingmao, Günther, Jann, what do you think?
> 
> I have to admit that so far, I still failed to wrap my head around the
> full patch set and its possible corner cases.  I hope I did not
> misunderstand things all too badly below:
> 
> As far as I understand the proposed patch, we are "checkpointing" the
> intermediate results of the path walk at every mount point boundary,
> and in the case where we run into a disconnected directory in one of
> the nested mount points, we restore from the intermediate result at
> the previous mount point directory and skip to the next mount point.

Correct

> 
> Visually speaking, if the layout is this (where ":" denotes a
> mountpoint boundary between the mountpoints MP1, MP2, MP3):
> 
>                           dirfd
>                             |
>           :                 V         :
> 	  :       ham <--- spam <--- eggs <--- x.txt
> 	  :    (disconn.)             :
>           :                           :
>   / <--- foo <--- bar <--- baz        :
>           :                           :
>     MP1                 MP2                  MP3
> 
> When a process holds a reference to the "spam" directory, which is now
> disconnected, and invokes openat(dirfd, "eggs/x.txt", ...), then we
> would:
> 
>   * traverse x.txt
>   * traverse eggs (checkpointing the intermediate result) <-.
>   * traverse spam                                           |
>   * traverse ham                                            |
>   * discover that ham is disconnected:                      |
>      * restore the intermediate result from "eggs" ---------'
>      * continue the walk at foo
>   * end up at the root
> 
> So effectively, since the results from "spam" and "ham" are discarded,
> we would traverse only the inodes in the outer and inner mountpoints
> MP1 and MP3, but effectively return a result that looks like we did
> not traverse MP2?

We'd still check MP2's inode, but otherwise yes.

> 
> Maybe (likely) I misread the code. :) It's not clear to me what the
> thinking behind this is.  Also, if there was another directory in
> between "spam" and "eggs" in MP2, wouldn't we be missing the access
> rights attached to this directory?

Yes, we would ignore this access right because we don't know that the
path was resolved from spam.

> 
> 
> Regarding the capability approach:
> 
> I agree that a "capability" approach would be the better solution, but
> it seems infeasible with the existing LSM hooks at the moment.  I
> would be in favor of it though.

Yes, it would be a new feature with potential important changes.

In the meantime, we still need a fix for disconnected directories, and
this fix needs to be backported.  That's why the capability approach is
not part of the two solutions. ;)

> 
> To spell it out a bit more explicitly what that would mean in my mind:
> 
> When a path is looked up relative to a dirfd, the path walk upwards
> would terminate at the dirfd and use previously calculated access
> rights stored in the associated struct file.  These access rights
> would be determined at the time of opening the dirfd, similar to how we
> are already storing the "truncate" access right today for regular
> files.
> 
> (Remark: There might still be corner cases where we have to solve it
> the hard way, if someone uses ".." together with a dirfd-relative
> lookup.)

Yep, real capabilities don't have ".." in their design.  On Linux (and
Landlock), we need to properly handle "..", which is challenging.

> 
> I also looked at what it would take to change the LSM hooks to pass
> the directory that the lookup was done relative to, but it seems that
> this would have to be passed through a bunch of VFS callbacks as well,
> which seems like a larger change.  I would be curious whether that
> would be deemed an acceptable change.
> 
> —Günther
> 
> 
> P.S. Related to relative directory lookups, there is some movement in
> the BSDs as well to use dirfds as capabilities, by adding a flag to
> open directories that enforces O_BENEATH on subsequent opens:
> 
>  * https://undeadly.org/cgi?action=article;sid=20250529080623
>  * https://reviews.freebsd.org/D50371
> 
> (both found via https://news.ycombinator.com/item?id=44575361)
> 
> If a dirfd had such a flag, that would get rid of the corner case
> above.

This would be nice but it would not solve the current issue because we
cannot force all processes to use this flag (which breaks some use
cases).

FYI, Capsicum is a more complete implementation:
https://man.freebsd.org/cgi/man.cgi?query=capsicum&sektion=4
See the vfs.lookup_cap_dotdot sysctl too.

^ permalink raw reply

* Re: [PATCH v2 11/13] bpftool: Add support for signing BPF programs
From: KP Singh @ 2025-07-24 17:07 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: bpf, linux-security-module, bboscaccy, paul, kys, ast, daniel,
	andrii
In-Reply-To: <2b417a1a-8f0b-4bca-ad44-aa4195040ef1@kernel.org>

On Tue, Jul 22, 2025 at 5:51 PM Quentin Monnet <qmo@kernel.org> wrote:
>
> 2025-07-21 23:19 UTC+0200 ~ KP Singh <kpsingh@kernel.org>
> > Two modes of operation being added:
> >
> > Add two modes of operation:
> >
> > * For prog load, allow signing a program immediately before loading. This
> >   is essential for command-line testing and administration.
> >
> >       bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> >
> > * For gen skeleton, embed a pre-generated signature into the C skeleton
> >   file. This supports the use of signed programs in compiled applications.
> >
> >       bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> >
> > Generation of the loader program and its metadata map is implemented in
> > libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
> > the program and automates the required steps: freezing the map, creating
> > an exclusive map, loading, and running. Users can use standard libbpf
> > APIs directly or integrate loader program generation into their own
> > toolchains.
>
>
> Thanks KP! Some bpftool-related comments below. Looks good overall, I
> mostly have minor comments.
>
> One concern might be the license for the new file, GPL-2.0 in your
> patch, whereas bpftool is dual-licensed. I hope this is simply an oversight?

An oversight, fixed.

>
>
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
> > ---
> >  .../bpf/bpftool/Documentation/bpftool-gen.rst |  12 +
> >  .../bpftool/Documentation/bpftool-prog.rst    |  12 +
> >  tools/bpf/bpftool/Makefile                    |   6 +-
> >  tools/bpf/bpftool/cgroup.c                    |   5 +-
> >  tools/bpf/bpftool/gen.c                       |  58 ++++-
> >  tools/bpf/bpftool/main.c                      |  21 +-
> >  tools/bpf/bpftool/main.h                      |  11 +
> >  tools/bpf/bpftool/prog.c                      |  25 +++
> >  tools/bpf/bpftool/sign.c                      | 210 ++++++++++++++++++
> >  9 files changed, 352 insertions(+), 8 deletions(-)
> >  create mode 100644 tools/bpf/bpftool/sign.c
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > index ca860fd97d8d..2997313003b1 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > @@ -185,6 +185,18 @@ OPTIONS
> >      For skeletons, generate a "light" skeleton (also known as "loader"
> >      skeleton). A light skeleton contains a loader eBPF program. It does not use
> >      the majority of the libbpf infrastructure, and does not need libelf.
>
>
> Blank line separator, please

done

>
>
> > +-S, --sign
> > +    For skeletons, generate a signed skeleton. This option must be used with
> > +    **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
> > +    See the "Signed Skeletons" section in the description of the
> > +    **gen skeleton** command for more details.
> > +
> > +-k <private_key.pem>
> > +    Path to the private key file in PEM format, required for signing.
> > +
> > +-i <certificate.x509>
> > +    Path to the X.509 certificate file in PEM or DER format, required for
> > +    signing.
>
>
> Please also update the options list in the SYNOPSIS section at the top
> of the page; and the option list at the bottom of gen.c (just like for
> "--use-loader").

done also, isn't this the right formatting for the SYNOPSIS given that
some of these are optional?

**bpftool** [*OPTIONS*] **prog** *COMMAND*
*OPTIONS* := { |COMMON_OPTIONS| [ { **-f** | **--bpffs** } ] [ {
**-m** | **--mapcompat** } ]
[ { **-n** | **--nomount** } ] [ { **-L** | **--use-loader** } ]
[ { { **-S** | **--sign** } **-k** <private_key.pem> **-i**
<certificate.x509> } ] }

not an expert here but I vaguely remember.

Also do you think we need to:

                { "use-loader", no_argument,    NULL,   'L' },
-               { "sign",       required_argument, NULL, 'S'},
+               { "sign",       no_argument,    NULL,   'S' },


Now that we don't use an argument blob for --sign?


>
> Can you also please take a look at the bash completion update? It
> shouldn't be too hard if you look at how it deals with other options, in
> particular --base-btf that also takes one argument - and I can help if
> necessary.

I will give it a go.

>
>
> >
> >  EXAMPLES
> >  ========
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index f69fd92df8d8..dc2ca196137e 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > @@ -248,6 +248,18 @@ OPTIONS
> >      creating the maps, and loading the programs (see **bpftool prog tracelog**
> >      as a way to dump those messages).
> >
> > +-S, --sign
> > +    Enable signing of the BPF program before loading. This option must be
> > +    used with **-k** and **-i**. Using this flag implicitly enables
> > +    **--use-loader**.
> > +
> > +-k <private_key.pem>
> > +    Path to the private key file in PEM format, required when signing.
> > +
> > +-i <certificate.x509>
> > +    Path to the X.509 certificate file in PEM or DER format, required when
> > +    signing.
>
>
> Same as for skeletons: please update the list of options in the synopsis
> and at the bottom of prog.c (bash completion for skeletons' options
> should also cover this case, so no additional work required here).
>
>
> > +
> >  EXAMPLES
> >  ========
> >  **# bpftool prog show**
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 9e9a5f006cd2..586d1b2595d1 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
> >  endif
> >  endif
> >
> > -LIBS = $(LIBBPF) -lelf -lz
> > -LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
> > +LIBS = $(LIBBPF) -lelf -lz -lcrypto
> > +LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
> >
> >  ifeq ($(feature-libelf-zstd),1)
> >  LIBS += -lzstd
> > @@ -194,7 +194,7 @@ endif
> >
> >  BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
> >
> > -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
> > +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
> >  $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
> >
> >  OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> > diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> > index 944ebe21a216..90c9aa297806 100644
> > --- a/tools/bpf/bpftool/cgroup.c
> > +++ b/tools/bpf/bpftool/cgroup.c
> > @@ -1,7 +1,10 @@
> >  // SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >  // Copyright (C) 2017 Facebook
> >  // Author: Roman Gushchin <guro@fb.com>
> > -
>
>
> Let's keep the blank line

Done

>
>
> > +#undef GCC_VERSION
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
>
>
> What are these for?

kpsingh@kpsingh-genoa:~/projects/linux/tools/bpf/bpftool$ vmk

Auto-detecting system features:
...                         clang-bpf-co-re: [ on  ]
...                                    llvm: [ on  ]
...                                  libcap: [ on  ]
...                                  libbfd: [ OFF ]

In file included from cgroup.c:19:
In file included from ./main.h:16:
/home/kpsingh/projects/linux/tools/bpf/bpftool/libbpf/include/bpf/skel_internal.h:87:9:
error: call to undeclared function 'syscall'; ISO C99 and later do not
support implicit function declarations
[-Wimplicit-function-declaration]
   87 |         return syscall(__NR_bpf, cmd, attr, size);
      |                ^
1 error generated.

>
>
> >  #define _XOPEN_SOURCE 500
> >  #include <errno.h>
> >  #include <fcntl.h>
>
> [...]
>
> > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> > index 2b7f2bd3a7db..fc25bb390ec7 100644
> > --- a/tools/bpf/bpftool/main.c
> > +++ b/tools/bpf/bpftool/main.c
> > @@ -33,6 +33,9 @@ bool relaxed_maps;
> >  bool use_loader;
> >  struct btf *base_btf;
> >  struct hashmap *refs_table;
> > +bool sign_progs;
> > +const char *private_key_path;
> > +const char *cert_path;
> >
> >  static void __noreturn clean_and_exit(int i)
> >  {
> > @@ -447,6 +450,7 @@ int main(int argc, char **argv)
> >               { "nomount",    no_argument,    NULL,   'n' },
> >               { "debug",      no_argument,    NULL,   'd' },
> >               { "use-loader", no_argument,    NULL,   'L' },
> > +             { "sign",       required_argument, NULL, 'S'},
> >               { "base-btf",   required_argument, NULL, 'B' },
> >               { 0 }
> >       };
> > @@ -473,7 +477,7 @@ int main(int argc, char **argv)
> >       bin_name = "bpftool";
> >
> >       opterr = 0;
> > -     while ((opt = getopt_long(argc, argv, "VhpjfLmndB:l",
> > +     while ((opt = getopt_long(argc, argv, "VhpjfLmndSi:k:B:l",
> >                                 options, NULL)) >= 0) {
> >               switch (opt) {
> >               case 'V':
> > @@ -519,6 +523,16 @@ int main(int argc, char **argv)
> >               case 'L':
> >                       use_loader = true;
> >                       break;
> > +             case 'S':
> > +                     sign_progs = true;
> > +                     use_loader = true;
> > +                     break;
> > +             case 'k':
> > +                     private_key_path = optarg;
> > +                     break;
> > +             case 'i':
> > +                     cert_path = optarg;
> > +                     break;
> >               default:
> >                       p_err("unrecognized option '%s'", argv[optind - 1]);
> >                       if (json_output)
> > @@ -533,6 +547,11 @@ int main(int argc, char **argv)
> >       if (argc < 0)
> >               usage();
> >
> > +     if (sign_progs && (private_key_path == NULL || cert_path == NULL)) {
> > +             p_err("-i <identity_x509_cert> and -k <private> key must be supplied with -S for signing");
> > +             return -EINVAL;
> > +     }
>
>
> What if -i and/or -k are passed without -S?

We can either print a warning or error out

A) User does not want to sign removes --sign and forgets to remove -i
-k (better with warning)
B) User wants to sign but forgets to --sign (better with error)

I'd say we print an error so that we don't accidentally not sign, WDYT?

The reason why I think we should keep an explicit --sign is because we
can also extend this to have e.g. --verify.

- KP

>
>
> > +
> >       if (version_requested)
> >               ret = do_version(argc, argv);
> >       else
> > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > index 6db704fda5c0..f921af3cda87 100644
> > --- a/tools/bpf/bpftool/main.h
> > +++ b/tools/bpf/bpftool/main.h
> > @@ -6,9 +6,14 @@
> >
> >  /* BFD and kernel.h both define GCC_VERSION, differently */
> >  #undef GCC_VERSION
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
> >  #include <stdbool.h>
> >  #include <stdio.h>
> > +#include <errno.h>
> >  #include <stdlib.h>
> > +#include <bpf/skel_internal.h>
>
>
> Wnat do you need these includes (and _GNU_SOURCE) in main.h for?

Explained above, let me know if you have better ideas on where to place these.

>
>
> >  #include <linux/bpf.h>
> >  #include <linux/compiler.h>
> >  #include <linux/kernel.h>
>
> [...]
>
> > diff --git a/tools/bpf/bpftool/sign.c b/tools/bpf/bpftool/sign.c
> > new file mode 100644
> > index 000000000000..f0b5dd10a46b
> > --- /dev/null
> > +++ b/tools/bpf/bpftool/sign.c
> > @@ -0,0 +1,210 @@
> > +// SPDX-License-Identifier: GPL-2.0
>
>
> Please consider making this file dual-licensed like the rest of
> bpftool's source code, "(GPL-2.0-only OR BSD-2-Clause)".

Done.

>
>
> > +
> > +/*
> > + * Copyright (C) 2022 Google LLC.
>
>
> 2025?

Let's keep it 2022, nah just kidding :) Thanks.

>
>
> > + */
> > +#define _GNU_SOURCE
>
>
> Please guard this:
>
>         #ifndef _GNU_SOURCE
>         #define _GNU_SOURCE
>         #endif
>
> This is because "llvm-config --cflags" passes -D_GNU_SOURCE and we may
> end up with a duplicate definition, otherwise.

ack, done.

>
>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +#include <string.h>
> > +#include <string.h>
> > +#include <getopt.h>
> > +#include <err.h>
> > +#include <openssl/opensslv.h>
> > +#include <openssl/bio.h>
> > +#include <openssl/evp.h>
> > +#include <openssl/pem.h>
> > +#include <openssl/err.h>
> > +#include <openssl/cms.h>
> > +#include <linux/keyctl.h>
> > +#include <errno.h>
> > +
> > +#include <bpf/skel_internal.h>
> > +
> > +#include "main.h"
> > +
> > +#define OPEN_SSL_ERR_BUF_LEN 256
> > +
> > +static void display_openssl_errors(int l)
> > +{
> > +     char buf[OPEN_SSL_ERR_BUF_LEN];
> > +     const char *file;
> > +     const char *data;
> > +     unsigned long e;
> > +     int flags;
> > +     int line;
> > +
> > +     while ((e = ERR_get_error_all(&file, &line, NULL, &data, &flags))) {
> > +             ERR_error_string_n(e, buf, sizeof(buf));
> > +             if (data && (flags & ERR_TXT_STRING)) {
> > +                     p_err("OpenSSL %s: %s:%d: %s\n", buf, file, line, data);
>
>
> Please remove the trailing '\n', p_err() handles it already.
>
>
> > +             } else {
> > +                     p_err("OpenSSL %s: %s:%d\n", buf, file, line);
>
>
> Same here.

done.

- KP

>
> [...]

^ permalink raw reply

* Re: [RFC PATCH v2 15/34] lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
From: Casey Schaufler @ 2025-07-24 15:48 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-51-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Rename ordered_lsm_parse() to lsm_order_parse() for the sake of
> consistency with the other LSM initialization routines, and also
> do some minor rework of the function.  Aside from some minor style
> decisions, the majority of the rework involved shuffling the order
> of the LSM_FLAG_LEGACY and LSM_ORDER_FIRST code so that the
> LSM_FLAG_LEGACY checks are handled first; it is important to note
> that this doesn't affect the order in which the LSMs are registered.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 82 ++++++++++++++++++++-------------------------
>  1 file changed, 37 insertions(+), 45 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 8c632ab77da9..b1156f414491 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -225,83 +225,75 @@ static void __init initialize_lsm(struct lsm_info *lsm)
>  	}
>  }
>  
> -/* Populate ordered LSMs list from comma-separated LSM name list. */
> -static void __init ordered_lsm_parse(const char *order, const char *origin)
> +/**
> + * lsm_order_parse - Parse the comma delimited LSM list
> + * @list: LSM list
> + * @src: source of the list
> + */
> +static void __init lsm_order_parse(const char *list, const char *src)
>  {
>  	struct lsm_info *lsm;
>  	char *sep, *name, *next;
>  
> -	/* LSM_ORDER_FIRST is always first. */
> -	lsm_for_each_raw(lsm) {
> -		if (lsm->order == LSM_ORDER_FIRST)
> -			lsm_order_append(lsm, "  first");
> -	}
> -
> -	/* Process "security=", if given. */
> +	/* Handle any Legacy LSM exclusions if one was specified. */
>  	if (lsm_order_legacy) {
> -		struct lsm_info *major;
> -
>  		/*
> -		 * To match the original "security=" behavior, this
> -		 * explicitly does NOT fallback to another Legacy Major
> -		 * if the selected one was separately disabled: disable
> -		 * all non-matching Legacy Major LSMs.
> +		 * To match the original "security=" behavior, this explicitly
> +		 * does NOT fallback to another Legacy Major if the selected
> +		 * one was separately disabled: disable all non-matching
> +		 * Legacy Major LSMs.
>  		 */
> -		lsm_for_each_raw(major) {
> -			if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&
> -			    strcmp(major->id->name, lsm_order_legacy) != 0) {
> -				lsm_enabled_set(major, false);
> +		lsm_for_each_raw(lsm) {
> +			if ((lsm->flags & LSM_FLAG_LEGACY_MAJOR) &&
> +			     strcmp(lsm->id->name, lsm_order_legacy)) {
> +				lsm_enabled_set(lsm, false);
>  				init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> -					   lsm_order_legacy, major->id->name);
> +					   lsm_order_legacy, lsm->id->name);
>  			}
>  		}
>  	}
>  
> -	sep = kstrdup(order, GFP_KERNEL);
> +	/* LSM_ORDER_FIRST */
> +	lsm_for_each_raw(lsm) {
> +		if (lsm->order == LSM_ORDER_FIRST)
> +			lsm_order_append(lsm, "first");
> +	}
> +
> +	/* Normal or "mutable" LSMs */
> +	sep = kstrdup(list, GFP_KERNEL);
>  	next = sep;
>  	/* Walk the list, looking for matching LSMs. */
>  	while ((name = strsep(&next, ",")) != NULL) {
> -		bool found = false;
> -
>  		lsm_for_each_raw(lsm) {
> -			if (strcmp(lsm->id->name, name) == 0) {
> -				if (lsm->order == LSM_ORDER_MUTABLE)
> -					lsm_order_append(lsm, origin);
> -				found = true;
> -			}
> +			if (!strcmp(lsm->id->name, name) &&
> +			    lsm->order == LSM_ORDER_MUTABLE)
> +				lsm_order_append(lsm, src);
>  		}
> -
> -		if (!found)
> -			init_debug("%s ignored: %s (not built into kernel)\n",
> -				   origin, name);
>  	}
> +	kfree(sep);
>  
> -	/* Process "security=", if given. */
> +	/* Legacy LSM if specified. */
>  	if (lsm_order_legacy) {
>  		lsm_for_each_raw(lsm) {
> -			if (lsm_order_exists(lsm))
> -				continue;
> -			if (strcmp(lsm->id->name, lsm_order_legacy) == 0)
> -				lsm_order_append(lsm, "security=");
> +			if (!strcmp(lsm->id->name, lsm_order_legacy))
> +				lsm_order_append(lsm, src);
>  		}
>  	}
>  
> -	/* LSM_ORDER_LAST is always last. */
> +	/* LSM_ORDER_LAST */
>  	lsm_for_each_raw(lsm) {
>  		if (lsm->order == LSM_ORDER_LAST)
> -			lsm_order_append(lsm, "   last");
> +			lsm_order_append(lsm, "last");
>  	}
>  
> -	/* Disable all LSMs not in the ordered list. */
> +	/* Disable all LSMs not previously enabled. */
>  	lsm_for_each_raw(lsm) {
>  		if (lsm_order_exists(lsm))
>  			continue;
>  		lsm_enabled_set(lsm, false);
>  		init_debug("%s skipped: %s (not in requested order)\n",
> -			   origin, lsm->id->name);
> +			   src, lsm->id->name);
>  	}
> -
> -	kfree(sep);
>  }
>  
>  /**
> @@ -319,9 +311,9 @@ static void __init lsm_init_ordered(void)
>  				lsm_order_legacy, lsm_order_cmdline);
>  			lsm_order_legacy = NULL;
>  		}
> -		ordered_lsm_parse(lsm_order_cmdline, "cmdline");
> +		lsm_order_parse(lsm_order_cmdline, "cmdline");
>  	} else
> -		ordered_lsm_parse(lsm_order_builtin, "builtin");
> +		lsm_order_parse(lsm_order_builtin, "builtin");
>  
>  	lsm_order_for_each(lsm) {
>  		lsm_prepare(*lsm);

^ permalink raw reply

* Re: [RFC PATCH v2 14/34] lsm: rename/rework append_ordered_lsm() into lsm_order_append()
From: Casey Schaufler @ 2025-07-24 15:47 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-50-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Rename append_ordered_lsm() to lsm_order_append() to better match
> convention and do some rework.  The rework includes moving the
> LSM_FLAG_EXCLUSIVE logic from lsm_prepare() to lsm_order_append()
> in order to consolidate the individual LSM append/activation code,
> and adding logic to skip appending explicitly disabled LSMs to the
> active LSM list.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 76 +++++++++++++++++++++++++--------------------
>  1 file changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 01825da2755f..8c632ab77da9 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -124,24 +124,48 @@ static bool __init lsm_order_exists(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
> + *
> + * Append @lsm to the enabled LSM array after ensuring that it hasn't been
> + * explicitly disabled, is a duplicate entry, or would run afoul of the
> + * LSM_FLAG_EXCLUSIVE logic.
> + */
> +static void __init lsm_order_append(struct lsm_info *lsm, const char *src)
>  {
>  	/* Ignore duplicate selections. */
>  	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))
> +		goto out;
>  
> -	/* 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;
> +	if (WARN(lsm_active_cnt == MAX_LSM_COUNT,
> +		 "%s: out of LSM static calls!?\n", src)) {
> +		lsm_enabled_set(lsm, false);
> +		goto out;
> +	}
>  
> -	init_debug("%s ordered: %s (%s)\n", from, lsm->id->name,
> +	if (lsm->flags & LSM_FLAG_EXCLUSIVE) {
> +		if (lsm_exclusive) {
> +			init_debug("exclusive disabled: %s\n", lsm->id->name);
> +			lsm_enabled_set(lsm, false);
> +			goto out;
> +		} else {
> +			init_debug("exclusive chosen:   %s\n", lsm->id->name);
> +			lsm_exclusive = lsm;
> +		}
> +	}
> +
> +	lsm_enabled_set(lsm, true);
> +	lsm_order[lsm_active_cnt] = lsm;
> +	lsm_idlist[lsm_active_cnt++] = lsm->id;
> +
> +out:
> +	init_debug("%s ordered: %s (%s)\n", src, lsm->id->name,
>  		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
> @@ -163,26 +187,12 @@ static void __init lsm_set_blob_size(int *need, int *lbs)
>   */
>  static void __init lsm_prepare(struct lsm_info *lsm)
>  {
> -	struct lsm_blob_sizes *blobs;
> +	struct lsm_blob_sizes *blobs = lsm->blobs;
>  
> -	if (!lsm_is_enabled(lsm)) {
> -		lsm_enabled_set(lsm, false);
> +	if (!blobs)
>  		return;
> -	} else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && lsm_exclusive) {
> -		init_debug("exclusive disabled: %s\n", lsm->id->name);
> -		lsm_enabled_set(lsm, false);
> -		return;
> -	}
> -
> -	/* Mark the LSM as enabled. */
> -	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;
> -	}
>  
>  	/* Register the LSM blob sizes. */
> -	blobs = lsm->blobs;
>  	lsm_set_blob_size(&blobs->lbs_cred, &blob_sizes.lbs_cred);
>  	lsm_set_blob_size(&blobs->lbs_file, &blob_sizes.lbs_file);
>  	lsm_set_blob_size(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> @@ -224,7 +234,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	/* LSM_ORDER_FIRST is always first. */
>  	lsm_for_each_raw(lsm) {
>  		if (lsm->order == LSM_ORDER_FIRST)
> -			append_ordered_lsm(lsm, "  first");
> +			lsm_order_append(lsm, "  first");
>  	}
>  
>  	/* Process "security=", if given. */
> @@ -256,7 +266,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  		lsm_for_each_raw(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;
>  			}
>  		}
> @@ -272,14 +282,14 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  			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. */
>  	lsm_for_each_raw(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. */
> @@ -409,8 +419,8 @@ 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_order_append(lsm, "early");
>  		lsm_prepare(lsm);
>  		initialize_lsm(lsm);
>  	}

^ permalink raw reply

* Re: [RFC PATCH v2 13/34] lsm: rename exists_ordered_lsm() to lsm_order_exists()
From: Casey Schaufler @ 2025-07-24 15:45 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-49-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Also add a header comment block to the function.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 14bb6f4b4628..01825da2755f 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -108,8 +108,11 @@ static inline bool lsm_is_enabled(struct lsm_info *lsm)
>  	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;
>  
> @@ -126,7 +129,7 @@ static int last_lsm __initdata;
>  static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)
>  {
>  	/* 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))
> @@ -266,7 +269,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	/* Process "security=", if given. */
>  	if (lsm_order_legacy) {
>  		lsm_for_each_raw(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=");
> @@ -281,7 +284,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  
>  	/* Disable all LSMs not in the ordered list. */
>  	lsm_for_each_raw(lsm) {
> -		if (exists_ordered_lsm(lsm))
> +		if (lsm_order_exists(lsm))
>  			continue;
>  		lsm_enabled_set(lsm, false);
>  		init_debug("%s skipped: %s (not in requested order)\n",

^ permalink raw reply

* Re: [RFC PATCH v2 12/34] lsm: rework the LSM enable/disable setter/getter functions
From: Casey Schaufler @ 2025-07-24 15:44 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-48-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> In addition to style changes, rename set_enabled() to lsm_enabled_set()
> and is_enabled() to lsm_is_enabled() to better fit within the LSM
> initialization code.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 62 ++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 80b57aef38a0..14bb6f4b4628 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -10,6 +10,10 @@
>  
>  #include "lsm.h"
>  
> +/* LSM enabled constants. */
> +static __initdata int lsm_enabled_true = 1;
> +static __initdata 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[];
> @@ -75,37 +79,33 @@ 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;
> +	return (lsm->enabled ? *lsm->enabled : false);
>  }
>  
>  /* Is an LSM already listed in the ordered LSMs list? */
> @@ -139,7 +139,7 @@ static void __init append_ordered_lsm(struct lsm_info *lsm, const char *from)
>  	lsm_idlist[last_lsm++] = lsm->id;
>  
>  	init_debug("%s ordered: %s (%s)\n", from, lsm->id->name,
> -		   is_enabled(lsm) ? "enabled" : "disabled");
> +		   lsm_is_enabled(lsm) ? "enabled" : "disabled");
>  }
>  
>  static void __init lsm_set_blob_size(int *need, int *lbs)
> @@ -162,17 +162,17 @@ static void __init lsm_prepare(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;
> @@ -203,7 +203,7 @@ static void __init lsm_prepare(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);
> @@ -237,7 +237,7 @@ 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->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);
>  			}
> @@ -283,7 +283,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	lsm_for_each_raw(lsm) {
>  		if (exists_ordered_lsm(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);
>  	}
> @@ -316,12 +316,12 @@ 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",
>  				first++ == 0 ? "" : ",", early->id->name);
>  	}
>  	lsm_order_for_each(lsm) {
> -		if (is_enabled(*lsm))
> +		if (lsm_is_enabled(*lsm))
>  			pr_cont("%s%s",
>  				first++ == 0 ? "" : ",", (*lsm)->id->name);
>  	}
> @@ -434,7 +434,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. */

^ permalink raw reply

* Re: [RFC PATCH v2 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Casey Schaufler @ 2025-07-24 15:39 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-47-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> The LSM currently has a lot of code to maintain a list of the currently
> active LSMs in a human readable string, with the only user being the
> "/sys/kernel/security/lsm" code.  Let's drop all of that code and
> generate the string on first use and then cache it for subsequent use.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  include/linux/lsm_hooks.h |  1 -
>  security/inode.c          | 59 +++++++++++++++++++++++++++++++++++++--
>  security/lsm_init.c       | 49 --------------------------------
>  3 files changed, 57 insertions(+), 52 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 4cd17c9a229f..bc477fb20d02 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -169,7 +169,6 @@ struct lsm_info {
>  
>  
>  /* DO NOT tamper with these variables outside of the LSM framework */
> -extern char *lsm_names;
>  extern struct lsm_static_calls_table static_calls_table __ro_after_init;
>  
>  /**
> diff --git a/security/inode.c b/security/inode.c
> index 3913501621fa..68ee6c9de833 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -22,6 +22,8 @@
>  #include <linux/lsm_hooks.h>
>  #include <linux/magic.h>
>  
> +#include "lsm.h"
> +
>  static struct vfsmount *mount;
>  static int mount_count;
>  
> @@ -339,12 +341,65 @@ void securityfs_recursive_remove(struct dentry *dentry)
>  EXPORT_SYMBOL_GPL(securityfs_recursive_remove);
>  
>  #ifdef CONFIG_SECURITY
> +#include <linux/spinlock.h>
> +
>  static struct dentry *lsm_dentry;
> +
> +/* NOTE: we never free the string below once it is set. */
> +static DEFINE_SPINLOCK(lsm_read_lock);
> +static char *lsm_read_str = NULL;
> +static ssize_t lsm_read_len = 0;
> +
>  static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count,
>  			loff_t *ppos)
>  {
> -	return simple_read_from_buffer(buf, count, ppos, lsm_names,
> -		strlen(lsm_names));
> +	int i;
> +	char *str;
> +	ssize_t len;
> +
> +restart:
> +
> +	rcu_read_lock();
> +	if (!lsm_read_str) {
> +		/* we need to generate the string and try again */
> +		rcu_read_unlock();
> +		goto generate_string;
> +	}
> +	len = simple_read_from_buffer(buf, count, ppos,
> +				      rcu_dereference(lsm_read_str),
> +				      lsm_read_len);
> +	rcu_read_unlock();
> +	return len;
> +
> +generate_string:
> +
> +	for (i = 0; i < lsm_active_cnt; i++)
> +		/* the '+ 1' accounts for either a comma or a NUL */
> +		len += strlen(lsm_idlist[i]->name) + 1;
> +
> +	str = kmalloc(len, GFP_KERNEL);
> +	if (!str)
> +		return -ENOMEM;
> +	str[0] = '\0';
> +
> +	for (i = 0; i < lsm_active_cnt; i++) {
> +		if (i > 0)
> +			strcat(str, ",");
> +		strcat(str, lsm_idlist[i]->name);
> +	}
> +
> +	spin_lock(&lsm_read_lock);
> +	if (lsm_read_str) {
> +		/* we raced and lost */
> +		spin_unlock(&lsm_read_lock);
> +		kfree(str);
> +		goto restart;
> +	}
> +	lsm_read_str = str; 
> +	lsm_read_len = len;

You're going to get a nul byte at the end of the string because
you accounted for the ',' above, but there isn't one at the end
of the string.

> +	spin_unlock(&lsm_read_lock);
> +
> +	goto restart;
>  }
>  
>  static const struct file_operations lsm_ops = {
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 03d3e140e0b1..80b57aef38a0 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -10,8 +10,6 @@
>  
>  #include "lsm.h"
>  
> -char *lsm_names;
> -
>  /* 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[];
> @@ -365,42 +363,6 @@ static void __init lsm_init_ordered(void)
>  	}
>  }
>  
> -static bool match_last_lsm(const char *list, const char *lsm)
> -{
> -	const char *last;
> -
> -	if (WARN_ON(!list || !lsm))
> -		return false;
> -	last = strrchr(list, ',');
> -	if (last)
> -		/* Pass the comma, strcmp() will check for '\0' */
> -		last++;
> -	else
> -		last = list;
> -	return !strcmp(last, lsm);
> -}
> -
> -static int lsm_append(const char *new, char **result)
> -{
> -	char *cp;
> -
> -	if (*result == NULL) {
> -		*result = kstrdup(new, GFP_KERNEL);
> -		if (*result == NULL)
> -			return -ENOMEM;
> -	} else {
> -		/* Check if it is the last registered name */
> -		if (match_last_lsm(*result, new))
> -			return 0;
> -		cp = kasprintf(GFP_KERNEL, "%s,%s", *result, new);
> -		if (cp == NULL)
> -			return -ENOMEM;
> -		kfree(*result);
> -		*result = cp;
> -	}
> -	return 0;
> -}
> -
>  static void __init lsm_static_call_init(struct security_hook_list *hl)
>  {
>  	struct lsm_static_call *scall = hl->scalls;
> @@ -437,15 +399,6 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  		hooks[i].lsmid = lsmid;
>  		lsm_static_call_init(&hooks[i]);
>  	}
> -
> -	/*
> -	 * Don't try to append during early_security_init(), we'll come back
> -	 * and fix this up afterwards.
> -	 */
> -	if (slab_is_available()) {
> -		if (lsm_append(lsmid->name, &lsm_names) < 0)
> -			panic("%s - Cannot get early memory.\n", __func__);
> -	}
>  }
>  
>  int __init early_security_init(void)
> @@ -482,8 +435,6 @@ 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");
> -		if (lsm->enabled)
> -			lsm_append(lsm->id->name, &lsm_names);
>  	}
>  
>  	/* Load LSMs in specified order. */

^ permalink raw reply

* Re: [RFC PATCH v2 10/34] lsm: rework lsm_active_cnt and lsm_idlist[]
From: Casey Schaufler @ 2025-07-24 15:34 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-46-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Move the LSM active count and lsm_id list declarations out of a header
> that is visible across the kernel and into a header that is limited to
> the LSM framework.  This not only helps keep the include/linux headers
> smaller and cleaner, it helps prevent misuse of these variables.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

One nit below, otherwise

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  include/linux/security.h | 2 --
>  security/lsm.h           | 5 +++++
>  security/lsm_init.c      | 8 +-------
>  security/lsm_syscalls.c  | 2 ++
>  security/security.c      | 3 +++
>  5 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e8d9f6069f0c..c57a9a7499ea 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -167,8 +167,6 @@ struct lsm_prop {
>  };
>  
>  extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
> -extern u32 lsm_active_cnt;
> -extern const struct lsm_id *lsm_idlist[];
>  
>  /* These functions are in security/commoncap.c */
>  extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> diff --git a/security/lsm.h b/security/lsm.h
> index 0e1731bad4a7..dbe755c45e57 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -7,6 +7,11 @@
>  #define _LSM_H_
>  
>  #include <linux/lsm_hooks.h>
> +#include <linux/lsm_count.h>
> +
> +/* List of configured LSMs */
> +extern unsigned int lsm_active_cnt;
> +extern const struct lsm_id *lsm_idlist[];
>  
>  /* LSM blob configuration */
>  extern struct lsm_blob_sizes blob_sizes;
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index cbdfac31ede4..03d3e140e0b1 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -22,8 +22,8 @@ static __initdata const char *lsm_order_cmdline;
>  static __initdata const char *lsm_order_legacy;
>  
>  /* Ordered list of LSMs to initialize. */
> -static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
>  static __initdata struct lsm_info *lsm_exclusive;
> +static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];

I can't see there's a good reason for the reordering. I'm not
objecting to it, but it's curious.

>  
>  static __initdata bool debug;
>  #define init_debug(...)							\
> @@ -214,12 +214,6 @@ static void __init initialize_lsm(struct lsm_info *lsm)
>  	}
>  }
>  
> -/*
> - * Current index to use while initializing the lsm id list.
> - */
> -u32 lsm_active_cnt __ro_after_init;
> -const struct lsm_id *lsm_idlist[MAX_LSM_COUNT];
> -
>  /* Populate ordered LSMs list from comma-separated LSM name list. */
>  static void __init ordered_lsm_parse(const char *order, const char *origin)
>  {
> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> index 8440948a690c..5648b1f0ce9c 100644
> --- a/security/lsm_syscalls.c
> +++ b/security/lsm_syscalls.c
> @@ -17,6 +17,8 @@
>  #include <linux/lsm_hooks.h>
>  #include <uapi/linux/lsm.h>
>  
> +#include "lsm.h"
> +
>  /**
>   * lsm_name_to_attr - map an LSM attribute name to its ID
>   * @name: name of the attribute
> diff --git a/security/security.c b/security/security.c
> index e77791058086..106f225f4a80 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -73,6 +73,9 @@ const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = {
>  	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>  };
>  
> +unsigned int lsm_active_cnt __ro_after_init;
> +const struct lsm_id *lsm_idlist[MAX_LSM_COUNT];
> +
>  struct lsm_blob_sizes blob_sizes;
>  
>  struct kmem_cache *lsm_file_cache;

^ permalink raw reply

* Re: [RFC PATCH v2 09/34] lsm: rename the lsm order variables for consistency
From: Casey Schaufler @ 2025-07-24 15:31 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-45-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> Rename the builtin_lsm_order variable to lsm_order_builtin,
> chosen_lsm_order to lsm_order_cmdline, chosen_major_lsm to
> lsm_order_legacy, ordered_lsms[] to lsm_order[], and exclusive
> to lsm_exclusive.
>
> This patch also renames the associated kernel command line parsing
> functions and adds some basic function comment blocks.  The parsing
> function choose_major_lsm() was renamed to lsm_choose_security(),
> choose_lsm_order() to lsm_choose_lsm(), and enable_debug() to
> lsm_debug_enable().
>
> Reviewed-by: Kees Cook <kees@kernel.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 86 +++++++++++++++++++++++++--------------------
>  1 file changed, 48 insertions(+), 38 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 96d51e4d625b..cbdfac31ede4 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -16,14 +16,14 @@ char *lsm_names;
>  extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
>  extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>  
> -/* Boot-time LSM user choice */
> -static __initconst const char *const builtin_lsm_order = CONFIG_LSM;
> -static __initdata const char *chosen_lsm_order;
> -static __initdata const char *chosen_major_lsm;
> +/* Build and boot-time LSM ordering. */
> +static __initconst const char *const lsm_order_builtin = CONFIG_LSM;
> +static __initdata const char *lsm_order_cmdline;
> +static __initdata const char *lsm_order_legacy;
>  
>  /* Ordered list of LSMs to initialize. */
> -static __initdata struct lsm_info *ordered_lsms[MAX_LSM_COUNT + 1];
> -static __initdata struct lsm_info *exclusive;
> +static __initdata struct lsm_info *lsm_order[MAX_LSM_COUNT + 1];
> +static __initdata struct lsm_info *lsm_exclusive;
>  
>  static __initdata bool debug;
>  #define init_debug(...)							\
> @@ -33,7 +33,7 @@ static __initdata bool debug;
>  	} while (0)
>  
>  #define lsm_order_for_each(iter)					\
> -	for ((iter) = ordered_lsms; *(iter); (iter)++)
> +	for ((iter) = lsm_order; *(iter); (iter)++)
>  #define lsm_for_each_raw(iter)						\
>  	for ((iter) = __start_lsm_info;					\
>  	     (iter) < __end_lsm_info; (iter)++)
> @@ -41,31 +41,41 @@ static __initdata bool debug;
>  	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 */
> -static int __init choose_major_lsm(char *str)
> +/**
> + * lsm_choose_security - Legacy "major" LSM selection
> + * @str: kernel command line parameter
> + */
> +static int __init lsm_choose_security(char *str)
>  {
> -	chosen_major_lsm = str;
> +	lsm_order_legacy = str;
>  	return 1;
>  }
> -__setup("security=", choose_major_lsm);
> +__setup("security=", lsm_choose_security);
>  
> -/* Explicitly choose LSM initialization order. */
> -static int __init choose_lsm_order(char *str)
> +/**
> + * lsm_choose_lsm - Modern LSM selection
> + * @str: kernel command line parameter
> + */
> +static int __init lsm_choose_lsm(char *str)
>  {
> -	chosen_lsm_order = str;
> +	lsm_order_cmdline = str;
>  	return 1;
>  }
> -__setup("lsm=", choose_lsm_order);
> +__setup("lsm=", lsm_choose_lsm);
>  
> -/* Enable LSM order debugging. */
> -static int __init enable_debug(char *str)
> +/**
> + * lsm_debug_enable - Enable LSM framework debugging
> + * @str: kernel command line parameter
> + *
> + * Currently we only provide debug info during LSM initialization, but we may
> + * want to expand this in the future.
> + */
> +static int __init lsm_debug_enable(char *str)
>  {
>  	debug = true;
>  	return 1;
>  }
> -__setup("lsm.debug", enable_debug);
> +__setup("lsm.debug", lsm_debug_enable);
>  
>  /* Mark an LSM's enabled flag. */
>  static int lsm_enabled_true __initdata = 1;
> @@ -127,7 +137,7 @@ 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;
> +	lsm_order[last_lsm] = lsm;
>  	lsm_idlist[last_lsm++] = lsm->id;
>  
>  	init_debug("%s ordered: %s (%s)\n", from, lsm->id->name,
> @@ -157,7 +167,7 @@ static void __init lsm_prepare(struct lsm_info *lsm)
>  	if (!is_enabled(lsm)) {
>  		set_enabled(lsm, false);
>  		return;
> -	} else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && exclusive) {
> +	} else if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && lsm_exclusive) {
>  		init_debug("exclusive disabled: %s\n", lsm->id->name);
>  		set_enabled(lsm, false);
>  		return;
> @@ -165,9 +175,9 @@ 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) {
> +	if ((lsm->flags & LSM_FLAG_EXCLUSIVE) && !lsm_exclusive) {
>  		init_debug("exclusive chosen:   %s\n", lsm->id->name);
> -		exclusive = lsm;
> +		lsm_exclusive = lsm;
>  	}
>  
>  	/* Register the LSM blob sizes. */
> @@ -223,7 +233,7 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	}
>  
>  	/* Process "security=", if given. */
> -	if (chosen_major_lsm) {
> +	if (lsm_order_legacy) {
>  		struct lsm_info *major;
>  
>  		/*
> @@ -234,10 +244,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->id->name, chosen_major_lsm) != 0) {
> +			    strcmp(major->id->name, lsm_order_legacy) != 0) {
>  				set_enabled(major, false);
>  				init_debug("security=%s disabled: %s (only one legacy major LSM)\n",
> -					   chosen_major_lsm, major->id->name);
> +					   lsm_order_legacy, major->id->name);
>  			}
>  		}
>  	}
> @@ -262,11 +272,11 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	}
>  
>  	/* Process "security=", if given. */
> -	if (chosen_major_lsm) {
> +	if (lsm_order_legacy) {
>  		lsm_for_each_raw(lsm) {
>  			if (exists_ordered_lsm(lsm))
>  				continue;
> -			if (strcmp(lsm->id->name, chosen_major_lsm) == 0)
> +			if (strcmp(lsm->id->name, lsm_order_legacy) == 0)
>  				append_ordered_lsm(lsm, "security=");
>  		}
>  	}
> @@ -298,15 +308,15 @@ static void __init lsm_init_ordered(void)
>  	struct lsm_info **lsm;
>  	struct lsm_info *early;
>  
> -	if (chosen_lsm_order) {
> -		if (chosen_major_lsm) {
> +	if (lsm_order_cmdline) {
> +		if (lsm_order_legacy) {
>  			pr_warn("security=%s is ignored because it is superseded by lsm=%s\n",
> -				chosen_major_lsm, chosen_lsm_order);
> -			chosen_major_lsm = NULL;
> +				lsm_order_legacy, lsm_order_cmdline);
> +			lsm_order_legacy = NULL;
>  		}
> -		ordered_lsm_parse(chosen_lsm_order, "cmdline");
> +		ordered_lsm_parse(lsm_order_cmdline, "cmdline");
>  	} else
> -		ordered_lsm_parse(builtin_lsm_order, "builtin");
> +		ordered_lsm_parse(lsm_order_builtin, "builtin");
>  
>  	lsm_order_for_each(lsm) {
>  		lsm_prepare(*lsm);
> @@ -467,9 +477,9 @@ int __init security_init(void)
>  {
>  	struct lsm_info *lsm;
>  
> -	init_debug("legacy security=%s\n", chosen_major_lsm ? : " *unspecified*");
> -	init_debug("  CONFIG_LSM=%s\n", builtin_lsm_order);
> -	init_debug("boot arg lsm=%s\n", chosen_lsm_order ? : " *unspecified*");
> +	init_debug("legacy security=%s\n", lsm_order_legacy ? : " *unspecified*");
> +	init_debug("  CONFIG_LSM=%s\n", lsm_order_builtin);
> +	init_debug("boot arg lsm=%s\n", lsm_order_cmdline ? : " *unspecified*");
>  
>  	/*
>  	 * Append the names of the early LSM modules now that kmalloc() is

^ permalink raw reply

* Re: [RFC PATCH v2 08/34] lsm: replace the name field with a pointer to the lsm_id struct
From: Casey Schaufler @ 2025-07-24 15:30 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-44-paul@paul-moore.com>

On 7/21/2025 4:21 PM, 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>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Significant improvement.

Reviewed-by: Casey Schaufler <casey@schaufler-ca.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 eeb4bfd60b79..4cd17c9a229f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -149,7 +149,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 9b6c2f157f83..a7f6a3274682 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -2272,7 +2272,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 28d4248bf001..e04aa4f50eaf 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1509,7 +1509,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 f99ab1a3b0f0..ded971bdeaae 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1253,7 +1253,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 c8af26a6ce14..96d51e4d625b 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;
>  	}
>  
> @@ -197,9 +198,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);
>  	}
>  }
>  
> @@ -233,10 +234,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);
>  			}
>  		}
>  	}
> @@ -248,7 +249,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;
> @@ -265,7 +266,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=");
>  		}
>  	}
> @@ -282,7 +283,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);
> @@ -314,11 +315,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");
>  
> @@ -426,18 +429,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]);
> @@ -485,10 +476,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 595ceb314aeb..d7ec6bc6168b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7640,7 +7640,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,
>  };

^ permalink raw reply

* Re: [RFC PATCH v2 05/34] lsm: integrate report_lsm_order() code into caller
From: Casey Schaufler @ 2025-07-24 15:19 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-41-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> With only one caller of report_lsm_order(), insert the function's code
> directly into the caller and ger rid of report_lsm_order().
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 33 ++++++++++++---------------------
>  1 file changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index a73c3769dfea..d20c6df55e7d 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -288,26 +288,6 @@ 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. */
> -	lsm_early_for_each_raw(early) {
> -		if (is_enabled(early))
> -			pr_cont("%s%s", first++ == 0 ? "" : ",", early->name);
> -	}
> -	lsm_order_for_each(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
> @@ -338,7 +318,9 @@ static void __init lsm_early_task(struct task_struct *task)
>  
>  static void __init ordered_lsm_init(void)
>  {
> +	unsigned int first = 0;
>  	struct lsm_info **lsm;
> +	struct lsm_info *early;
>  
>  	if (chosen_lsm_order) {
>  		if (chosen_major_lsm) {
> @@ -354,7 +336,16 @@ static void __init ordered_lsm_init(void)
>  		lsm_prepare(*lsm);
>  	}
>  
> -	report_lsm_order();
> +	pr_info("initializing lsm=");
> +	lsm_early_for_each_raw(early) {
> +		if (is_enabled(early))
> +			pr_cont("%s%s", first++ == 0 ? "" : ",", early->name);
> +	}
> +	lsm_order_for_each(lsm) {
> +		if (is_enabled(*lsm))
> +			pr_cont("%s%s", first++ == 0 ? "" : ",", (*lsm)->name);
> +	}
> +	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);

^ permalink raw reply

* Re: [RFC PATCH v2 07/34] lsm: rename ordered_lsm_init() to lsm_init_ordered()
From: Casey Schaufler @ 2025-07-24 15:28 UTC (permalink / raw)
  To: Paul Moore, linux-security-module, linux-integrity, selinux
  Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
	Mickaël Salaün, Günther Noack, Kees Cook,
	Micah Morton, Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng,
	Casey Schaufler
In-Reply-To: <20250721232142.77224-43-paul@paul-moore.com>

On 7/21/2025 4:21 PM, Paul Moore wrote:
> The new name more closely fits the rest of the naming scheme in
> security/lsm_init.c.  This patch also adds a trivial comment block to
> the top of the function.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>


> ---
>  security/lsm_init.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7e736c20458a..c8af26a6ce14 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -288,7 +288,10 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
>  	kfree(sep);
>  }
>  
> -static void __init ordered_lsm_init(void)
> +/**
> + * lsm_init_ordered - Initialize the ordered LSMs
> + */
> +static void __init lsm_init_ordered(void)
>  {
>  	unsigned int first = 0;
>  	struct lsm_info **lsm;
> @@ -336,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,
> @@ -492,7 +492,7 @@ int __init security_init(void)
>  	}
>  
>  	/* Load LSMs in specified order. */
> -	ordered_lsm_init();
> +	lsm_init_ordered();
>  
>  	return 0;
>  }

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox