[PATCH v3 0/3] bpf: fix sysctl new-value handling in __cgroup_bpf_run_filter_sysctl

[PATCH v3 0/3] bpf: fix sysctl new-value handling in __cgroup_bpf_run_filter_sysctl

[Dawei Feng]

This series fixes three bugs in the sysctl write-buffer replacement path
of __cgroup_bpf_run_filter_sysctl(). It resolves a kvzalloc()/kfree()
mismatch, adds a missing NUL terminator to the replacement string, and
updates a stale return value check to safely restore the replacement
functionality.

Patch Summary:
- patch 1 NUL-terminates the replaced sysctl value
- patch 2 uses kvfree() for the replaced sysctl write buffer
- patch 3 restores sysctl new-value replacement

Changelog:
v2 -> v3:
- reordered patches 1 and 2
- added the missing Reviewed-by/Acked-by tags to patches 2 and 3
- fixed the incorrect Fixes tag in patch 3
- simplified the dynamic test logs in patch 1 and 2, and updated
  titles

Dawei Feng (3):
  bpf: NUL-terminate replaced sysctl value
  bpf: use kvfree() for replaced sysctl write buffer
  bpf: Restore sysctl new-value from 1 to 0

 kernel/bpf/cgroup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.34.1

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

[Dawei Feng]

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@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 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';
 	ctx->new_len = buf_len;
 	ctx->new_updated = 1;
 
-- 
2.34.1

[PATCH v3 2/3] bpf: use kvfree() for replaced sysctl write buffer

[Dawei Feng]

proc_sys_call_handler() allocates its temporary sysctl buffer with
kvzalloc() and passes it to __cgroup_bpf_run_filter_sysctl(). Since
kvzalloc() may fall back to vmalloc() for large allocations, freeing
that buffer with kfree() is wrong and can corrupt memory.

Use kvfree() to safely handle both kmalloc and kvzalloc()/vmalloc
allocations.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still
present in v7.1-rc5.

Reproduced the bug based on v7.1-rc4 in a QEMU x86_64 guest booted with
KASAN and CONFIG_FAILSLAB enabled. To exercise the replacement path, the
test tree also included the accompanying fix for the stale ret == 1
check in __cgroup_bpf_run_filter_sysctl(). The reproducer confines
failslab injections to the proc_sys_call_handler() range, uses
stacktrace-depth=32, and injects fail-nth=1 while writing 8191 bytes to
/proc/sys/kernel/domainname from a task in the target cgroup. Under
that setup, fail-nth=1 triggered the fault:

  BUG: unable to handle page fault for address: ffffeb0200024d48
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 0 P4D 0
  Oops: Oops: 0000  SMP KASAN NOPTI
  CPU: 2 UID: 0 PID: 209 Comm: repro_proc_sys_ Not tainted 7.1.0-rc4-00686-g97625979a5d4  PREEMPT(lazy)
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
  RIP: 0010:kfree+0x6e/0x510
  ...
  Call Trace:
   <TASK>
   ? __cgroup_bpf_run_filter_sysctl+0x626/0xc30
   __cgroup_bpf_run_filter_sysctl+0x74d/0xc30
   ? __pfx___cgroup_bpf_run_filter_sysctl+0x10/0x10
   ? srso_return_thunk+0x5/0x5f
   ? __kvmalloc_node_noprof+0x345/0x870
   ? proc_sys_call_handler+0x250/0x480
   ? srso_return_thunk+0x5/0x5f
   proc_sys_call_handler+0x3a2/0x480
   ? __pfx_proc_sys_call_handler+0x10/0x10
   ? srso_return_thunk+0x5/0x5f
   ? selinux_file_permission+0x39f/0x500
   ? srso_return_thunk+0x5/0x5f
   ? lock_is_held_type+0x9e/0x120
   vfs_write+0x98e/0x1000
   ...
   </TASK>

With this fix applied on top of the same test setup, rerunning the
reproducer with fail-nth=1 yields no corresponding Oops reports.

