BPF List
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
@ 2026-08-27  5:18 Cen Zhang (Microsoft Security FORGE Labs)
  2026-08-27  5:38 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Cen Zhang (Microsoft Security FORGE Labs) @ 2026-08-27  5:18 UTC (permalink / raw)
  To: ast, daniel, martin.lau
  Cc: ameryhung, davem, edumazet, kuba, pabeni, horms, wangfushuai,
	bestswngs, mattbobrowski, kees, menglong8.dong, bpf, netdev,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys,
	blbllhy

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-08-27  5:18 [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show Cen Zhang (Microsoft Security FORGE Labs)
@ 2026-08-27  5:38 ` sashiko-bot
  2026-08-27 10:33   ` Amery Hung
  2026-08-27  5:49 ` bot+bpf-ci
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-27  5:38 UTC (permalink / raw)
  To: Cen Zhang (Microsoft Security FORGE Labs); +Cc: bpf

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

Pre-existing issues:
- [High] Socket local storage elements are removed from the RCU-protected BPF map inside the socket's destruction path (`__sk_destruct`), violating RCU-001 rules and causing a use-after-free of the socket because the socket can be freed while a BPF iterator is still using it.
- [High] `bpf_selem_unlink_nofail()` concurrently sets `SDATA(selem)->smap` and `selem->local_storage` to NULL, causing guaranteed NULL pointer dereferences in concurrent `bpf_selem_unlink()` and `bpf_selem_unlink_map()` paths.
--

commit cabffb2f3787054c5c6ba2a413eaf5dc639ba23d
Author: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show

This commit addresses a NULL pointer dereference in
__bpf_sk_storage_map_seq_show() during concurrent socket closure. It adds a
NULL check after re-reading selem->local_storage to prevent dereferencing a
cleared pointer.

> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index 1d295a8769fad..7228520aa2b6e 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;

[Severity: High]
This is a pre-existing issue, but does this patch fully address the
underlying TOCTOU race?

If a socket without SOCK_RCU_FREE (e.g., AF_UNIX) is closed, __sk_destruct()
in net/core/sock.c calls bpf_sk_storage_free(), which unlinks the storage
from the BPF map.

Couldn't a concurrent BPF iterator in __bpf_sk_storage_map_seq_show()
operating under rcu_read_lock() find the storage element before it is
unlinked, and read ctx.sk = sk_storage->owner? The socket is then
synchronously freed by __sk_destruct() without waiting for an RCU grace
period, leaving the iterator to pass a dangling pointer to the BPF program
via bpf_iter_run_prog(prog, &ctx).

This appears to violate the remove-before-reclaim ordering (RCU-001) since
the socket is removed from the RCU-protected BPF map inside the socket's
destruction path.

[Severity: High]
This is a pre-existing issue, but might this only fix a downstream symptom
in the iterator path while leaving other concurrent map operations
vulnerable?

When a socket goes away, bpf_selem_unlink_nofail() in
kernel/bpf/bpf_local_storage.c sets SDATA(selem)->smap to NULL without
holding local_storage->lock.

If a concurrent map deletion calls bpf_selem_unlink(), it acquires
local_storage->lock first, sees selem->snode is still hashed, and calls
bpf_selem_unlink_map(). Could bpf_selem_unlink_map() then unconditionally
dereference the now-NULL SDATA(selem)->smap, crashing inside
select_bucket()?

kernel/bpf/bpf_local_storage.c:bpf_selem_unlink_map() {
    ...
    smap = rcu_dereference_check(SDATA(selem)->smap,
                                 bpf_rcu_lock_held());
    b = select_bucket(smap, local_storage);
    ...
}

Similarly, it looks like bpf_selem_unlink() could use a potentially NULL
local_storage for lock acquisition:

kernel/bpf/bpf_local_storage.c:bpf_selem_unlink() {
    ...
    local_storage = rcu_dereference_check(selem->local_storage,
                                          bpf_rcu_lock_held());
    err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
    ...
}

>  			ctx.value = SDATA(selem)->data;
>  		}
>  		ret = bpf_iter_run_prog(prog, &ctx);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827051859.45511-1-blbllhy@gmail.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-08-27  5:18 [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show Cen Zhang (Microsoft Security FORGE Labs)
  2026-08-27  5:38 ` sashiko-bot
@ 2026-08-27  5:49 ` bot+bpf-ci
  2026-08-27 10:41 ` Amery Hung
  2026-09-11  7:45 ` Xuanqiang Luo
  3 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-08-27  5:49 UTC (permalink / raw)
  To: blbllhy, ast, daniel, martin.lau
  Cc: ameryhung, davem, edumazet, kuba, pabeni, horms, wangfushuai,
	bestswngs, mattbobrowski, kees, menglong8.dong, bpf, netdev,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys,
	blbllhy, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
	clm, ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-08-27  5:38 ` sashiko-bot
@ 2026-08-27 10:33   ` Amery Hung
  0 siblings, 0 replies; 7+ messages in thread
From: Amery Hung @ 2026-08-27 10:33 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Cen Zhang (Microsoft Security FORGE Labs), bpf

On Wed, Aug 26, 2026 at 10:38 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Socket local storage elements are removed from the RCU-protected BPF map inside the socket's destruction path (`__sk_destruct`), violating RCU-001 rules and causing a use-after-free of the socket because the socket can be freed while a BPF iterator is still using it.
> - [High] `bpf_selem_unlink_nofail()` concurrently sets `SDATA(selem)->smap` and `selem->local_storage` to NULL, causing guaranteed NULL pointer dereferences in concurrent `bpf_selem_unlink()` and `bpf_selem_unlink_map()` paths.
> --
>
> commit cabffb2f3787054c5c6ba2a413eaf5dc639ba23d
> Author: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
> bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
>
> This commit addresses a NULL pointer dereference in
> __bpf_sk_storage_map_seq_show() during concurrent socket closure. It adds a
> NULL check after re-reading selem->local_storage to prevent dereferencing a
> cleared pointer.
>
> > diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> > index 1d295a8769fad..7228520aa2b6e 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;
>
> [Severity: High]
> This is a pre-existing issue, but does this patch fully address the
> underlying TOCTOU race?
>
> If a socket without SOCK_RCU_FREE (e.g., AF_UNIX) is closed, __sk_destruct()
> in net/core/sock.c calls bpf_sk_storage_free(), which unlinks the storage
> from the BPF map.

It looks like a valid issue that should be addressed in another patch.
Probably need to defer free in sk_destruct if local storage is used:

#ifdef CONFIG_BPF_SYSCALL
      if (rcu_access_pointer(sk->sk_bpf_storage))
              use_call_rcu = true;
#endif

>
> Couldn't a concurrent BPF iterator in __bpf_sk_storage_map_seq_show()
> operating under rcu_read_lock() find the storage element before it is
> unlinked, and read ctx.sk = sk_storage->owner? The socket is then
> synchronously freed by __sk_destruct() without waiting for an RCU grace
> period, leaving the iterator to pass a dangling pointer to the BPF program
> via bpf_iter_run_prog(prog, &ctx).
>
> This appears to violate the remove-before-reclaim ordering (RCU-001) since
> the socket is removed from the RCU-protected BPF map inside the socket's
> destruction path.
>
> [Severity: High]
> This is a pre-existing issue, but might this only fix a downstream symptom
> in the iterator path while leaving other concurrent map operations
> vulnerable?
>
> When a socket goes away, bpf_selem_unlink_nofail() in
> kernel/bpf/bpf_local_storage.c sets SDATA(selem)->smap to NULL without
> holding local_storage->lock.
>
> If a concurrent map deletion calls bpf_selem_unlink(), it acquires
> local_storage->lock first, sees selem->snode is still hashed, and calls
> bpf_selem_unlink_map(). Could bpf_selem_unlink_map() then unconditionally
> dereference the now-NULL SDATA(selem)->smap, crashing inside
> select_bucket()?

Callers of bpf_selem_unlink_nofail() (i.e., map_free and destroy) do
not race with bpf_selem_unlink() so this should be a false positive.

>
> kernel/bpf/bpf_local_storage.c:bpf_selem_unlink_map() {
>     ...
>     smap = rcu_dereference_check(SDATA(selem)->smap,
>                                  bpf_rcu_lock_held());
>     b = select_bucket(smap, local_storage);
>     ...
> }
>
> Similarly, it looks like bpf_selem_unlink() could use a potentially NULL
> local_storage for lock acquisition:
>
> kernel/bpf/bpf_local_storage.c:bpf_selem_unlink() {
>     ...
>     local_storage = rcu_dereference_check(selem->local_storage,
>                                           bpf_rcu_lock_held());
>     err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
>     ...
> }
>
> >                       ctx.value = SDATA(selem)->data;
> >               }
> >               ret = bpf_iter_run_prog(prog, &ctx);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260827051859.45511-1-blbllhy@gmail.com?part=1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-08-27  5:18 [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show Cen Zhang (Microsoft Security FORGE Labs)
  2026-08-27  5:38 ` sashiko-bot
  2026-08-27  5:49 ` bot+bpf-ci
@ 2026-08-27 10:41 ` Amery Hung
  2026-09-11  7:45 ` Xuanqiang Luo
  3 siblings, 0 replies; 7+ messages in thread
From: Amery Hung @ 2026-08-27 10:41 UTC (permalink / raw)
  To: Cen Zhang (Microsoft Security FORGE Labs)
  Cc: ast, daniel, martin.lau, davem, edumazet, kuba, pabeni, horms,
	wangfushuai, bestswngs, mattbobrowski, kees, menglong8.dong, bpf,
	netdev, linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath,
	kys

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-08-27  5:18 [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show Cen Zhang (Microsoft Security FORGE Labs)
                   ` (2 preceding siblings ...)
  2026-08-27 10:41 ` Amery Hung
@ 2026-09-11  7:45 ` Xuanqiang Luo
  2026-09-11 13:57   ` Cen Zhang (Microsoft Security FORGE Labs)
  3 siblings, 1 reply; 7+ messages in thread
From: Xuanqiang Luo @ 2026-09-11  7:45 UTC (permalink / raw)
  To: Cen Zhang (Microsoft Security FORGE Labs)
  Cc: ameryhung, davem, edumazet, kuba, pabeni, horms, wangfushuai,
	bestswngs, mattbobrowski, kees, menglong8.dong, bpf, netdev,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys, ast,
	daniel, martin.lau

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show
  2026-09-11  7:45 ` Xuanqiang Luo
@ 2026-09-11 13:57   ` Cen Zhang (Microsoft Security FORGE Labs)
  0 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang (Microsoft Security FORGE Labs) @ 2026-09-11 13:57 UTC (permalink / raw)
  To: xuanqiang.luo
  Cc: AutonomousCodeSecurity, ameryhung, ast, bestswngs, bpf, daniel,
	davem, edumazet, horms, kees, kuba, kys, linux-kernel, martin.lau,
	mattbobrowski, menglong8.dong, netdev, pabeni, tgopinath,
	wangfushuai, xmei5

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-11 13:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27  5:18 [PATCH bpf] bpf: Fix NULL pointer dereference in __bpf_sk_storage_map_seq_show Cen Zhang (Microsoft Security FORGE Labs)
2026-08-27  5:38 ` sashiko-bot
2026-08-27 10:33   ` Amery Hung
2026-08-27  5:49 ` bot+bpf-ci
2026-08-27 10:41 ` Amery Hung
2026-09-11  7:45 ` Xuanqiang Luo
2026-09-11 13:57   ` Cen Zhang (Microsoft Security FORGE Labs)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox