Re: [PATCH bpf] bpf, cgroup: Fix storage null-ptr-deref after replacing prog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In-place updates of `item->cgroup_storage` in the active RCU-protected `bpf_prog_array` cause data races with concurrent RCU readers, leading to NULL pointer dereferences or Out-Of-Bounds memory accesses.
--

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

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

This commit fixes a storage null pointer dereference when replacing a
program by allocating, assigning, and linking the required cgroup storage
for the replacement program during link_update.

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 4355ccb78a9c6..52b5b685a93c5 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -1014,6 +1014,7 @@ static void replace_effective_prog(struct cgroup *cgrp,
>  				desc->bpf.effective[atype],
>  				lockdep_is_held(&cgroup_mutex));
>  		item = &progs->items[pos];
> +		bpf_cgroup_storages_assign(item->cgroup_storage, pl->storage);

[Severity: High]
Does this in-place update of item->cgroup_storage create a data race with
concurrent RCU readers?

If CPU1 is executing bpf_prog_run_array_cg() under the RCU read lock and
starts running the old program:

CPU1:
bpf_prog_run_array_cg()
    while ((prog = READ_ONCE(item->prog))) {
        run_ctx.prog_item = item;
        func_ret = run_prog(prog, ctx);

And CPU2 concurrently replaces the program with one that doesn't use storage,
it updates the storage in-place:

CPU2:
replace_effective_prog()
    bpf_cgroup_storages_assign(item->cgroup_storage, pl->storage);
    WRITE_ONCE(item->prog, pl->link->link.prog);

If CPU1's executing program then calls bpf_get_local_storage(), it could
read the updated NULL storage pointer while still executing the old program:

CPU1:
bpf_get_local_storage()
    storage = ctx->prog_item->cgroup_storage[stype];
    if (stype == BPF_CGROUP_STORAGE_SHARED)
        ptr = &READ_ONCE(storage->buf)->data[0];

Could this lead to a NULL pointer dereference or out-of-bounds access?

>  		WRITE_ONCE(item->prog, pl->link->link.prog);
>  	}
>  }

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