On Thu, May 21, 2026 at 06:33:14AM -0700, Kees Cook wrote:
>
> param_array_get() appends each element's string representation into the
> shared sysfs page buffer by passing buffer + off to the element getter.
>
> That works for getters that only write a small bounded string, but
> param_get_charp() and similar helpers format against PAGE_SIZE from the
> pointer they receive. Once off is non-zero, an element getter can
> therefore write past the end of the original sysfs page buffer.
>
> Collect each element into a temporary PAGE_SIZE buffer first and then
> copy only the remaining space into the caller's page buffer.
...
> + elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
get_free_page() (or how it is called)?
> + if (!elem_buf)
> + return -ENOMEM;
> +
> for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> - /* Replace \n with comma */
> - if (i)
> - buffer[off - 1] = ',';
> p.arg = arr->elem + arr->elemsize * i;
> check_kparam_locked(p.mod);
> - ret = arr->ops->get(buffer + off, &p);
> + ret = arr->ops->get(elem_buf, &p);
> if (ret < 0)
> - return ret;
> + goto out;
> + ret = min(ret, (int)(PAGE_SIZE - 1 - off));
It's usually discouraged to use castings in min/max/clamp. Can we make ret long
or do something different here?
> + if (!ret)
> + break;
> + /* Replace the previous element's trailing newline with a
> comma. */
> + if (i)
> + buffer[off - 1] = ',';
Can't we do this after with help of strreplace()?
> + memcpy(buffer + off, elem_buf, ret);
> off += ret;
> + if (off == PAGE_SIZE - 1)
> + break;
> }
> buffer[off] = '\0';
> - return off;
> + ret = off;
> +out:
> + kfree(elem_buf);
> + return ret;
--
With Best Regards,
Andy Shevchenko
