BPF List
 help / color / mirror / Atom feed
* [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
@ 2026-08-05  3:53 Pu Lehui
  2026-08-05  4:05 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Pu Lehui @ 2026-08-05  3:53 UTC (permalink / raw)
  To: bpf, linux-kernel, Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Yonghong Song,
	Song Liu, Jiri Olsa, Emil Tsalapatis, Amery Hung, Pu Lehui,
	Pu Lehui

From: Pu Lehui <pulehui@huawei.com>

Syzkaller reported a storage null-ptr-deref issue after replacing prog.
This occurs in the following scenario:
1. prog A, an empty prog, is attached to a cgrp.
2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the
   bpf_get_local_storage helper.
3. link_update is called to replace prog A with prog B.

The reason is that __cgroup_bpf_replace fails to alloc and assign the
required cgrp storage for the incoming replacement prog. Consequently,
the new prog inherits an uninit storage, leading to null-ptr-deref panic
when kick the new prog.

Fix this by rejecting link update with -EPERM if new_prog's cgroup
storage is incompatible with link->prog.

Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
Reviewed-by: Amery Hung <ameryhung@gmail.com>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
v5:
- Reject link update if new_prog's storage is incompatible with link->prog. (Andrii)

v4: https://lore.kernel.org/bpf/20260728133058.2858367-1-pulehui@huaweicloud.com
- Extract the fix for __cgroup_bpf_attach() into a standalone patch [0].
- Add Reviewed-by tag by Amery.
- Separate from patchset [1]. (Andrii)

Link: https://lore.kernel.org/bpf/20260728132336.2857800-1-pulehui@huaweicloud.com [0]
Link: https://lore.kernel.org/bpf/f87b53c0-8f00-45a6-82db-8242fa9b143f@huaweicloud.com [1]

v3: https://lore.kernel.org/bpf/20260720134547.1289964-5-pulehui@huaweicloud.com
- Include the storage and flags rollbacks to patch4 for sake of code rigor,
  as it's hard to make update_effective_progs fail in __cgroup_bpf_attach.

v2: https://lore.kernel.org/bpf/20260717073343.958862-1-pulehui@huaweicloud.com
- Fix invalid access for in-place update when storage changed. (Sashiko)

v1: https://lore.kernel.org/bpf/20260714014659.401063-1-pulehui@huaweicloud.com

 kernel/bpf/cgroup.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 57fd1d784e44..31c63ff276fa 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1026,6 +1026,20 @@ static void replace_effective_prog(struct cgroup *cgrp,
 	}
 }
 
