Re: [PATCH v3 1/3] bpf: NUL-terminate replaced sysctl value

[bot+bpf-ci]

> When writing to sysctls, proc_sys_call_handler() guarantees that the
> buffer passed to proc handlers is NUL-terminated. If
> bpf_sysctl_set_new_value() replaces the pending sysctl value, it can
> hand a replacement buffer directly to proc handlers. However, the
> helper currently copies only buf_len bytes into that buffer without
> appending a NUL terminator, leaving downstream parsers vulnerable to
> out-of-bounds access.
> Fix this by appending a '\0' after the replaced value to restore the
> expected sysctl semantics. Since the helper already rejects buf_len
> greater than PAGE_SIZE - 1, there is always room for the extra byte.
> [ ... KASAN report snipped ... ]
> Signed-off-by: Zilin Guan <zilin@xxxxxxxxxx>
> Signed-off-by: Dawei Feng <dawei.feng@xxxxxxxxxx>

This looks like a fix for a bug introduced when the helper was first added.
The pattern this corrects, the memcpy(ctx->new_val, buf, buf_len) without a
trailing NUL together with the buf_len > PAGE_SIZE - 1 check the fix relies
on, was added by commit 4e63acdff864 which introduced
bpf_sysctl_set_new_value().

Should this carry:

  Fixes: 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value helpers")

The bpf CI review bot raised the same point during the v2 review:

  https://lore.kernel.org/all/12a9251e9f1a616fc86f4a2b6a87e547d75f820ce517895cdbf87bae81c970f1@xxxxxxxxxxxxxxx/

  "This fixes an out-of-bounds read in bpf_sysctl_set_new_value(). The
  missing NUL terminator after the memcpy() of the replacement value looks
  like it was introduced by the commit that added the helper. Should this
  carry: Fixes: 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value
  helpers")"

The v3 commit still has no Fixes: tag and that suggestion does not appear to
have been answered. Without it the fix is unlikely to be picked up for stable
backporting.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26881443787