All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.