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

[Alexei Starovoitov]

On Wed, Jun 3, 2026 at 7:37 AM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
>
>
> On 6/3/26 3:53 AM, Dawei Feng wrote:
> > 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.
> >
> > Reproduced in a QEMU x86_64 guest booted with KASAN while exercising
> > the sysctl replacement path with a cgroup/sysctl BPF program. The
> > reproducer targets `/proc/sys/net/core/flow_limit_cpu_bitmap`, fills
> > the original user write buffer with non-zero bytes, and overrides the
> > sysctl value so the replacement buffer lacks a terminating NUL. Under
> > that setup, the pre-fix kernel reported:
> >
> >    BUG: KASAN: slab-out-of-bounds in strnchrnul+0x72/0x90
> >    Read of size 1 at addr ffff88800de57000 by task repro_patch3/66
> >    CPU: 0 UID: 0 PID: 66 Comm: repro_patch3 Not tainted 7.1.0-rc3-00269-g8370ca1f87cc #6 PREEMPT(lazy)
> >    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> >    Call Trace:
> >     <TASK>
> >     dump_stack_lvl+0x68/0xa0
> >     print_report+0xcb/0x5e0
> >     ? __virt_addr_valid+0x21d/0x3f0
> >     ? strnchrnul+0x72/0x90
> >     ? strnchrnul+0x72/0x90
> >     kasan_report+0xca/0x100
> >     ? strnchrnul+0x72/0x90
> >     strnchrnul+0x72/0x90
> >     bitmap_parse+0x37/0x2e0
> >     flow_limit_cpu_sysctl+0xc6/0x840
> >     ? __pfx_flow_limit_cpu_sysctl+0x10/0x10
> >     ? __kvmalloc_node_noprof+0x5ba/0x870
> >     proc_sys_call_handler+0x31d/0x480
> >     ? __pfx_proc_sys_call_handler+0x10/0x10
> >     ? selinux_file_permission+0x39f/0x500
> >     ? lock_is_held_type+0x9e/0x120
> >     vfs_write+0x98e/0x1000
> >     ...
> >     </TASK>
> >    The buggy address is located 0 bytes to the right of
> >    allocated 4096-byte region [ffff88800de56000, ffff88800de57000)
> > With this fix applied, rerunning the same sysctl-targeted path yields
> > no corresponding KASAN reports.
> >
> > Signed-off-by: Zilin Guan <zilin@xxxxxxxxxx>
> > Signed-off-by: Dawei Feng <dawei.feng@xxxxxxxxxx>
> > ---
> >   kernel/bpf/cgroup.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> > index 876f6a81a9b6..2c7f72d3fb11 100644
> > --- a/kernel/bpf/cgroup.c
> > +++ b/kernel/bpf/cgroup.c
> > @@ -2342,6 +2342,7 @@ BPF_CALL_3(bpf_sysctl_set_new_value, struct bpf_sysctl_kern *, ctx,
> >               return -E2BIG;
> >
> >       memcpy(ctx->new_val, buf, buf_len);
> > +     ((char *)ctx->new_val)[buf_len] = '\0';
>
> In v2 (https://lore.kernel.org/bpf/bf25d653-d856-4ad7-a751-b97d38f38892@xxxxxxxxx/)
> I suggested
>      memcpy(ctx->new_val, buf, buf_len + 1);
> Does it work?

may be it should be strscpy()?
The input is a string, right?