+static bool cgroup_bpf_storages_compatible(struct bpf_prog *old_prog,
+					   struct bpf_prog *new_prog)
+{
+	enum bpf_cgroup_storage_type stype;
+
+	for_each_cgroup_storage_type(stype) {
+		if (old_prog->aux->cgroup_storage[stype] !=
+		    new_prog->aux->cgroup_storage[stype])
+			return false;
+	}
+
+	return true;
+}
+
 /**
  * __cgroup_bpf_replace() - Replace link's program and propagate the change
  *                          to descendants
@@ -1064,6 +1078,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
 	if (!found)
 		return -ENOENT;
 
+	if (!cgroup_bpf_storages_compatible(link->link.prog, new_prog))
+		return -EINVAL;
+
 	cgrp->bpf.revisions[atype] += 1;
 	old_prog = xchg(&link->link.prog, new_prog);
 	replace_effective_prog(cgrp, atype, pl);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
  2026-08-05  3:53 [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog Pu Lehui
@ 2026-08-05  4:05 ` sashiko-bot
  2026-08-05  4:31   ` Pu Lehui
  2026-08-05  4:52 ` bot+bpf-ci
  2026-08-05  4:59 ` Leon Hwang
  2 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-08-05  4:05 UTC (permalink / raw)
  To: Pu Lehui; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The commit message claims to reject the link update with -EPERM, but the code actually returns -EINVAL.

Pre-existing issues:
- [High] pl->storage is not rolled back in the error path of __cgroup_bpf_attach(), leading to a Use-After-Free of cgroup storages.
- [High] Lockless access to `link->prog` in `bpf_link_show_fdinfo` and `bpf_link_get_info_by_fd` can lead to a Use-After-Free.
--

--- Patch [1]: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog ---
commit f5ba6e7a5022bb7261a93a1c9312a6415db0249f
Author: Pu Lehui <pulehui@huawei.com>

bpf, cgroup: Fix storage null-ptr-deref after replacing prog

This commit fixes a null pointer dereference issue when replacing a BPF
program attached to a cgroup. It addresses the problem by rejecting link
updates if the new program's cgroup storage is incompatible with the existing
link's program.

Link: https://lore.kernel.org/bpf/20260728132336.2857800-1-pulehui@huaweicloud.com
Link: https://lore.kernel.org/bpf/f87b53c0-8f00-45a6-82db-8242fa9b143f@huaweicloud.com

>     Fix this by rejecting link update with -EPERM if new_prog's cgroup
>     storage is incompatible with link->prog.

[Severity: Medium]
The commit message claims to reject the link update with -EPERM, but the new
code actually returns -EINVAL. Could the commit message be updated to match
the implementation?

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 4355ccb78a9c6..5cc9b2d92fd58 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c

[ ... ]

> @@ -1056,6 +1070,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
>  	if (!found)
>  		return -ENOENT;
>  
> +	if (!cgroup_bpf_storages_compatible(link->link.prog, new_prog))
> +		return -EINVAL;
> +
>  	cgrp->bpf.revisions[atype] += 1;
>  	old_prog = xchg(&link->link.prog, new_prog);
>  	replace_effective_prog(cgrp, atype, pl);

[Severity: High]
This is a pre-existing issue, but while reviewing this code I noticed a
potential use-after-free in the error path of __cgroup_bpf_attach().

In __cgroup_bpf_attach(), bpf_cgroup_storages_assign(pl->storage, storage)
updates the storage pointers in the existing program list element pl to point
to the storages of the new program. 

If a subsequent operation like update_effective_progs() fails, the code jumps
to the cleanup label:

kernel/bpf/cgroup.c:__cgroup_bpf_attach() {
    ...
cleanup:
	if (old_prog) {
		pl->prog = old_prog;
		pl->link = NULL;
	}
	bpf_cgroup_storages_free(new_storage);
    ...
}

In this cleanup block, pl->prog is reverted to old_prog, but pl->storage is
never reverted to its original state. Any newly allocated storages for the
failed program, which reside in new_storage, are freed via
bpf_cgroup_storages_free(new_storage).

Because pl remains in the cgroup's active program list, its storage array now
contains dangling pointers to these freed RCU-reclaimed storages. Can any
future execution of the BPF program dereference them, resulting in a
use-after-free?


[Severity: High]
This is a pre-existing issue, but I also noticed potential lockless access to
link->prog which could lead to a use-after-free.

BPF link types that support dynamic program updates (such as cgroup links)
replace their attached program by atomically swapping link->prog (using xchg)
and then calling bpf_prog_put() on the old program. This decreases the
reference count and defers the actual freeing of the program until after an
RCU grace period.

However, bpf_link_show_fdinfo() fetches link->prog locklessly and
dereferences its tag without holding rcu_read_lock():

kernel/bpf/syscall.c:bpf_link_show_fdinfo() {
    ...
	const struct bpf_prog *prog = link->prog;
    ...
	if (prog) {
		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
    ...
}

Similarly, bpf_link_get_info_by_fd() fetches link->prog and accesses its aux
field without RCU protection:

kernel/bpf/syscall.c:bpf_link_get_info_by_fd() {
    ...
	if (link->prog)
		info.prog_id = link->prog->aux->id;
    ...
}

If a program update occurs concurrently and the reader thread is preempted
long enough for the RCU grace period to elapse, would the old program be
freed before the reader dereferences its memory? Should these accesses be
protected by rcu_read_lock()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805035342.349421-1-pulehui@huaweicloud.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
  2026-08-05  4:05 ` sashiko-bot
@ 2026-08-05  4:31   ` Pu Lehui
  0 siblings, 0 replies; 6+ messages in thread
From: Pu Lehui @ 2026-08-05  4:31 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: bpf



On 2026/8/5 12:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [Medium] The commit message claims to reject the link update with -EPERM, but the code actually returns -EINVAL.
> 
> Pre-existing issues:
> - [High] pl->storage is not rolled back in the error path of __cgroup_bpf_attach(), leading to a Use-After-Free of cgroup storages.
> - [High] Lockless access to `link->prog` in `bpf_link_show_fdinfo` and `bpf_link_get_info_by_fd` can lead to a Use-After-Free.
> --
> 
> --- Patch [1]: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog ---
> commit f5ba6e7a5022bb7261a93a1c9312a6415db0249f
> Author: Pu Lehui <pulehui@huawei.com>
> 
> bpf, cgroup: Fix storage null-ptr-deref after replacing prog
> 
> This commit fixes a null pointer dereference issue when replacing a BPF
> program attached to a cgroup. It addresses the problem by rejecting link
> updates if the new program's cgroup storage is incompatible with the existing
> link's program.
> 
> Link: https://lore.kernel.org/bpf/20260728132336.2857800-1-pulehui@huaweicloud.com
> Link: https://lore.kernel.org/bpf/f87b53c0-8f00-45a6-82db-8242fa9b143f@huaweicloud.com
> 
>>      Fix this by rejecting link update with -EPERM if new_prog's cgroup
>>      storage is incompatible with link->prog.
> 
> [Severity: Medium]
> The commit message claims to reject the link update with -EPERM, but the new
> code actually returns -EINVAL. Could the commit message be updated to match
> the implementation?

Oops, sorry about that. I forgot to update commit message as I thought 
-EINVAL seems more appropriate here.

I'll wait for the review feedback first. If the patch looks good 
overall, I'd be very grateful if the maintainer could fix up the commit 
message on apply. Otherwise, I'm happy to resend.


> 
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index 4355ccb78a9c6..5cc9b2d92fd58 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
> 
> [ ... ]
> 
>> @@ -1056,6 +1070,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
>>   	if (!found)
>>   		return -ENOENT;
>>   
>> +	if (!cgroup_bpf_storages_compatible(link->link.prog, new_prog))
>> +		return -EINVAL;
>> +
>>   	cgrp->bpf.revisions[atype] += 1;
>>   	old_prog = xchg(&link->link.prog, new_prog);
>>   	replace_effective_prog(cgrp, atype, pl);
> 
> [Severity: High]
> This is a pre-existing issue, but while reviewing this code I noticed a
> potential use-after-free in the error path of __cgroup_bpf_attach().
> 
> In __cgroup_bpf_attach(), bpf_cgroup_storages_assign(pl->storage, storage)
> updates the storage pointers in the existing program list element pl to point
> to the storages of the new program.

Fixed.

https://git.kernel.org/bpf/bpf-next/c/6655c409707e

> 
> If a subsequent operation like update_effective_progs() fails, the code jumps
> to the cleanup label:
> 
> kernel/bpf/cgroup.c:__cgroup_bpf_attach() {
>      ...
> cleanup:
> 	if (old_prog) {
> 		pl->prog = old_prog;
> 		pl->link = NULL;
> 	}
> 	bpf_cgroup_storages_free(new_storage);
>      ...
> }
> 
> In this cleanup block, pl->prog is reverted to old_prog, but pl->storage is
> never reverted to its original state. Any newly allocated storages for the
> failed program, which reside in new_storage, are freed via
> bpf_cgroup_storages_free(new_storage).
> 
> Because pl remains in the cgroup's active program list, its storage array now
> contains dangling pointers to these freed RCU-reclaimed storages. Can any
> future execution of the BPF program dereference them, resulting in a
> use-after-free?
> 
> 
> [Severity: High]
> This is a pre-existing issue, but I also noticed potential lockless access to
> link->prog which could lead to a use-after-free.
> 
> BPF link types that support dynamic program updates (such as cgroup links)
> replace their attached program by atomically swapping link->prog (using xchg)
> and then calling bpf_prog_put() on the old program. This decreases the
> reference count and defers the actual freeing of the program until after an
> RCU grace period.
> 
> However, bpf_link_show_fdinfo() fetches link->prog locklessly and
> dereferences its tag without holding rcu_read_lock():
> 
> kernel/bpf/syscall.c:bpf_link_show_fdinfo() {
>      ...
> 	const struct bpf_prog *prog = link->prog;
>      ...
> 	if (prog) {
> 		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
>      ...
> }
> 
> Similarly, bpf_link_get_info_by_fd() fetches link->prog and accesses its aux
> field without RCU protection:
> 
> kernel/bpf/syscall.c:bpf_link_get_info_by_fd() {
>      ...
> 	if (link->prog)
> 		info.prog_id = link->prog->aux->id;
>      ...
> }
> 
> If a program update occurs concurrently and the reader thread is preempted
> long enough for the RCU grace period to elapse, would the old program be
> freed before the reader dereferences its memory? Should these accesses be
> protected by rcu_read_lock()?
> 

Fixed.

https://git.kernel.org/bpf/bpf-next/c/863f3ddd0b8a


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
  2026-08-05  3:53 [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog Pu Lehui
  2026-08-05  4:05 ` sashiko-bot
@ 2026-08-05  4:52 ` bot+bpf-ci
  2026-08-05  4:59 ` Leon Hwang
  2 siblings, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-08-05  4:52 UTC (permalink / raw)
  To: pulehui, bpf, linux-kernel, andrii
  Cc: ast, daniel, eddyz87, memxor, martin.lau, yonghong.song, song,
	jolsa, emil, ameryhung, pulehui, pulehui, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

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

> commit c953408b860e553dc6831c7b95e870d0d1ef878d
> Author: Pu Lehui <pulehui@huawei.com>
> Date:   Wed Aug 5 03:53:42 2026 +0000
>
>     bpf, cgroup: Fix storage null-ptr-deref after replacing prog
>
>     Syzkaller reported a storage null-ptr-deref issue after replacing prog.
>     This occurs in the following scenario:
>     1. prog A, an empty prog, is attached to a cgrp.
>     2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the
>        bpf_get_local_storage helper.
>     3. link_update is called to replace prog A with prog B.
>
>     The reason is that __cgroup_bpf_replace fails to alloc and assign the
>     required cgrp storage for the incoming replacement prog. Consequently,
>     the new prog inherits an uninit storage, leading to null-ptr-deref panic
>     when kick the new prog.
>
>     Fix this by rejecting link update with -EPERM if new_prog's cgroup
>     storage is incompatible with link->prog.
>
>     Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
>     Reviewed-by: Amery Hung <ameryhung@gmail.com>
>     Signed-off-by: Pu Lehui <pulehui@huawei.com>

The commit message says "Syzkaller reported" but there's no Reported-by
or Closes tag. Two public syzbot reports exist for this exact issue:

  - syzbot+4fe468a3f7fac86ea2c9@syzkaller.appspotmail.com
  - syzbot+e6e8f6618a2d4b35e4e0@syzkaller.appspotmail.com

Both reports contain KASAN null-ptr-deref traces at
____bpf_get_local_storage, and the public C reproducer matches the
described 3-step scenario exactly.

Should the commit include Reported-by and Closes tags so syzbot can
auto-close these reports?

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 4355ccb78a9c..5cc9b2d92fd5 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c

[ ... ]


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
  2026-08-05  3:53 [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog Pu Lehui
  2026-08-05  4:05 ` sashiko-bot
  2026-08-05  4:52 ` bot+bpf-ci
@ 2026-08-05  4:59 ` Leon Hwang
  2026-08-07 10:31   ` Pu Lehui
  2 siblings, 1 reply; 6+ messages in thread
From: Leon Hwang @ 2026-08-05  4:59 UTC (permalink / raw)
  To: Pu Lehui, bpf, linux-kernel, Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Yonghong Song,
	Song Liu, Jiri Olsa, Emil Tsalapatis, Amery Hung, Pu Lehui

On 5/8/26 11:53, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> Syzkaller reported a storage null-ptr-deref issue after replacing prog.
> This occurs in the following scenario:
> 1. prog A, an empty prog, is attached to a cgrp.
> 2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the
>    bpf_get_local_storage helper.
> 3. link_update is called to replace prog A with prog B.
> 
> The reason is that __cgroup_bpf_replace fails to alloc and assign the
> required cgrp storage for the incoming replacement prog. Consequently,
> the new prog inherits an uninit storage, leading to null-ptr-deref panic
> when kick the new prog.
> 
> Fix this by rejecting link update with -EPERM if new_prog's cgroup
> storage is incompatible with link->prog.
> 
> Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
> Reviewed-by: Amery Hung <ameryhung@gmail.com>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>


Looks much simpler than the previous versions.

Other than the -EPERM in the commit msg, lgtm.

Acked-by: Leon Hwang <leon.hwang@linux.dev>

> [...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog
  2026-08-05  4:59 ` Leon Hwang
@ 2026-08-07 10:31   ` Pu Lehui
  0 siblings, 0 replies; 6+ messages in thread
From: Pu Lehui @ 2026-08-07 10:31 UTC (permalink / raw)
  To: Leon Hwang, Pu Lehui, bpf, linux-kernel, Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Yonghong Song,
	Song Liu, Jiri Olsa, Emil Tsalapatis, Amery Hung



On 2026/8/5 12:59, Leon Hwang wrote:
> On 5/8/26 11:53, Pu Lehui wrote:
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> Syzkaller reported a storage null-ptr-deref issue after replacing prog.
>> This occurs in the following scenario:
>> 1. prog A, an empty prog, is attached to a cgrp.
>> 2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the
>>     bpf_get_local_storage helper.
>> 3. link_update is called to replace prog A with prog B.
>>
>> The reason is that __cgroup_bpf_replace fails to alloc and assign the
>> required cgrp storage for the incoming replacement prog. Consequently,
>> the new prog inherits an uninit storage, leading to null-ptr-deref panic
>> when kick the new prog.
>>
>> Fix this by rejecting link update with -EPERM if new_prog's cgroup
>> storage is incompatible with link->prog.
>>
>> Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
>> Reviewed-by: Amery Hung <ameryhung@gmail.com>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> 
> 
> Looks much simpler than the previous versions.
> 
> Other than the -EPERM in the commit msg, lgtm.

Thanks review, I'd better send a new version first.

> 
> Acked-by: Leon Hwang <leon.hwang@linux.dev>
> 
>> [...]
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-07 10:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  3:53 [PATCH bpf v5] bpf, cgroup: Fix storage null-ptr-deref after replacing prog Pu Lehui
2026-08-05  4:05 ` sashiko-bot
2026-08-05  4:31   ` Pu Lehui
2026-08-05  4:52 ` bot+bpf-ci
2026-08-05  4:59 ` Leon Hwang
2026-08-07 10:31   ` Pu Lehui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox