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

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

[Cen Zhang (Microsoft Security FORGE Labs)]

Iterating a sk_storage map is two stages:
bpf_sk_storage_map_seq_find_next()
returns a selem, then __bpf_sk_storage_map_seq_show() uses that selem.
__bpf_sk_storage_map_seq_show() re-reads selem->local_storage via
rcu_dereference() with no NULL check. A concurrent close() can run
bpf_selem_unlink_nofail() in between and set that field to NULL, 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 if the re-read yields NULL.

Fixes: 5d800f87d0a5 ("bpf: Support lockless unlink when freeing map or local storage")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Cc: AutonomousCodeSecurity@microsoft.com
Assisted-by: Copilot (Grok 4.6)
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
---
 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..7228520aa2b6 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 0;
 			ctx.sk = sk_storage->owner;
 			ctx.value = SDATA(selem)->data;
 		}
-- 
2.55.0

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4801 bytes --]

> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 1d295a8769fa..7228520aa2b6 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 0;
                                   ^
Should this return SEQ_SKIP instead of 0?

Returning 0 tells bpf_seq_read() that the object was shown, but it was
actually skipped. In kernel/bpf/bpf_iter.c bpf_seq_read():

	err = seq->op->show(seq, p);
	if (err > 0) {
		/* object is skipped, decrease seq_num, so next
		 * valid object can reuse the same seq_num.
		 */
		bpf_iter_dec_seq_num(seq);
		seq->count = offs;
	} else if (err < 0 || seq_has_overflowed(seq)) {

With return 0, bpf_iter_dec_seq_num() is not called, so the skipped
element consumes a seq_num that no BPF program invocation ever observes.

If the skipped element is the first one returned by
bpf_sk_storage_map_seq_start(), seq_num 0 is consumed with no program
run, the next valid element is shown with ctx->meta->seq_num == 1, and
iterator programs that emit a header on `ctx->meta->seq_num == 0`
silently lose their header. Examples that would break:

tools/testing/selftests/bpf/progs/bpf_iter_bpf_hash_map.c:
	if (ctx->meta->seq_num == 0)
		BPF_SEQ_PRINTF(seq, "      id   refcnt  ...\n");

tools/testing/selftests/bpf/progs/bpf_iter_tcp4.c:
	if (ctx->meta->seq_num == 0)
		BPF_SEQ_PRINTF(seq, "  sl  local_address ...\n");

For later elements the observable effect is a gap in the seq_num
sequence.

The other socket iterators handle this exact situation (object found
stale after handed to ->show()) by returning SEQ_SKIP:

net/ipv4/tcp_ipv4.c bpf_iter_tcp_seq_show():
	if (unlikely(sk_unhashed(sk))) {
		ret = SEQ_SKIP;
		goto unlock;
	}

net/ipv4/udp.c bpf_iter_udp_seq_show():
	if (unlikely(sk_unhashed(sk))) {
		ret = SEQ_SKIP;
		goto unlock;
	}

net/unix/af_unix.c bpf_iter_unix_seq_show():
	ret = SEQ_SKIP;

Returning SEQ_SKIP is safe for the first object too: seq_num is u64
and bpf_iter_dec_seq_num()/bpf_iter_inc_seq_num() in
kernel/bpf/bpf_iter.c wrap back to 0 before the next ->show().

---

A subsystem pattern flags the following as potentially concerning:

>  			sk_storage = rcu_dereference(selem->local_storage);
> +			if (!sk_storage)
> +				return 0;
>  			ctx.sk = sk_storage->owner;
>  			ctx.value = SDATA(selem)->data;

The fix adds a single check-based abort path for a race that has no
exclusion mechanism, and the instructions that follow the abort point
still consume the contested selem.

Looking at bpf_selem_unlink_nofail() in kernel/bpf/bpf_local_storage.c,
it tears the selem down in this order:

1. b->lock: hlist_del_init_rcu(&selem->map_node);
   bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
2. RCU_INIT_POINTER(SDATA(selem)->smap, NULL);
3. local_storage->lock: hlist_del_init_rcu(&selem->snode);
4. RCU_INIT_POINTER(selem->local_storage, NULL);    <- new check sees this

A concurrent iterator that re-reads selem->local_storage between steps
1 and 4 still sees a non-NULL pointer and proceeds to:

	ctx.sk = sk_storage->owner;
	/* owner is deliberately NOT cleared in the destroy path:
	 * "Okay to skip clearing owner_storage and storage->owner
	 * in destroy() since the owner is going away" - so ctx.sk
	 * can be a sock already inside __sk_destruct() */

	ctx.value = SDATA(selem)->data;
	/* bpf_obj_free_fields() has already released the value's
	 * special fields (kptr/timer/etc.) at step 1 */

So the NULL check closes the specific general-protection fault in the
report (sk_storage == NULL, offset 0x88 == offsetof(struct
bpf_local_storage, owner), matching the KASAN address
0xdffffc0000000011) but does not make the iteration race-safe; it
narrows the window rather than closing it.

Worth confirming with the local-storage maintainers whether the
iterator needs to observe selem->state / SDATA(selem)->smap (or take
the same exclusion bpf_selem_unlink_nofail uses) rather than only
local_storage?

The residual window is not introduced by this patch - the same two
lines executed unconditionally before it, so relative to the parent
commit the change is a strict improvement. Practical impact of the
residual window is also limited: ctx.sk is registered
PTR_TO_BTF_ID_OR_NULL so programs must NULL-check it, and ctx.value is
PTR_TO_BUF, so a BPF program cannot dereference a released kptr read
out of it without a kfunc.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33042447773

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

[Amery Hung]

On Wed, Aug 26, 2026 at 10:19 PM Cen Zhang (Microsoft Security FORGE
Labs) <blbllhy@gmail.com> wrote:
>
> Iterating a sk_storage map is two stages:
> bpf_sk_storage_map_seq_find_next()
> returns a selem, then __bpf_sk_storage_map_seq_show() uses that selem.
> __bpf_sk_storage_map_seq_show() re-reads selem->local_storage via
> rcu_dereference() with no NULL check. A concurrent close() can run
> bpf_selem_unlink_nofail() in between and set that field to NULL, 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 if the re-read yields NULL.
>
> Fixes: 5d800f87d0a5 ("bpf: Support lockless unlink when freeing map or local storage")
> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Cc: AutonomousCodeSecurity@microsoft.com
> Assisted-by: Copilot (Grok 4.6)
> Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
> ---
>  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..7228520aa2b6 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 0;

As pointed out by CI bot, should this be SEQ_SKIP?

On a seperate note, __cgroup_iter_seq_show() also seem to return the
wrong value.

>                         ctx.sk = sk_storage->owner;
>                         ctx.value = SDATA(selem)->data;
>                 }
> --
> 2.55.0

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

