Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare()
@ 2026-06-29 21:01 Matt Bobrowski
  2026-06-30 16:26 ` Casey Schaufler
  2026-07-02 19:15 ` Paul Moore
  0 siblings, 2 replies; 5+ messages in thread
From: Matt Bobrowski @ 2026-06-29 21:01 UTC (permalink / raw)
  To: linux-security-module
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Matt Bobrowski

Centralize the definition of LSM security blob fields using an X-macro
(LSM_BLOBS_LIST). This reduces repetitive boilerplate code across
struct lsm_blob_sizes, blob size registration in lsm_prepare(), and
debug log printing in security_init().

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 include/linux/lsm_hooks.h | 42 ++++++++++++++++------------
 security/lsm_init.c       | 59 ++++++++++-----------------------------
 2 files changed, 38 insertions(+), 63 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b4f8cad53ddb..0e73b22bdeea 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -98,28 +98,34 @@ struct security_hook_list {
 	const struct lsm_id *lsmid;
 } __randomize_layout;
 
+#define LSM_BLOBS_LIST(X) \
+	X(cred)          \
+	X(file)          \
+	X(backing_file)  \
+	X(ib)            \
+	X(inode)         \
+	X(sock)          \
+	X(superblock)    \
+	X(ipc)           \
+	X(key)           \
+	X(msg_msg)       \
+	X(perf_event)    \
+	X(task)          \
+	X(tun_dev)       \
+	X(xattr_count)   \
+	X(bdev)          \
+	X(bpf_map)       \
+	X(bpf_prog)      \
+	X(bpf_token)
+
 /*
  * Security blob size or offset data.
+ * Note: lbs_xattr_count is the number of xattr slots in new_xattrs array.
  */
 struct lsm_blob_sizes {
-	unsigned int lbs_cred;
-	unsigned int lbs_file;
-	unsigned int lbs_backing_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;
-	unsigned int lbs_bpf_map;
-	unsigned int lbs_bpf_prog;
-	unsigned int lbs_bpf_token;
+#define LSM_BLOB_SIZE(name) unsigned int lbs_##name;
+	LSM_BLOBS_LIST(LSM_BLOB_SIZE);
+#undef LSM_BLOB_SIZE
 };
 
 /*
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 7c0fd17f1601..c256f1c33efa 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -282,40 +282,24 @@ static void __init lsm_blob_size_update(unsigned int *sz_req,
  * lsm_prepare - Prepare the LSM framework for a new LSM
  * @lsm: LSM definition
  */
