Re: [PATCH bpf v2] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Passing `sk_storage->owner` to the BPF program in `__bpf_sk_storage_map_seq_show` introduces a Use-After-Free for non-RCU-safe sockets because the socket memory can be synchronously freed concurrently.
--

commit 64d29cd038f4fabc766558da2cca84aac8eec45a
Author: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>

bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show

Iterating a sk_storage map is a two-stage operation:
bpf_sk_storage_map_seq_find_next() returns a selem, then
__bpf_sk_storage_map_seq_show() uses it. The latter re-reads
selem->local_storage via rcu_dereference() without checking for NULL.

Link: https://lore.kernel.org/all/CAMB2axNFOC9G2RwOCnsWDth83REMWnmPE8gxMwbLYoGusw9miA@mail.gmail.com/
Link: https://lore.kernel.org/all/c3f2a61d-d5bc-454c-987d-717b5f8c8809@linux.dev/

> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 1d295a8769fad..7d02245aa7049 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -806,6 +806,8 @@ static int __bpf_sk_storage_map_seq_show(struct seq_file *seq,
>  		ctx.map = info->map;
>  		if (selem) {
>  			sk_storage = rcu_dereference(selem->local_storage);
> +			if (!sk_storage)
> +				return SEQ_SKIP;
>  			ctx.sk = sk_storage->owner;

[Severity: High]
This is a pre-existing issue, but does assigning the socket pointer to ctx.sk
without taking a reference or ensuring RCU protection introduce a
use-after-free risk for non-RCU-safe sockets?

For sockets that are not RCU-safe, such as UNIX domain sockets, the socket
memory can be synchronously freed. If CPU 1 is delayed after reading
selem->local_storage in __bpf_sk_storage_map_seq_show(), CPU 2 can execute
the final sock_put() and call __sk_destruct().

Because SOCK_RCU_FREE is not set, __sk_destruct() immediately calls
bpf_sk_storage_free() followed by sk_prot_free(sk), which frees the socket
memory to the slab allocator.

When CPU 1 resumes, it will call bpf_iter_run_prog(prog, &ctx) with ctx.sk
pointing to freed memory.

>  			ctx.value = SDATA(selem)->data;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911140308.81560-1-cenzhang@linux.microsoft.com?part=1