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 <[email protected]> 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/[email protected]?part=6
