On Wed, Apr 9, 2025 at 7:13 PM Kees Cook <k...@kernel.org> wrote:
>
> On Wed, Apr 09, 2025 at 02:49:53PM -0400, 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 an as-needed basis when
> > userspace reads "/sys/kernel/security/lsm".
> >
> > Signed-off-by: Paul Moore <p...@paul-moore.com>
> > ---
> > include/linux/lsm_hooks.h | 1 -
> > security/inode.c | 27 +++++++++++++++++++--
> > security/lsm_init.c | 49 ---------------------------------------
> > 3 files changed, 25 insertions(+), 52 deletions(-)
...
> > @@ -343,8 +345,29 @@ static struct dentry *lsm_dentry;
> > 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 rc, len = 0;
> > +
> > + for (i = 0; i < lsm_count; i++)
> > + /* the '+ 1' accounts for either a comma or a NUL terminator
> > */
> > + len += strlen(lsm_order[i]->id->name) + 1;
> > +
> > + str = kmalloc(len, GFP_KERNEL);
> > + if (!str)
> > + return -ENOMEM;
> > + str[0] = '\0';
> > +
> > + i = 0;
> > + while (i < lsm_count) {
> > + strcat(str, lsm_order[i]->id->name);
> > + if (++i < lsm_count)
> > + strcat(str, ",");
> > + }
> > +
> > + rc = simple_read_from_buffer(buf, count, ppos, str, len);
> > + kfree(str);
> > + return rc;
>
> Hrm, at least cache it?
Are you aware of a performance critical use of this?
> Better yet, do this whole thing in a initcall after LSMs are loaded, and
> both can gain __ro_after_init...
I *really* disliked all the stuff we were having to do during boot,
and all the redundant global state we were keeping around. I'll go
ahead and cache the lsm_read() result local to the function but that's
probably all I'm going to accept at this point in time.
--
paul-moore.com
