On 7/24/2025 7:28 PM, Paul Moore wrote:
> On Thu, Jul 24, 2025 at 11:39 AM Casey Schaufler <ca...@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 <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(-)
> ..
>
>>> +/* 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?
"lockdown,capability,...,evm\0" You get the '\0' because you always expect
a trailing ','. On the last element there is no ',' but the length is added
as if there is.
+ lsm_read_len = len - 1;
will fix the problem.
>
> 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;
>>> }
