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

[Paul Moore]

On Thu, Sep 4, 2025 at 4:48 AM John Johansen
<john.johansen at canonical.com> wrote:
> On 9/4/25 01:12, Roberto Sassu wrote:
> > 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 <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
> >>
> >>> +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.
>
> doh, yes.

D'oh indeed!  Thanks for catching this.

> But we shouldn't need RCU here either. This is a write once, never update
> again situation. Instead we can get away with just a lock on the write
> side to ensure exclusion on setting the value.
>
> We don't even need the read side memory barrier, if the assumption that
> the pointer is read as a single value holds, and the the null read will
> go to the lock, and end up rereading.

It's funny you bring this up, my first draft of this function
(unposted) did just that, although I figured I'd add the RCU read side
protection ... "just in case".

I'll rework this function, but I'll hold off on posting another
revision until I hear back on some of the reviews that are still
pending in case additional edits are needed.

-- 
paul-moore.com