public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jason Xing" <kerneljasonxing@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH net v2 3/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
Date: Tue, 21 Apr 2026 09:40:23 +0000	[thread overview]
Message-ID: <20260421094024.5E8EBC2BCB0@smtp.kernel.org> (raw)
In-Reply-To: <20260420082805.14844-4-kerneljasonxing@gmail.com>

Sashiko AI review found 1 potential issue(s):
- [Critical] Dangling `xs->skb` pointer on `-EOVERFLOW` error path leading to Use-After-Free.
--

commit 4ce0cecf5e8e18bd369c02242f70a42cc8fbdf03
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path

This commit aims to fix a use-after-free in the xsk_build_skb() error
handling path by ensuring we only free newly allocated skbs when a
continuation descriptor fails. It prevents freeing an in-progress
multi-buffer skb by adding a check for !xs->skb before kfree_skb(skb).

Link: https://lore.kernel.org/all/20260415082654.21026-4-kerneljasonxing@gmail.com/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -969,7 +969,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
[ ... ]
>  free_err:
> -	if (skb && !skb_shinfo(skb)->nr_frags)
> +	if (skb && !xs->skb && !skb_shinfo(skb)->nr_frags)
>  		kfree_skb(skb);
>  
>  	if (err == -EOVERFLOW) {

Does this patch leave a use-after-free on the -EOVERFLOW error path?

When xsk_build_skb() handles a continuation descriptor and encounters an
-EOVERFLOW error, it jumps to free_err and enters this block. Looking at the
subsequent code in xsk_build_skb():

	if (err == -EOVERFLOW) {
		if (xs->skb) {
			/* Drop the packet */
			xsk_inc_num_desc(xs->skb);
			xsk_drop_skb(xs->skb);
		} else {

The xsk_drop_skb(xs->skb) call consumes and frees the socket buffer, but
xs->skb is never set to NULL.

Because xsk_build_skb() returns ERR_PTR(-EOVERFLOW), the caller
__xsk_generic_xmit() intercepts this error and continues its transmission
loop without aborting.

On the next iteration, __xsk_generic_xmit() would pass the dangling xs->skb
pointer back into xsk_build_skb(), which will dereference the freed memory
when checking skb_shinfo(skb)->nr_frags.

Should xs->skb be cleared to NULL after calling xsk_drop_skb() here?

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

  parent reply	other threads:[~2026-04-21  9:40 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-20  8:27 [PATCH net v2 0/8] xsk: fix bugs around xsk skb allocation Jason Xing
2026-04-20  8:27 ` [PATCH net v2 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices Jason Xing
2026-04-20 19:34   ` Stanislav Fomichev
2026-04-20 23:51     ` Jason Xing
2026-04-21 22:20       ` Stanislav Fomichev
2026-04-21  9:40   ` sashiko-bot
2026-04-21 12:39     ` Jason Xing
2026-04-20  8:27 ` [PATCH net v2 2/8] xsk: handle NULL dereference of the skb without frags issue Jason Xing
2026-04-20 19:34   ` Stanislav Fomichev
2026-04-21  9:40   ` sashiko-bot
2026-04-21 12:46     ` Jason Xing
2026-04-20  8:28 ` [PATCH net v2 3/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path Jason Xing
2026-04-20 19:34   ` Stanislav Fomichev
2026-04-21  0:01     ` Jason Xing
2026-04-21  9:40   ` sashiko-bot [this message]
2026-04-21 12:51     ` Jason Xing
2026-04-20  8:28 ` [PATCH net v2 4/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb() Jason Xing
2026-04-20 19:34   ` Stanislav Fomichev
2026-04-21  0:51     ` Jason Xing
2026-04-20  8:28 ` [PATCH net v2 5/8] xsk: avoid skb leak in XDP_TX_METADATA case Jason Xing
2026-04-21  9:40   ` sashiko-bot
2026-04-21 12:58     ` Jason Xing
2026-04-20  8:28 ` [PATCH net v2 6/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS Jason Xing
2026-04-20  8:28 ` [PATCH net v2 7/8] xsk: fix xsk_addrs slab leak on multi-buffer error path Jason Xing
2026-04-20 19:58   ` Stanislav Fomichev
2026-04-20  8:28 ` [PATCH net v2 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures Jason Xing
2026-04-20 19:49   ` Stanislav Fomichev
2026-04-21  0:49     ` Jason Xing
2026-04-21 22:23       ` Stanislav Fomichev
2026-04-22  2:54         ` Jason Xing
2026-04-21  9:40   ` sashiko-bot
2026-04-21 13:01     ` Jason Xing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260421094024.5E8EBC2BCB0@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=kerneljasonxing@gmail.com \
    --cc=sashiko@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox