Re: [PATCH 11/11] moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch logic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional call to `arr->ops->get()` without a NULL check can cause a kernel panic if the array parameter element lacks a `.get` callback.
--

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

moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch logic

All struct kernel_param_ops .get callbacks have been migrated to using
struct seq_buf. Drop the migration scaffolding.

> diff --git a/kernel/params.c b/kernel/params.c
> index 25f0c8d5d19f3..6b410189297b1 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -470,23 +469,9 @@ static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
>  		p.arg = arr->elem + arr->elemsize * i;
>  		check_kparam_locked(p.mod);
>  
> -		if (arr->ops->get) {
> -			ret = arr->ops->get(s, &p);
> -			if (ret < 0)
> -				goto out;
> -		} else {
[ ... ]
> -		}
> +		ret = arr->ops->get(s, &p);

[Severity: High]
This isn't a bug introduced by this patch, but does this unconditional call to
arr->ops->get() need a NULL check to avoid a kernel panic?

If a module defines a write-only array parameter where the element's
kernel_param_ops struct lacks a .get callback, and assigns it write-only sysfs
permissions, root can still attempt to read the file. When this happens, sysfs
invokes param_attr_show().

For normal parameters, param_attr_show() has a safety check that rejects the
read if ops->get is NULL. However, for array parameters, ops resolves to
param_array_ops which has a valid .get callback, bypassing the safeguard.

Could this result in a NULL pointer dereference when arr->ops->get(s, &p) is
executed? Before this patch, the code fell back to calling .get_str(), which
would also be NULL for a write-only type, resulting in the same kernel panic.

> +		if (ret < 0)
> +			return ret;
>  
>  		/* Nothing got written (e.g. overflow) — stop. */
>  		if (s->len == before)

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