Re: [PATCH net v5 5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In `xsk_build_skb_zerocopy()`, if `xsk_skb_metadata()` fails, the newly allocated `skb` is leaked.
--

commit 88b75ff1d12b482a55e1724cc457909cf3ea19dd
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()

Postpone the skb initialization phase in xsk_build_skb() until the first
frag allocation is completed to prevent the skb destructor from running
on error paths. Before the initialization, the skb can be safely freed
by kfree_skb().

Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 55378c3855d5..af3c5752bb63 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -819,8 +819,6 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
>  			return ERR_PTR(err);
>  
>  		skb_reserve(skb, hr);
> -
> -		xsk_skb_init_misc(skb, xs, desc->addr);
>  		if (desc->options & XDP_TX_METADATA) {
>  			err = xsk_skb_metadata(skb, buffer, desc, pool, hr);
>  			if (unlikely(err))

Could this leak the newly allocated skb if xsk_skb_metadata() fails?

If xsk_skb_metadata() returns an error, xsk_build_skb_zerocopy() will return
ERR_PTR(err) directly without freeing the skb.

The caller receives the error pointer, sets its local skb variable to NULL,
and jumps to the free_err label:

net/xdp/xsk.c:xsk_build_skb() {
    ...
	if (dev->priv_flags & IFF_TX_SKB_NO_LINEAR) {
		skb = xsk_build_skb_zerocopy(xs, desc);
		if (IS_ERR(skb)) {
			err = PTR_ERR(skb);
			skb = NULL;
			goto free_err;
		}
    ...
}

Because the skb is explicitly nullified here, the kfree_skb(skb) call in
the cleanup path is bypassed, leaving the socket buffer and its associated
sk_wmem_alloc charge leaked.

While this pre-existing issue is explicitly fixed later in this patch series
in commit "xsk: avoid skb leak in XDP_TX_METADATA case", should it be
addressed earlier to ensure intermediate states remain safe during a bisect?

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