[PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup

[John Johansen]

On 9/3/25 13:26, Paul Moore wrote:
> On Tue, Sep 2, 2025 at 1:20 PM John Johansen
> <john.johansen at canonical.com> 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 <paul at 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
> 
> Fair point, I'll rename it to lsm_read_str_lock, it still has "read"
> in the name, but it should be a bit more clear that it references the
> lsm_read_str variable.
> 
>>> +static char *lsm_read_str = NULL;
>>> +static ssize_t lsm_read_len = 0;
> 
> Similarly, I'm renaming lsm_read_len to lsm_read_str_len.
> 
>>>    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) {
>>
>> should probably be
>> if (!rcu_access_pointer(lsm_read_str)) {
> 
> The description for rcu_access_pointer() contains the following sentence:
> 
>    "Within an RCU read-side critical section, there is little reason to
>     use rcu_access_pointer()."
>    https://elixir.bootlin.com/linux/v6.17-rc4/source/include/linux/rcupdate.h#L628
> 
> Perhaps I'm reading it wrong, but it looks like the RCU folks would
> prefer we not use rcu_access_pointer() here?
> 
no, I just forgot that detail