Re: [PATCH RFC net-next v4 03/14] xsk: add xsk_alloc_batch_skb() to build skbs in batch

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Pre-allocated SKB and socket memory charge (`sk_wmem_alloc`) are permanently leaked when zerocopy metadata processing fails.
- [High] Double-free and Use-After-Free of `xs->skb` when fragment allocation fails with `-EAGAIN` in copy mode.
--

commit 3ef2c03b6204759327ab337fe34617d9dd8e04ef
Author: Jason Xing <kernelxing@tencent.com>

xsk: add xsk_alloc_batch_skb() to build skbs in batch

This commit introduces xsk_alloc_batch_skb() to support allocating and
building skbs in batch. It reserves skbs, batches the sk_wmem_alloc charge,
and then copies data to complete initialization.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c

[ ... ]

> +	/* Phase 3: Build SKBs with packet data */
> +	for (j = 0; j < alloc_descs; j++) {
> +		if (!xs->skb) {
> +			skb = skbs[skb_count - 1 - k];
> +			k++;
> +		}
> +
> +		skb = xsk_build_skb(xs, skb, &descs[j]);
> +		if (IS_ERR(skb)) {
> +			*err = PTR_ERR(skb);
> +			break;
> +		}

Can this lead to a double-free and use-after-free of xs->skb if a fragment
allocation fails?

If xsk_build_skb() fails to allocate a page for a subsequent fragment in copy
mode, it frees the partially built skb and returns -EAGAIN. However, it
leaves xs->skb pointing to this freed memory.

When this loop breaks and returns -EAGAIN up the call chain, the caller
__xsk_generic_xmit_batch() checks xs->skb and calls xsk_drop_skb(xs->skb),
which would double-free the skb. Furthermore, any subsequent transmissions
could dereference the dangling pointer. Should xs->skb be explicitly cleared
when handling this error?

> +
> +		if (xp_mb_desc(&descs[j])) {
> +			xs->skb = skb;
> +			continue;
> +		}
> +
> +		xs->skb = NULL;
> +		__skb_queue_tail(&batch->send_queue, skb);
> +	}
> +
> +	/* Phase 4: Reclaim unused allocated SKBs */
> +	while (k < i)
> +		kfree_skb(skbs[skb_count - 1 - k++]);

Does this permanently leak the pre-allocated skb and its socket memory charge
if xsk_build_skb() fails?

When xsk_build_skb() encounters an error like a zerocopy metadata processing
failure, it returns an ERR_PTR() without freeing the skb. Because k was
already incremented before the xsk_build_skb() call, the Phase 4 loop starts
reclaiming at the next skb.

The failed skb at index k - 1 is bypassed. Since it is never freed,
sock_wfree is skipped, leaking both the skb and the batched sk_wmem_alloc
charge.

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