Re: [PATCH 08/11] params: Convert generic kernel_param_ops .get helpers to seq_buf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Information Leak / Uninitialized Memory Disclosure in param_array_get()
--

commit 4bb10449d07e3a1b177001d61d53053c25dd4767
Author: Kees Cook <kees@kernel.org>

params: Convert generic kernel_param_ops .get helpers to seq_buf

This commit converts the generic kernel parameter .get helpers in
kernel/params.c to use the seq_buf signature instead of char pointers. It
allows param_array_get to write element output directly into the parent
seq_buf when supported, updating various dependent callers to match the new
signature.

> diff --git a/kernel/params.c b/kernel/params.c
> index 4eda2d23ddf28..25f0c8d5d19f3 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -453,36 +457,46 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
[ ... ]
> -static int param_array_get(char *buffer, const struct kernel_param *kp)
> +static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
>  {
[ ... ]
> -	for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> +	for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> +		size_t before = s->len;
>  
>  		p.arg = arr->elem + arr->elemsize * i;
>  		check_kparam_locked(p.mod);
> -		ret = arr->ops->get_str(elem_buf, &p);
> -		if (ret < 0)
> -			goto out;
> -		ret = min(ret, (int)(PAGE_SIZE - 1 - off));
> -		if (!ret)
> +
> +		if (arr->ops->get) {
> +			ret = arr->ops->get(s, &p);
> +			if (ret < 0)
> +				goto out;
> +		} else {
[ ... ]
> +			ret = arr->ops->get_str(elem_buf, &p);
> +			if (ret < 0)
> +				goto out;
> +			seq_buf_putmem(s, elem_buf, ret);
> +		}
> +
> +		/* Nothing got written (e.g. overflow) — stop. */
> +		if (s->len == before)
>  			break;

[Severity: High]
When a seq_buf overflows, it calls seq_buf_set_overflow() which sets
s->len = s->size. Because of this, does s->len actually equal before
when an overflow occurs?

If s->len becomes s->size, the condition s->len == before will evaluate
to false, causing the loop to continue for one more iteration rather than
breaking. On the next iteration, before will be s->size, the write will
fail again, and the loop will finally break.

Because the overflow mechanism sets the length to the buffer size without
actually writing data into the remaining space, could this leak uninitialized
kernel heap memory to userspace when sysfs returns the buffer?

> +
> +		/* Replace the previous element's trailing newline with a comma. */
> -		if (i)
> -			buffer[off - 1] = ',';
> -		memcpy(buffer + off, elem_buf, ret);
> -		off += ret;
> -		if (off == PAGE_SIZE - 1)
> -			break;
> +		if (i && s->buffer[before - 1] == '\n')
> +			s->buffer[before - 1] = ',';
>  	}
> -	buffer[off] = '\0';
> -	ret = off;
> +	ret = 0;
>  out:
>  	kfree(elem_buf);
>  return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521133315.work.845-kees@kernel.org?part=8