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

On 2026/8/3 9:50, [email protected] 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 <[email protected]>
> 
> 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/[email protected]/

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