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

[Tetsuo Handa]

On 2025/09/05 2:52, Paul Moore wrote:
> +       if (unlikely(!str)) {
> +               char *str_tmp;
> +               size_t len_tmp = 0;
> +

Wants a comment that lsm_active_cnt > 0 is guaranteed, or someone
(maybe static analyzers) thinks that we hit ZERO_SIZE_PTR pointer
dereference when lsm_active_cnt == 0.

> +               for (i = 0; i < lsm_active_cnt; i++)
> +                       /* the '+ 1' accounts for either a comma or a NUL */
> +                       len_tmp += strlen(lsm_idlist[i]->name) + 1;
> +
> +               str_tmp = kmalloc(len_tmp, GFP_KERNEL);
> +               if (!str_tmp)
> +                       return -ENOMEM;
> +               str_tmp[0] = '\0';
> +
> +               for (i = 0; i < lsm_active_cnt; i++) {
> +                       if (i > 0)
> +                               strcat(str_tmp, ",");
> +                       strcat(str_tmp, lsm_idlist[i]->name);
> +               }
> +
> +               spin_lock(&lock);
> +               if (!str) {
> +                       str = str_tmp;
> +                       len = len_tmp - 1;

This needs to be

                        len = len_tmp - 1;
                        mb();
                        str = str_tmp;

, or concurrent access might reach simple_read_from_buffer()
with str != 0 and len == 0. (If you don't want mb(), you can use

-       if (unlikely(!str)) {
+       if (unlikely(!str || !len)) {

instead).

> +               } else
> +                       kfree(str_tmp);
> +               spin_unlock(&lock);
> +       }
> +
> +       return simple_read_from_buffer(buf, count, ppos, str, len);
> }