Thread (5 messages) 5 messages, 3 authors, 10d ago

Re: [PATCH] lsm: cleanup repeated lsm_blob_size_update() calls in lsm_prepare()

From: Paul Moore <paul@paul-moore.com>
Date: 2026-07-02 21:16:00

On Thu, Jul 2, 2026 at 4:18 PM Matt Bobrowski [off-list ref] wrote:
On Thu, Jul 02, 2026 at 03:15:02PM -0400, Paul Moore wrote:
quoted
On Jun 29, 2026 Matt Bobrowski [off-list ref] wrote:
quoted
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).
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help