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

[Paul Moore]

On Thu, Jul 24, 2025 at 11:39 AM Casey Schaufler <casey at schaufler-ca.com> wrote:
> On 7/21/2025 4:21 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 <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(-)

...

> > +/* 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;
>
> You're going to get a nul byte at the end of the string because
> you accounted for the ',' above, but there isn't one at the end
> of the string.

I'm not sure I understand your concern here, can you phrase it differently?

If you're worried about lsm_read_str potentially not being terminated
with a NUL byte, the strcat() should copy over the trailing NUL.

> > +     spin_unlock(&lsm_read_lock);
> > +
> > +     goto restart;
> >  }

-- 
paul-moore.com