* [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