[Xuanqiang Luo]

Hi Cen Zhang,

在 2026/8/27 13:18, Cen Zhang (Microsoft Security FORGE Labs) 写道:

> Iterating a sk_storage map is two stages:
> bpf_sk_storage_map_seq_find_next()
> returns a selem, then __bpf_sk_storage_map_seq_show() uses that selem.
> __bpf_sk_storage_map_seq_show() re-reads selem->local_storage via
> rcu_dereference() with no NULL check. A concurrent close() can run
> bpf_selem_unlink_nofail() in between and set that field to NULL, 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 if the re-read yields NULL.

I also reproduced this issue and came across your patch while preparing
to submit a fix. Do you have any plans to send a v2?

Here is the crash log from my reproducer:

[  756.754624] Unable to handle kernel paging request at virtual address dfff800000000011
[  756.754631] KASAN: null-ptr-deref in range [0x0000000000000088-0x000000000000008f]
[  756.754635] Mem abort info:
[  756.754638]   ESR = 0x0000000096000006
[  756.754641]   EC = 0x25: DABT (current EL), IL = 32 bits
[  756.754645]   SET = 0, FnV = 0
[  756.754649]   EA = 0, S1PTW = 0
[  756.754651]   FSC = 0x06: level 2 translation fault
[  756.754655] Data abort info:
[  756.754657]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[  756.754660]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[  756.754664]   GCS = 0, Overlay = 0, DirtyBit = 0
[  756.754667] [dfff800000000011] address between user and kernel address ranges
[  756.754673] Internal error: Oops: 0000000096000006 [#1]  SMP
[  756.815224] Modules linked in: vfat fat sr_mod cdrom virtio_scsi virtio_console cfg80211 rfkill binfmt_misc sch_fq_codel virtio_net net_failover failover fuse vsock_loopback vmw_vsock_virtio_transport_common vsock virtio_rng
[  756.821030] CPU: 1 UID: 0 PID: 914 Comm: poc Tainted: G        W           7.3.0-rc1+ #33 PREEMPTLAZY
[  756.823909] Tainted: [W]=WARN
[  756.824703] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[  756.826443] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  756.828224] pc : __bpf_sk_storage_map_seq_show+0x158/0x1d8
[  756.830842] lr : __bpf_sk_storage_map_seq_show+0x148/0x1d8
[  756.832300] sp : ffff8000a32afa30
[  756.833144] x29: ffff8000a32afa30 x28: ffff0000ceb5ec80 x27: ffff0000ceb5eca8
[  756.834956] x26: ffff0000d516d400 x25: ffffa9f37d85e3a0 x24: ffff8000a32afad0
[  756.836764] x23: 1ffff00014655f4e x22: ffff8000a3300000 x21: ffff0000d516d400
[  756.838716] x20: 0000000000000000 x19: ffffa9f37d85e080 x18: 0000000000000000
[  756.840556] x17: 65642d6572702030 x16: 3030303033336130 x15: 303038666666663d
[  756.842400] x14: 0000000000000000 x13: 0000000000000001 x12: ffff700014655eef
[  756.844210] x11: 1ffff00014655eee x10: ffff700014655eee x9 : ffffa9f37a9be66c
[  756.846018] x8 : 00008fffeb9aa112 x7 : ffff8000a32af777 x6 : 0000000000000001
[  756.847825] x5 : ffff8000a32af770 x4 : ffff700014655eef x3 : ffffa9f37a9be380
[  756.849630] x2 : 0000000000000011 x1 : dfff800000000000 x0 : 0000000000000088
[  756.851437] Call trace:
[  756.852086]  __bpf_sk_storage_map_seq_show+0x158/0x1d8 (P)
[  756.853498]  bpf_sk_storage_map_seq_show+0x18/0x30
[  756.854715]  bpf_seq_read+0x2ec/0xd28
[  756.855707]  vfs_read+0x188/0x7b0
[  756.856577]  ksys_read+0xe4/0x1b8
[  756.857424]  __arm64_sys_read+0x78/0xb0
[  756.858402]  invoke_syscall+0x78/0x260
[  756.859372]  el0_svc_common.constprop.0+0xb0/0x240
[  756.860585]  do_el0_svc+0x4c/0x70
[  756.861430]  el0_svc+0x54/0x118
[  756.862287]  el0t_64_sync_handler+0xa0/0xe8
[  756.863352]  el0t_64_sync+0x198/0x1a0
[  756.864288] Code: 91022280 d2d00001 f2fbffe1 d343fc02 (38e16841)
[  756.865828] ---[ end trace 0000000000000000 ]---

> Fixes: 5d800f87d0a5 ("bpf: Support lockless unlink when freeing map or local storage")

Would 0be08389c7f2 be a more accurate Fixes tag?

5d800f87d0a5 only introduced bpf_selem_unlink_nofail(), while
0be08389c7f2 switched the destruction path to use it.

> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Cc: AutonomousCodeSecurity@microsoft.com
> Assisted-by: Copilot (Grok 4.6)
> Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
> ---
>   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..7228520aa2b6 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 0;

Also, I agree that returning SEQ_SKIP would be appropriate here.

Thanks,
Xuanqiang

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

[Cen Zhang (Microsoft Security FORGE Labs)]

Hi Amery and Xuanqiang,

Sorry for the delayed response. I somehow missed this thread.

For clarity, I am using my new Microsoft-approved email address,
cenzhang@linux.microsoft.com, for this reply and v2.

Thanks for all the reviews. I'll send v2 shortly with SEQ_SKIP as the
return value and the suggested Fixes tag.

Thanks,
Cen