-static void __init lsm_prepare(struct lsm_info *lsm)
+static void __init lsm_prepare(const 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_backing_file,
-			     &blob_sizes.lbs_backing_file);
-	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
-	/* inode blob gets an rcu_head in addition to LSM blobs. */
+	/* The inode blob (inode->i_security) 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_blob_size_update(&blobs->lbs_bpf_map, &blob_sizes.lbs_bpf_map);
-	lsm_blob_size_update(&blobs->lbs_bpf_prog, &blob_sizes.lbs_bpf_prog);
-	lsm_blob_size_update(&blobs->lbs_bpf_token, &blob_sizes.lbs_bpf_token);
+
+	/* Register the LSM blob sizes. */
+#define UPDATE_LSM_BLOB_SIZE(name) \
+	lsm_blob_size_update(&blobs->lbs_##name, &blob_sizes.lbs_##name);
+	LSM_BLOBS_LIST(UPDATE_LSM_BLOB_SIZE);
+#undef UPDATE_LSM_BLOB_SIZE
 }
 
 /**
@@ -441,25 +425,10 @@ int __init security_init(void)
 		lsm_prepare(*lsm);
 
 	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(backing_file) size %d\n",
-		       blob_sizes.lbs_backing_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_pr("blob(bpf_map) size %d\n", blob_sizes.lbs_bpf_map);
-		lsm_pr("blob(bpf_prog) size %d\n", blob_sizes.lbs_bpf_prog);
-		lsm_pr("blob(bpf_token) size %d\n", blob_sizes.lbs_bpf_token);
+#define PRINT_LSM_BLOB_SIZE(name) \
+	lsm_pr("blob(" #name ") size %d\n", blob_sizes.lbs_##name);
+		LSM_BLOBS_LIST(PRINT_LSM_BLOB_SIZE);
+#undef PRINT_LSM_BLOB_SIZE
 	}
 
 	if (blob_sizes.lbs_file)
-- 
2.55.0.rc0.799.gd6f94ed593-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare()
  2026-06-29 21:01 [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare() Matt Bobrowski
@ 2026-06-30 16:26 ` Casey Schaufler
  2026-07-02 19:15 ` Paul Moore
  1 sibling, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2026-06-30 16:26 UTC (permalink / raw)
  To: Matt Bobrowski, linux-security-module
  Cc: Paul Moore, James Morris, Serge E . Hallyn, Casey Schaufler

On 6/29/2026 2:01 PM, Matt Bobrowski wrote:
> Centralize the definition of LSM security blob fields using an X-macro
> (LSM_BLOBS_LIST). This reduces repetitive boilerplate code across
> struct lsm_blob_sizes, blob size registration in lsm_prepare(), and
> debug log printing in security_init().
>
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
>  include/linux/lsm_hooks.h | 42 ++++++++++++++++------------
>  security/lsm_init.c       | 59 ++++++++++-----------------------------
>  2 files changed, 38 insertions(+), 63 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index b4f8cad53ddb..0e73b22bdeea 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -98,28 +98,34 @@ struct security_hook_list {
>  	const struct lsm_id *lsmid;
>  } __randomize_layout;
>  
> +#define LSM_BLOBS_LIST(X) \
> +	X(cred)          \
> +	X(file)          \
> +	X(backing_file)  \
> +	X(ib)            \
> +	X(inode)         \
> +	X(sock)          \
> +	X(superblock)    \
> +	X(ipc)           \
> +	X(key)           \
> +	X(msg_msg)       \
> +	X(perf_event)    \
> +	X(task)          \
> +	X(tun_dev)       \
> +	X(xattr_count)   \
> +	X(bdev)          \
> +	X(bpf_map)       \
> +	X(bpf_prog)      \
> +	X(bpf_token)
> +
>  /*
>   * Security blob size or offset data.
> + * Note: lbs_xattr_count is the number of xattr slots in new_xattrs array.
>   */
>  struct lsm_blob_sizes {
> -	unsigned int lbs_cred;
> -	unsigned int lbs_file;
> -	unsigned int lbs_backing_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;
> -	unsigned int lbs_bpf_map;
> -	unsigned int lbs_bpf_prog;
> -	unsigned int lbs_bpf_token;
> +#define LSM_BLOB_SIZE(name) unsigned int lbs_##name;
> +	LSM_BLOBS_LIST(LSM_BLOB_SIZE);
> +#undef LSM_BLOB_SIZE
>  };
>  
>  /*
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7c0fd17f1601..c256f1c33efa 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -282,40 +282,24 @@ static void __init lsm_blob_size_update(unsigned int *sz_req,
>   * lsm_prepare - Prepare the LSM framework for a new LSM
>   * @lsm: LSM definition
>   */
> -static void __init lsm_prepare(struct lsm_info *lsm)
> +static void __init lsm_prepare(const struct lsm_info *lsm)

This is an unrelated change which should be in a separate patch.

>  {
>  	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_backing_file,
> -			     &blob_sizes.lbs_backing_file);
> -	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> -	/* inode blob gets an rcu_head in addition to LSM blobs. */
> +	/* The inode blob (inode->i_security) gets an rcu_head in addition to
> +	 * LSM blobs.
> +	 */

Please retain the comment style of the file. Either
	/* ... */
	/*
	 * ...
	 */
but not
	/* ...
	 */

>  	if (blobs->lbs_inode && blob_sizes.lbs_inode == 0)
>  		blob_sizes.lbs_inode = sizeof(struct rcu_head);

This is where I dislike the macro approach. While this particular
exception is easily handled, others may not be. A blob whose size
isn't represented as an unsigned int (e.g a bool) would not use
lsm_blob_size_update(). While all of the existing blob sizes are
unsigned ints, I have considered bools on more than one occasion.

> -	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_blob_size_update(&blobs->lbs_bpf_map, &blob_sizes.lbs_bpf_map);
> -	lsm_blob_size_update(&blobs->lbs_bpf_prog, &blob_sizes.lbs_bpf_prog);
> -	lsm_blob_size_update(&blobs->lbs_bpf_token, &blob_sizes.lbs_bpf_token);
> +
> +	/* Register the LSM blob sizes. */
> +#define UPDATE_LSM_BLOB_SIZE(name) \
> +	lsm_blob_size_update(&blobs->lbs_##name, &blob_sizes.lbs_##name);
> +	LSM_BLOBS_LIST(UPDATE_LSM_BLOB_SIZE);
> +#undef UPDATE_LSM_BLOB_SIZE
>  }
>  
>  /**
> @@ -441,25 +425,10 @@ int __init security_init(void)
>  		lsm_prepare(*lsm);
>  
>  	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(backing_file) size %d\n",
> -		       blob_sizes.lbs_backing_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_pr("blob(bpf_map) size %d\n", blob_sizes.lbs_bpf_map);
> -		lsm_pr("blob(bpf_prog) size %d\n", blob_sizes.lbs_bpf_prog);
> -		lsm_pr("blob(bpf_token) size %d\n", blob_sizes.lbs_bpf_token);
> +#define PRINT_LSM_BLOB_SIZE(name) \
> +	lsm_pr("blob(" #name ") size %d\n", blob_sizes.lbs_##name);
> +		LSM_BLOBS_LIST(PRINT_LSM_BLOB_SIZE);
> +#undef PRINT_LSM_BLOB_SIZE
>  	}
>  
>  	if (blob_sizes.lbs_file)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in  lsm_prepare()
  2026-06-29 21:01 [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare() Matt Bobrowski
  2026-06-30 16:26 ` Casey Schaufler
@ 2026-07-02 19:15 ` Paul Moore
  2026-07-02 20:18   ` Matt Bobrowski
  1 sibling, 1 reply; 5+ messages in thread
From: Paul Moore @ 2026-07-02 19:15 UTC (permalink / raw)
  To: Matt Bobrowski, linux-security-module
  Cc: James Morris, Serge E . Hallyn, Matt Bobrowski

On Jun 29, 2026 Matt Bobrowski <mattbobrowski@google.com> wrote:
> 
> Centralize the definition of LSM security blob fields using an X-macro
> (LSM_BLOBS_LIST). This reduces repetitive boilerplate code across
> struct lsm_blob_sizes, blob size registration in lsm_prepare(), and
> debug log printing in security_init().
> 
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
>  include/linux/lsm_hooks.h | 42 ++++++++++++++++------------
>  security/lsm_init.c       | 59 ++++++++++-----------------------------
>  2 files changed, 38 insertions(+), 63 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index b4f8cad53ddb..0e73b22bdeea 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -98,28 +98,34 @@ struct security_hook_list {
>  	const struct lsm_id *lsmid;
>  } __randomize_layout;
>  
> +#define LSM_BLOBS_LIST(X) \
> +	X(cred)          \
> +	X(file)          \
> +	X(backing_file)  \
> +	X(ib)            \
> +	X(inode)         \
> +	X(sock)          \
> +	X(superblock)    \
> +	X(ipc)           \
> +	X(key)           \
> +	X(msg_msg)       \
> +	X(perf_event)    \
> +	X(task)          \
> +	X(tun_dev)       \
> +	X(xattr_count)   \
> +	X(bdev)          \
> +	X(bpf_map)       \
> +	X(bpf_prog)      \
> +	X(bpf_token)
> +
>  /*
>   * Security blob size or offset data.
> + * Note: lbs_xattr_count is the number of xattr slots in new_xattrs array.
>   */
>  struct lsm_blob_sizes {
> -	unsigned int lbs_cred;
> -	unsigned int lbs_file;
> -	unsigned int lbs_backing_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;
> -	unsigned int lbs_bpf_map;
> -	unsigned int lbs_bpf_prog;
> -	unsigned int lbs_bpf_token;
> +#define LSM_BLOB_SIZE(name) unsigned int lbs_##name;
> +	LSM_BLOBS_LIST(LSM_BLOB_SIZE);
> +#undef LSM_BLOB_SIZE
>  };

Generally speaking I greatly prefer to see the structure fields typed out
as it makes it easier to find them using grep, code indexers, etc.  There
is a similar argument for the security_init() changes (below).

> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7c0fd17f1601..c256f1c33efa 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -282,40 +282,24 @@ static void __init lsm_blob_size_update(unsigned int *sz_req,
>   * lsm_prepare - Prepare the LSM framework for a new LSM
>   * @lsm: LSM definition
>   */
> -static void __init lsm_prepare(struct lsm_info *lsm)
> +static void __init lsm_prepare(const 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_backing_file,
> -			     &blob_sizes.lbs_backing_file);
> -	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> -	/* inode blob gets an rcu_head in addition to LSM blobs. */
> +	/* The inode blob (inode->i_security) 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_blob_size_update(&blobs->lbs_bpf_map, &blob_sizes.lbs_bpf_map);
> -	lsm_blob_size_update(&blobs->lbs_bpf_prog, &blob_sizes.lbs_bpf_prog);
> -	lsm_blob_size_update(&blobs->lbs_bpf_token, &blob_sizes.lbs_bpf_token);
> +
> +	/* Register the LSM blob sizes. */
> +#define UPDATE_LSM_BLOB_SIZE(name) \
> +	lsm_blob_size_update(&blobs->lbs_##name, &blob_sizes.lbs_##name);
> +	LSM_BLOBS_LIST(UPDATE_LSM_BLOB_SIZE);
> +#undef UPDATE_LSM_BLOB_SIZE
>  }

This is one case where the macro approach *might* have some value, but as
it is the one instance, and multiple sequential blob operations like this
are going to be *very* limited, I'm not sure there is enough value to
be worth the change.

I appreciate the initiative to search out areas that you think could use
improvement, but for the reasons mentioned, I'm not going to merge this
patch.

> @@ -441,25 +425,10 @@ int __init security_init(void)
>  		lsm_prepare(*lsm);
>  
>  	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(backing_file) size %d\n",
> -		       blob_sizes.lbs_backing_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_pr("blob(bpf_map) size %d\n", blob_sizes.lbs_bpf_map);
> -		lsm_pr("blob(bpf_prog) size %d\n", blob_sizes.lbs_bpf_prog);
> -		lsm_pr("blob(bpf_token) size %d\n", blob_sizes.lbs_bpf_token);
> +#define PRINT_LSM_BLOB_SIZE(name) \
> +	lsm_pr("blob(" #name ") size %d\n", blob_sizes.lbs_##name);
> +		LSM_BLOBS_LIST(PRINT_LSM_BLOB_SIZE);
> +#undef PRINT_LSM_BLOB_SIZE

This is another case where having the text string grep'able can be a good
thing.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare()
  2026-07-02 19:15 ` Paul Moore
@ 2026-07-02 20:18   ` Matt Bobrowski
  2026-07-02 21:15     ` Paul Moore
  0 siblings, 1 reply; 5+ messages in thread
From: Matt Bobrowski @ 2026-07-02 20:18 UTC (permalink / raw)
  To: Paul Moore; +Cc: linux-security-module, James Morris, Serge E . Hallyn

On Thu, Jul 02, 2026 at 03:15:02PM -0400, Paul Moore wrote:
> On Jun 29, 2026 Matt Bobrowski <mattbobrowski@google.com> wrote:
> > 
> > Centralize the definition of LSM security blob fields using an X-macro
> > (LSM_BLOBS_LIST). This reduces repetitive boilerplate code across
> > struct lsm_blob_sizes, blob size registration in lsm_prepare(), and
> > debug log printing in security_init().
> > 
> > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> > ---
> >  include/linux/lsm_hooks.h | 42 ++++++++++++++++------------
> >  security/lsm_init.c       | 59 ++++++++++-----------------------------
> >  2 files changed, 38 insertions(+), 63 deletions(-)
> > 
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index b4f8cad53ddb..0e73b22bdeea 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -98,28 +98,34 @@ struct security_hook_list {
> >  	const struct lsm_id *lsmid;
> >  } __randomize_layout;
> >  
> > +#define LSM_BLOBS_LIST(X) \
> > +	X(cred)          \
> > +	X(file)          \
> > +	X(backing_file)  \
> > +	X(ib)            \
> > +	X(inode)         \
> > +	X(sock)          \
> > +	X(superblock)    \
> > +	X(ipc)           \
> > +	X(key)           \
> > +	X(msg_msg)       \
> > +	X(perf_event)    \
> > +	X(task)          \
> > +	X(tun_dev)       \
> > +	X(xattr_count)   \
> > +	X(bdev)          \
> > +	X(bpf_map)       \
> > +	X(bpf_prog)      \
> > +	X(bpf_token)
> > +
> >  /*
> >   * Security blob size or offset data.
> > + * Note: lbs_xattr_count is the number of xattr slots in new_xattrs array.
> >   */
> >  struct lsm_blob_sizes {
> > -	unsigned int lbs_cred;
> > -	unsigned int lbs_file;
> > -	unsigned int lbs_backing_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;
> > -	unsigned int lbs_bpf_map;
> > -	unsigned int lbs_bpf_prog;
> > -	unsigned int lbs_bpf_token;
> > +#define LSM_BLOB_SIZE(name) unsigned int lbs_##name;
> > +	LSM_BLOBS_LIST(LSM_BLOB_SIZE);
> > +#undef LSM_BLOB_SIZE
> >  };
> 
> Generally speaking I greatly prefer to see the structure fields typed out
> as it makes it easier to find them using grep, code indexers, etc.  There
> is a similar argument for the security_init() changes (below).
> 
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 7c0fd17f1601..c256f1c33efa 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -282,40 +282,24 @@ static void __init lsm_blob_size_update(unsigned int *sz_req,
> >   * lsm_prepare - Prepare the LSM framework for a new LSM
> >   * @lsm: LSM definition
> >   */
> > -static void __init lsm_prepare(struct lsm_info *lsm)
> > +static void __init lsm_prepare(const 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_backing_file,
> > -			     &blob_sizes.lbs_backing_file);
> > -	lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> > -	/* inode blob gets an rcu_head in addition to LSM blobs. */
> > +	/* The inode blob (inode->i_security) 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_blob_size_update(&blobs->lbs_bpf_map, &blob_sizes.lbs_bpf_map);
> > -	lsm_blob_size_update(&blobs->lbs_bpf_prog, &blob_sizes.lbs_bpf_prog);
> > -	lsm_blob_size_update(&blobs->lbs_bpf_token, &blob_sizes.lbs_bpf_token);
> > +
> > +	/* Register the LSM blob sizes. */
> > +#define UPDATE_LSM_BLOB_SIZE(name) \
> > +	lsm_blob_size_update(&blobs->lbs_##name, &blob_sizes.lbs_##name);
> > +	LSM_BLOBS_LIST(UPDATE_LSM_BLOB_SIZE);
> > +#undef UPDATE_LSM_BLOB_SIZE
> >  }
> 
> This is one case where the macro approach *might* have some value, but as
> it is the one instance, and multiple sequential blob operations like this
> are going to be *very* limited, I'm not sure there is enough value to
> be worth the change.
> 
> I appreciate the initiative to search out areas that you think could use
> improvement, but for the reasons mentioned, I'm not going to merge this
> patch.

That's not a problem. Perhaps you may want to consider only taking the
minor cleanup targets the redundant lsm->blobs access within
lsm_prepare().

> > @@ -441,25 +425,10 @@ int __init security_init(void)
> >  		lsm_prepare(*lsm);
> >  
> >  	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(backing_file) size %d\n",
> > -		       blob_sizes.lbs_backing_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_pr("blob(bpf_map) size %d\n", blob_sizes.lbs_bpf_map);
> > -		lsm_pr("blob(bpf_prog) size %d\n", blob_sizes.lbs_bpf_prog);
> > -		lsm_pr("blob(bpf_token) size %d\n", blob_sizes.lbs_bpf_token);
> > +#define PRINT_LSM_BLOB_SIZE(name) \
> > +	lsm_pr("blob(" #name ") size %d\n", blob_sizes.lbs_##name);
> > +		LSM_BLOBS_LIST(PRINT_LSM_BLOB_SIZE);
> > +#undef PRINT_LSM_BLOB_SIZE
> 
> This is another case where having the text string grep'able can be a good
> thing.
> 
> --
> paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare()
  2026-07-02 20:18   ` Matt Bobrowski
@ 2026-07-02 21:15     ` Paul Moore
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2026-07-02 21:15 UTC (permalink / raw)
  To: Matt Bobrowski; +Cc: linux-security-module, James Morris, Serge E . Hallyn

On Thu, Jul 2, 2026 at 4:18 PM Matt Bobrowski <mattbobrowski@google.com> wrote:
> On Thu, Jul 02, 2026 at 03:15:02PM -0400, Paul Moore wrote:
> > On Jun 29, 2026 Matt Bobrowski <mattbobrowski@google.com> wrote:
> > >
> > > Centralize the definition of LSM security blob fields using an X-macro
> > > (LSM_BLOBS_LIST). This reduces repetitive boilerplate code across
> > > struct lsm_blob_sizes, blob size registration in lsm_prepare(), and
> > > debug log printing in security_init().
> > >
> > > Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> > > ---
> > >  include/linux/lsm_hooks.h | 42 ++++++++++++++++------------
> > >  security/lsm_init.c       | 59 ++++++++++-----------------------------
> > >  2 files changed, 38 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > > index b4f8cad53ddb..0e73b22bdeea 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -98,28 +98,34 @@ struct security_hook_list {
> > >     const struct lsm_id *lsmid;
> > >  } __randomize_layout;
> > >
> > > +#define LSM_BLOBS_LIST(X) \
> > > +   X(cred)          \
> > > +   X(file)          \
> > > +   X(backing_file)  \
> > > +   X(ib)            \
> > > +   X(inode)         \
> > > +   X(sock)          \
> > > +   X(superblock)    \
> > > +   X(ipc)           \
> > > +   X(key)           \
> > > +   X(msg_msg)       \
> > > +   X(perf_event)    \
> > > +   X(task)          \
> > > +   X(tun_dev)       \
> > > +   X(xattr_count)   \
> > > +   X(bdev)          \
> > > +   X(bpf_map)       \
> > > +   X(bpf_prog)      \
> > > +   X(bpf_token)
> > > +
> > >  /*
> > >   * Security blob size or offset data.
> > > + * Note: lbs_xattr_count is the number of xattr slots in new_xattrs array.
> > >   */
> > >  struct lsm_blob_sizes {
> > > -   unsigned int lbs_cred;
> > > -   unsigned int lbs_file;
> > > -   unsigned int lbs_backing_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;
> > > -   unsigned int lbs_bpf_map;
> > > -   unsigned int lbs_bpf_prog;
> > > -   unsigned int lbs_bpf_token;
> > > +#define LSM_BLOB_SIZE(name) unsigned int lbs_##name;
> > > +   LSM_BLOBS_LIST(LSM_BLOB_SIZE);
> > > +#undef LSM_BLOB_SIZE
> > >  };
> >
> > Generally speaking I greatly prefer to see the structure fields typed out
> > as it makes it easier to find them using grep, code indexers, etc.  There
> > is a similar argument for the security_init() changes (below).
> >
> > > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > > index 7c0fd17f1601..c256f1c33efa 100644
> > > --- a/security/lsm_init.c
> > > +++ b/security/lsm_init.c
> > > @@ -282,40 +282,24 @@ static void __init lsm_blob_size_update(unsigned int *sz_req,
> > >   * lsm_prepare - Prepare the LSM framework for a new LSM
> > >   * @lsm: LSM definition
> > >   */
> > > -static void __init lsm_prepare(struct lsm_info *lsm)
> > > +static void __init lsm_prepare(const 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_backing_file,
> > > -                        &blob_sizes.lbs_backing_file);
> > > -   lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
> > > -   /* inode blob gets an rcu_head in addition to LSM blobs. */
> > > +   /* The inode blob (inode->i_security) 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_blob_size_update(&blobs->lbs_bpf_map, &blob_sizes.lbs_bpf_map);
> > > -   lsm_blob_size_update(&blobs->lbs_bpf_prog, &blob_sizes.lbs_bpf_prog);
> > > -   lsm_blob_size_update(&blobs->lbs_bpf_token, &blob_sizes.lbs_bpf_token);
> > > +
> > > +   /* Register the LSM blob sizes. */
> > > +#define UPDATE_LSM_BLOB_SIZE(name) \
> > > +   lsm_blob_size_update(&blobs->lbs_##name, &blob_sizes.lbs_##name);
> > > +   LSM_BLOBS_LIST(UPDATE_LSM_BLOB_SIZE);
> > > +#undef UPDATE_LSM_BLOB_SIZE
> > >  }
> >
> > This is one case where the macro approach *might* have some value, but as
> > it is the one instance, and multiple sequential blob operations like this
> > are going to be *very* limited, I'm not sure there is enough value to
> > be worth the change.
> >
> > I appreciate the initiative to search out areas that you think could use
> > improvement, but for the reasons mentioned, I'm not going to merge this
> > patch.
>
> That's not a problem. Perhaps you may want to consider only taking the
> minor cleanup targets the redundant lsm->blobs access within
> lsm_prepare().

I did think about that, but I ever *so* slightly prefer it the way it
is now, although as I said this is the one case where there may be
some value.  If there had been other areas where it was a clear cut
win I likely would have just accepted this change too, but there
wasn't (at least not now, and with this revision) so I just decided to
leave it well enough alone.

However, as I said, thank you taking the initiative on this.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-02 21:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 21:01 [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare() Matt Bobrowski
2026-06-30 16:26 ` Casey Schaufler
2026-07-02 19:15 ` Paul Moore
2026-07-02 20:18   ` Matt Bobrowski
2026-07-02 21:15     ` Paul Moore

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