[PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

[PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

A potential invalid storage access issue can occur after replacing a
cgroup bpf prog.

This occurs in the following scenario:
1. prog1 with storage is attached to a cgroup in multi-attach mode.
2. prog1 is replaced with prog2 using BPF_F_REPLACE in multi-attach
   mode, but fails midway (e.g. in bpf_trampoline_link_cgroup_shim or
   update_effective_progs).
3. A new prog3 is attached to the cgroup in multi-attach mode.

The reason is that __cgroup_bpf_attach overwrites pl->storage with the
new storage prior to attachment completion. When attachment fails
midway, the cleanup path calls bpf_cgroup_storages_free(new_storage) to
free the newly allocated storage, but fails to restore pl->storage back
to old_storage.

Consequently, the still-active prog1 holds invalid or dangling storage
pointers, leading to an invalid memory access when prog1 executes and
calls bpf_get_local_storage. Additionally, original pl->flags and
cgrp->bpf.flags[atype] are left unrestored.

Fix this by saving old_pl_flags, old_storage, and old_flags prior to the
update, and properly restoring all of them in the cleanup path on error.

Fixes: 7d9c3427894f ("bpf: Make cgroup storages shared between programs on the same cgroup")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
v3:
- Remove confusing commit message. (Emil)
- Add Reviewed-by tag by Emil.

v2: https://lore.kernel.org/bpf/20260729100208.3076769-1-pulehui@huaweicloud.com
- Remove the link relative code as link only support
  BPF_F_ALLOW_MULTI attach, so will not occur UAF pl.

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

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

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index e2fa0ebeed83..be24ca453cab 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -813,8 +813,10 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 	struct bpf_prog *old_prog = NULL;
 	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
+	struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {};
 	struct bpf_prog *new_prog = prog ? : link->link.prog;
 	enum cgroup_bpf_attach_type atype;
+	u32 old_flags, old_pl_flags;
 	struct bpf_prog_list *pl;
 	struct hlist_head *progs;
 	int err;
@@ -865,6 +867,8 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 
 	if (pl) {
 		old_prog = pl->prog;
+		old_pl_flags = pl->flags;
+		bpf_cgroup_storages_assign(old_storage, pl->storage);
 	} else {
 		pl = kmalloc_obj(*pl);
 		if (!pl) {
@@ -884,6 +888,7 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 	pl->link = link;
 	pl->flags = flags;
 	bpf_cgroup_storages_assign(pl->storage, storage);
+	old_flags = cgrp->bpf.flags[atype];
 	cgrp->bpf.flags[atype] = saved_flags;
 
 	if (type == BPF_LSM_CGROUP) {
@@ -915,12 +920,15 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp,
 	if (old_prog) {
 		pl->prog = old_prog;
 		pl->link = NULL;
+		pl->flags = old_pl_flags;
+		bpf_cgroup_storages_assign(pl->storage, old_storage);
 	}
 	bpf_cgroup_storages_free(new_storage);
 	if (!old_prog) {
 		hlist_del(&pl->node);
 		kfree(pl);
 	}
+	cgrp->bpf.flags[atype] = old_flags;
 	return err;
 }
 
-- 
2.34.1

Re: [PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

[sashiko-bot]

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

Pre-existing issues:
- [Critical] When updating a cgroup BPF link via `bpf_link_update`, `__cgroup_bpf_replace` does not allocate new `cgroup_storage` for the new program and lacks map compatibility checks, leading to severe out-of-bounds kernel memory access.
--

--- Patch [1]: [PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed ---
commit 91e5396c1d1312a251035a5e4902151e56fc5bdf
Author: Pu Lehui <pulehui@huawei.com>

bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

This commit fixes a potential invalid storage access issue when replacing a
cgroup BPF program in multi-attach mode fails midway. It saves the old
program's state and storage prior to the update, and properly restores them
in the cleanup path on error.

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

[ ... ]

While reviewing this file, I noticed a potential issue in __cgroup_bpf_replace():

> static int __cgroup_bpf_replace(struct cgroup *cgrp,
> 				struct bpf_cgroup_link *link,
> 				struct bpf_prog *new_prog)
> {
[ ... ]
> 	cgrp->bpf.revisions[atype] += 1;
> 	old_prog = xchg(&link->link.prog, new_prog);

[Severity: Critical]
This is a pre-existing issue, but does this function properly handle
cgroup_storage when replacing the program?

If a user replaces an existing cgroup BPF program using bpf_link_update with
a new program that uses a BPF_MAP_TYPE_CGROUP_STORAGE map with a larger
value_size, could this result in an out-of-bounds memory access?

It appears we swap the attached BPF program on the link here without
verifying that the new program's cgroup_storage maps have identical
value sizes to the old program, nor do we allocate new appropriately
sized storage buffers.

Since the verifier authorized the memory access for the new program based on
its larger map size, could bpf_get_local_storage return the smaller old
buffer leading to out-of-bounds read and write accesses?

> 	replace_effective_prog(cgrp, atype, pl);
> 	bpf_prog_put(old_prog);
> 	return 0;
> }

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

Re: [PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

[Pu Lehui]

On 2026/8/3 9:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] When updating a cgroup BPF link via `bpf_link_update`, `__cgroup_bpf_replace` does not allocate new `cgroup_storage` for the new program and lacks map compatibility checks, leading to severe out-of-bounds kernel memory access.
> --
> 
> --- Patch [1]: [PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed ---
> commit 91e5396c1d1312a251035a5e4902151e56fc5bdf
> Author: Pu Lehui <pulehui@huawei.com>
> 
> bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed
> 
> This commit fixes a potential invalid storage access issue when replacing a
> cgroup BPF program in multi-attach mode fails midway. It saves the old
> program's state and storage prior to the update, and properly restores them
> in the cleanup path on error.
> 
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index 4355ccb78a9c6..57fd1d784e440 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
> 
> [ ... ]
> 
> While reviewing this file, I noticed a potential issue in __cgroup_bpf_replace():
> 
>> static int __cgroup_bpf_replace(struct cgroup *cgrp,
>> 				struct bpf_cgroup_link *link,
>> 				struct bpf_prog *new_prog)
>> {
> [ ... ]
>> 	cgrp->bpf.revisions[atype] += 1;
>> 	old_prog = xchg(&link->link.prog, new_prog);
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this function properly handle
> cgroup_storage when replacing the program?
> 
> If a user replaces an existing cgroup BPF program using bpf_link_update with
> a new program that uses a BPF_MAP_TYPE_CGROUP_STORAGE map with a larger
> value_size, could this result in an out-of-bounds memory access?
> 
> It appears we swap the attached BPF program on the link here without
> verifying that the new program's cgroup_storage maps have identical
> value sizes to the old program, nor do we allocate new appropriately
> sized storage buffers.
> 
> Since the verifier authorized the memory access for the new program based on
> its larger map size, could bpf_get_local_storage return the smaller old
> buffer leading to out-of-bounds read and write accesses?

The following patch has been covered this issue.

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

> 
>> 	replace_effective_prog(cgrp, atype, pl);
>> 	bpf_prog_put(old_prog);
>> 	return 0;
>> }
> 

Re: [PATCH bpf v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon,  3 Aug 2026 01:39:34 +0000 you wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> A potential invalid storage access issue can occur after replacing a
> cgroup bpf prog.
> 
> This occurs in the following scenario:
> 1. prog1 with storage is attached to a cgroup in multi-attach mode.
> 2. prog1 is replaced with prog2 using BPF_F_REPLACE in multi-attach
>    mode, but fails midway (e.g. in bpf_trampoline_link_cgroup_shim or
>    update_effective_progs).
> 3. A new prog3 is attached to the cgroup in multi-attach mode.
> 
> [...]

Here is the summary with links:
  - [bpf,v3] bpf, cgroup: Fix invalid storage access after __cgroup_bpf_attach failed
    https://git.kernel.org/bpf/bpf-next/c/6655c409707e

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