Fixes: 4508943794ef ("proc: use kvzalloc for our kernel buffer")
Cc: stable@vger.kernel.org

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 kernel/bpf/cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 2c7f72d3fb11..a0b5f8cd8b10 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1936,7 +1936,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
 	kfree(ctx.cur_val);
 
 	if (ret == 1 && ctx.new_updated) {
-		kfree(*buf);
+		kvfree(*buf);
 		*buf = ctx.new_val;
 		*pcount = ctx.new_len;
 	} else {
-- 
2.34.1

[PATCH v3 3/3] bpf: Restore sysctl new-value from 1 to 0

[Dawei Feng]

Commit 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value
helpers") changed the success return value to 0, but failed to update the
corresponding check in __cgroup_bpf_run_filter_sysctl(). Since
bpf_prog_run_array_cg() now returns 0 on success, the legacy ret == 1
condition is never satisfied. As a result, the modified value is ignored,
and bpf_sysctl_set_new_value() fails to replace the write buffer.

Fix this by checking for a return value of 0 instead, so cgroup/sysctl
programs can correctly replace the pending sysctl buffer.

This bug was discovered during a manual code review. Tested via a
cgroup/sysctl BPF reproducer overriding writes to a target sysctl.
Pre-fix, bpf_sysctl_set_new_value("foo") was silently ignored: the write
returned 8192 and the value remained "600". Post-fix, the BPF replacement
buffer properly propagates: the write returns 3 and the value updates to
"foo".

Fixes: f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err instead of allow boolean")
Cc: stable@vger.kernel.org

Acked-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 kernel/bpf/cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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;
-- 
2.34.1

Re: [PATCH v3 3/3] bpf: Restore sysctl new-value from 1 to 0

[Jiayuan Chen]

On 6/3/26 6:53 PM, Dawei Feng wrote:
> Commit 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value
> helpers") changed the success return value to 0, but failed to update the
> corresponding check in __cgroup_bpf_run_filter_sysctl(). Since
> bpf_prog_run_array_cg() now returns 0 on success, the legacy ret == 1
> condition is never satisfied. As a result, the modified value is ignored,
> and bpf_sysctl_set_new_value() fails to replace the write buffer.
>
> Fix this by checking for a return value of 0 instead, so cgroup/sysctl
> programs can correctly replace the pending sysctl buffer.
>
> This bug was discovered during a manual code review. Tested via a
> cgroup/sysctl BPF reproducer overriding writes to a target sysctl.
> Pre-fix, bpf_sysctl_set_new_value("foo") was silently ignored: the write
> returned 8192 and the value remained "600". Post-fix, the BPF replacement
> buffer properly propagates: the write returns 3 and the value updates to
> "foo".
>
> Fixes: f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err instead of allow boolean")
> Cc: stable@vger.kernel.org
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

> 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@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>

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@mail.kernel.org/

  "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

Re: [PATCH v3 3/3] bpf: Restore sysctl new-value from 1 to 0

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 4bf0ec94e719..35d1f1428ef3 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -1936,7 +1936,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) {

The success check now tests !ret, but the kerneldoc above
__cgroup_bpf_run_filter_sysctl() still describes the old convention:

kernel/bpf/cgroup.c:
 * 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 read "returned value != 0" to match the new check?

This was raised by the bpf-ci reviewer on both v1 and v2
(https://lore.kernel.org/all/45201482368d330f6301ea4d90e29fa1585bf985c12a191d9fcd975087eb0649@mail.kernel.org/)
but the wording does not appear to have changed in v3.


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

Re: [PATCH v3 3/3] bpf: Restore sysctl new-value from 1 to 0

[Mykyta Yatsenko]

On 6/3/26 11:53 AM, Dawei Feng wrote:
> Commit 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value
> helpers") changed the success return value to 0, but failed to update the
> corresponding check in __cgroup_bpf_run_filter_sysctl(). Since
> bpf_prog_run_array_cg() now returns 0 on success, the legacy ret == 1
> condition is never satisfied. As a result, the modified value is ignored,
> and bpf_sysctl_set_new_value() fails to replace the write buffer.
> 
> Fix this by checking for a return value of 0 instead, so cgroup/sysctl
> programs can correctly replace the pending sysctl buffer.
> 
> This bug was discovered during a manual code review. Tested via a
> cgroup/sysctl BPF reproducer overriding writes to a target sysctl.
> Pre-fix, bpf_sysctl_set_new_value("foo") was silently ignored: the write
> returned 8192 and the value remained "600". Post-fix, the BPF replacement
> buffer properly propagates: the write returns 3 and the value updates to
> "foo".

I wonder if we can make that reproducer into a selftest, clearly this
codepath is not tested automatically at all, which is a problem.

> 
> Fixes: f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err instead of allow boolean")
> Cc: stable@vger.kernel.org
> 
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>  kernel/bpf/cgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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;

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

[Yonghong Song]

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@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>   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@linux.dev/)
I suggested
     memcpy(ctx->new_val, buf, buf_len + 1);
Does it work?

>   	ctx->new_len = buf_len;
>   	ctx->new_updated = 1;
>   

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

[Yonghong Song]

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@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>   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';

Okay, I looked at your v2 comment and checked the bpf_sysctl_set_new_value again.
You above implementation is correct.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

>   	ctx->new_len = buf_len;
>   	ctx->new_updated = 1;
>   

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@linux.dev> 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@seu.edu.cn>
> > Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> > ---
> >   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@linux.dev/)
> 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?

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

