Re: [PATCH 06/11] moduleparam: Add seq_buf-based .get callback alongside .get_str

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent data truncation and spurious kernel warning when the sysfs buffer is exactly filled.
--

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

moduleparam: Add seq_buf-based .get callback alongside .get_str

This commit adds a new seq_buf-based .get callback to struct kernel_param_ops 
to format parameter values safely. It updates param_attr_show() to prefer 
this new callback when available, using a page-sized buffer. The legacy 
.get_str callback is retained as a fallback.

> diff --git a/kernel/params.c b/kernel/params.c
> index 6852caea17854..4eda2d23ddf28 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -553,12 +553,34 @@ static ssize_t param_attr_show(const struct module_attribute *mattr,
[ ... ]
>  	kernel_param_lock(mk->mod);
> -	count = attribute->param->ops->get_str(buf, attribute->param);
> +	if (ops->get) {
> +		struct seq_buf s;
> +
> +		seq_buf_init(&s, buf, PAGE_SIZE);
> +		count = ops->get(&s, attribute->param);
> +		if (count >= 0) {
> +			WARN_ON_ONCE(count > 0);
> +			count = seq_buf_used(&s);
> +			/* Make sure string is terminated. */
> +			seq_buf_str(&s);
> +			/*
> +			 * If overflowed, reduce count by 1 for trailing
> +			 * NUL byte.
> +			 */
> +			if (seq_buf_has_overflowed(&s))
> +				count--;

[Severity: Medium]
Does this code silently truncate data and trigger a kernel warning when the
buffer is exactly filled?

If the .get() callback writes exactly PAGE_SIZE bytes without triggering an
overflow, seq_buf_has_overflowed(&s) will evaluate to false because s.len
exactly equals s.size. In this case, count remains PAGE_SIZE.

However, seq_buf_str(&s) unconditionally writes a NUL byte at the end of the
buffer, which will replace the last byte of valid data at PAGE_SIZE - 1.

Furthermore, returning PAGE_SIZE causes the sysfs core to print a bad count
warning to dmesg and truncate the count anyway. Should this code explicitly
decrement count when count == PAGE_SIZE to avoid overwriting data and causing
the warning?

> +		}
> +	} else {
> +		count = ops->get_str(buf, attribute->param);
> +	}
>  	kernel_param_unlock(mk->mod);
>  	return count;
>  }

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