bpf: cgroup: fix sysctl new-value handling in __cgroup_bpf_run_filter_sysctl

[kernel-patches-daemon-bpf]

Pull request for series with
subject: bpf: cgroup: fix sysctl new-value handling in __cgroup_bpf_run_filter_sysctl
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1102583

[kernel-patches-daemon-bpf]

Upstream branch: 9a720e0
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1102583
version: 2

[kernel-patches-review-bot]

commit e0dceb780c938513d0c789ea7a3a3f13393d645d
Author: Dawei Feng <dawei.feng@seu.edu.cn>

bpf: cgroup: NUL-terminate replaced sysctl value

This appends a NUL terminator after the replacement sysctl value written
by bpf_sysctl_set_new_value(), so downstream proc handlers do not read
past the end of the buffer.  A KASAN slab-out-of-bounds read was
reproduced via the cgroup/sysctl replacement path before the fix.

> 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.
>
> [ ... KASAN report snipped ... ]
>
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>

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")

---

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

In-Reply-To-Subject: bpf: cgroup: NUL-terminate replaced sysctl value
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26616337824

[kernel-patches-review-bot]

commit dcbe395f223e5524109eb40fa57257eec6d887d5
Author: Dawei Feng <dawei.feng@seu.edu.cn>

bpf: cgroup: restore sysctl new-value replacement

This changes the success check in __cgroup_bpf_run_filter_sysctl() from
ret == 1 to !ret so that a cgroup/sysctl program can replace the pending
sysctl write buffer again, since bpf_prog_run_array_cg() returns 0 on
success.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index a0b5f8cd8b10..3f06e2270f5c 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -1935,7 +1935,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
>
>  	kfree(ctx.cur_val);
>
> -	if (ret == 1 && ctx.new_updated) {
> +	if (!ret && ctx.new_updated) {
>  		kvfree(*buf);
>  		*buf = ctx.new_val;
>  		*pcount = ctx.new_len;

With the success check now testing !ret, does the kerneldoc above
__cgroup_bpf_run_filter_sysctl() still describe the right convention?  It
currently reads:

 * This function will return %-EPERM if an attached program is found and
 * returned value != 1 during execution. In all other cases 0 is returned.

Since bpf_prog_run_array_cg() returns 0 on success rather than 1, should
that line say "returned value != 0" to match the new check?

This point was raised in v1 review but does not appear to have been
changed in v2.

Now for the Fixes: tag.  The commit message states:

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

Is this the commit that actually introduced the problem?  At 4e63acdff864
the runner was BPF_PROG_RUN_ARRAY, which returned 1 on success, so the
ret == 1 check was correct at that time.

There appear to be two candidate commits that changed the return
convention to 0-on-success and left the ret == 1 check stale:

  c4dcfdd406aa ("bpf: Move getsockopt retval to struct bpf_cg_run_ctx")
  f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err instead of allow boolean")

Should the Fixes: tag point at one of these instead of 4e63acdff864?

The f10d05966196 suggestion was raised by the bpf-ci review in v1 but the
tag was unchanged in v2.

---

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

In-Reply-To-Subject: bpf: cgroup: restore sysctl new-value replacement
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26616337824

[kernel-patches-daemon-bpf]

Forwarding comment 4570323229 via email
In-Reply-To: 20260529031026.2716641-3-dawei.feng@seu.edu.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260529031026.2716641-3-dawei.feng@seu.edu.cn/

[kernel-patches-daemon-bpf]

Forwarding comment 4570343110 via email
In-Reply-To: 20260529031026.2716641-4-dawei.feng@seu.edu.cn
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/20260529031026.2716641-4-dawei.feng@seu.edu.cn/

[kernel-patches-daemon-bpf]

Upstream branch: 9a720e0
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1102583
version: 2