On 8/14/2025 3:50 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 <p...@paul-moore.com>

Reviewed-by: Casey Schaufler <ca...@schaufler-ca.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);
> +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 - 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. */

Reply via email to