On Tue, 2025-09-02 at 10:20 -0700, John Johansen wrote:
> On 8/14/25 15:50, 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 <p...@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 7343dd60b1d5..65a8227bece7 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -172,7 +172,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 43382ef8896e..a5e7a073e672 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;
> >
> > @@ -315,12 +317,65 @@ void securityfs_remove(struct dentry *dentry)
> > EXPORT_SYMBOL_GPL(securityfs_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);
>
> nit, this is only used on the write side, so not the best name
>
> > +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();
Uhm, it seems we cannot use plain RCU here, simple_read_from_buffer()
can sleep.
Roberto
> > + if (!lsm_read_str) {
> should probably be
> if (!rcu_access_pointer(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 - 1;
> > + 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 9e495a36a332..87e2147016b3 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[];
> > @@ -371,42 +369,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;
> > @@ -443,15 +405,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)
> > @@ -488,8 +441,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. */
>
