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

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

[Cen Zhang (Microsoft Security FORGE Labs)]

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.
A concurrent socket close can unlink the selem and clear that pointer
between the two stages, causing a NULL dereference of sk_storage->owner.

Oops: general protection fault, probably for non-canonical
   address 0xdffffc0000000011
  net/core/bpf_sk_storage.c:809 __bpf_sk_storage_map_seq_show()
  bpf_seq_read+0x366/0x1120
  vfs_read+0x174/0xa50
  ksys_read+0xfc/0x1d0

Return SEQ_SKIP if the re-read yields NULL. This prevents the dereference
and tells bpf_seq_read() that the stale element was skipped, so it does
not consume an iterator sequence number without running the BPF program.

Fixes: 0be08389c7f2 ("bpf: Switch to bpf_selem_unlink_nofail in bpf_local_storage_{map_free, destroy}")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Closes: https://lore.kernel.org/all/20260827051859.45511-1-blbllhy@gmail.com/
Suggested-by: Amery Hung <ameryhung@gmail.com>
Suggested-by: Xuanqiang Luo <xuanqiang.luo@linux.dev>
Link: https://lore.kernel.org/all/CAMB2axNFOC9G2RwOCnsWDth83REMWnmPE8gxMwbLYoGusw9miA@mail.gmail.com/
Link: https://lore.kernel.org/all/c3f2a61d-d5bc-454c-987d-717b5f8c8809@linux.dev/
Cc: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Assisted-by: Copilot (Grok 4.6)
---
Changes in v2:
- Return SEQ_SKIP instead of 0 when the storage disappeared, preserving
  the iterator sequence number for the next valid element.
- Correct the Fixes tag to the commit that switched the destruction path
  to bpf_selem_unlink_nofail().
- Rebase onto the current bpf master branch.

 net/core/bpf_sk_storage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 1d295a8769fa..7d02245aa704 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;
 			ctx.value = SDATA(selem)->data;
 		}

base-commit: 15071f2a1263e82150c77eeb1e94dbfc31950a8e
-- 
2.55.0

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

[luoxuanqiang]

在 2026/9/11 22:03, Cen Zhang (Microsoft Security FORGE Labs) 写道:

> 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.
> A concurrent socket close can unlink the selem and clear that pointer
> between the two stages, causing a NULL dereference of sk_storage->owner.
>
> Oops: general protection fault, probably for non-canonical
>     address 0xdffffc0000000011
>    net/core/bpf_sk_storage.c:809 __bpf_sk_storage_map_seq_show()
>    bpf_seq_read+0x366/0x1120
>    vfs_read+0x174/0xa50
>    ksys_read+0xfc/0x1d0
>
> Return SEQ_SKIP if the re-read yields NULL. This prevents the dereference
> and tells bpf_seq_read() that the stale element was skipped, so it does
> not consume an iterator sequence number without running the BPF program.
>
> Fixes: 0be08389c7f2 ("bpf: Switch to bpf_selem_unlink_nofail in bpf_local_storage_{map_free, destroy}")
> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Closes: https://lore.kernel.org/all/20260827051859.45511-1-blbllhy@gmail.com/
> Suggested-by: Amery Hung <ameryhung@gmail.com>
> Suggested-by: Xuanqiang Luo <xuanqiang.luo@linux.dev>
> Link: https://lore.kernel.org/all/CAMB2axNFOC9G2RwOCnsWDth83REMWnmPE8gxMwbLYoGusw9miA@mail.gmail.com/
> Link: https://lore.kernel.org/all/c3f2a61d-d5bc-454c-987d-717b5f8c8809@linux.dev/
> Cc: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
> Assisted-by: Copilot (Grok 4.6)

Reviewed-by: Xuanqiang Luo<luoxuanqiang@kylinos.cn>

Thanks,
Xuanqiang

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

[sashiko-bot]

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

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

[Cen Zhang (Microsoft Security FORGE Labs)]

> This is a pre-existing issue, but does assigning the socket pointer to
> ctx.sk ... introduce a use-after-free risk for non-RCU-safe sockets?

Yes. As Amery noted in the v1 discussion, "It looks like a valid issue
that should be addressed in another patch" [1]. It requires separate
handling and is independent of the NULL selem->local_storage dereference
fixed here.

[1] https://lore.kernel.org/all/CAMB2axPQ1LVXNybsPY7RNa+12+7mtLaahvr0vFL-xHf8sbJK8A@mail.gmail.com/

Thanks,
Cen