On Tue, Sep 2, 2025 at 1:20 PM John Johansen
<john.johan...@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 <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
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?
--
paul-moore.com