[Yonghong Song]

On 6/3/26 4:23 PM, Alexei Starovoitov wrote:
> On Wed, Jun 3, 2026 at 7:37 AM Yonghong Song <yonghong.song@linux.dev> 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@seu.edu.cn>
>>> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
>>> ---
>>>    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@linux.dev/)
>> 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?

The following is the bpf_sysctl_set_new_value proto:

static const struct bpf_func_proto bpf_sysctl_set_new_value_proto = {
         .func           = bpf_sysctl_set_new_value,
         .gpl_only       = false,
         .ret_type       = RET_INTEGER,
         .arg1_type      = ARG_PTR_TO_CTX,
         .arg2_type      = ARG_PTR_TO_MEM | MEM_RDONLY,
         .arg3_type      = ARG_CONST_SIZE,
};

So the input may not be a string.

Re: [PATCH v3 3/3] bpf: Restore sysctl new-value from 1 to 0

[Xu Kuohai]

On 6/3/2026 6:53 PM, Dawei Feng wrote:
> Commit 4e63acdff864 ("bpf: Introduce bpf_sysctl_{get,set}_new_value
> helpers") changed the success return value to 0, but failed to update the
> corresponding check in __cgroup_bpf_run_filter_sysctl(). Since
> bpf_prog_run_array_cg() now returns 0 on success, the legacy ret == 1
> condition is never satisfied. As a result, the modified value is ignored,
> and bpf_sysctl_set_new_value() fails to replace the write buffer.
> 
> Fix this by checking for a return value of 0 instead, so cgroup/sysctl
> programs can correctly replace the pending sysctl buffer.
> 
> This bug was discovered during a manual code review. Tested via a
> cgroup/sysctl BPF reproducer overriding writes to a target sysctl.
> Pre-fix, bpf_sysctl_set_new_value("foo") was silently ignored: the write
> returned 8192 and the value remained "600". Post-fix, the BPF replacement
> buffer properly propagates: the write returns 3 and the value updates to
> "foo".
> 
> Fixes: f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err instead of allow boolean")
> Cc: stable@vger.kernel.org
> 
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
>   kernel/bpf/cgroup.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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;

Acked-by: Xu Kuohai <xukuohai@huawei.com>

Re: [PATCH v3 0/3] bpf: fix sysctl new-value handling in __cgroup_bpf_run_filter_sysctl

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed,  3 Jun 2026 18:53:14 +0800 you wrote:
> This series fixes three bugs in the sysctl write-buffer replacement path
> of __cgroup_bpf_run_filter_sysctl(). It resolves a kvzalloc()/kfree()
> mismatch, adds a missing NUL terminator to the replacement string, and
> updates a stale return value check to safely restore the replacement
> functionality.
> 
> Patch Summary:
> - patch 1 NUL-terminates the replaced sysctl value
> - patch 2 uses kvfree() for the replaced sysctl write buffer
> - patch 3 restores sysctl new-value replacement
> 
> [...]

Here is the summary with links:
  - [v3,1/3] bpf: NUL-terminate replaced sysctl value
    https://git.kernel.org/bpf/bpf-next/c/a66e3b5bacf3
  - [v3,2/3] bpf: use kvfree() for replaced sysctl write buffer
    https://git.kernel.org/bpf/bpf-next/c/4c21b5927d43
  - [v3,3/3] bpf: Restore sysctl new-value from 1 to 0
    https://git.kernel.org/bpf/bpf-next/c/2566c3b24219

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html