Re: [PATCH RFC net-next v4 14/14] xsk: optimize xsk_build_skb for batch copy-mode fast path

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] An SKB memory leak occurs when xsk_build_skb() returns an error for a newly assigned SKB.
- [Medium] The prefetch instruction uses the raw AF_XDP descriptor address without extracting the offset, creating an invalid pointer in unaligned mode.
--

commit 7bdeef673e571c57c404cb8d05446f46b3e2676e
Author: Jason Xing <kernelxing@tencent.com>

xsk: optimize xsk_build_skb for batch copy-mode fast path

This commit introduces three targeted optimizations for the batch
copy-mode TX hot path in XDP sockets, including replacing skb_store_bits()
with memcpy(), inlining UMEM address computation, and prefetching the
next descriptor's UMEM data.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 3b3969adfd23f..0fccb0c8cbe90 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -751,14 +751,28 @@ int xsk_alloc_batch_skb(struct xdp_sock *xs, u32 nb_pkts, u32 nb_descs, int *err
>  	if (total_truesize)
>  		refcount_add(total_truesize, &xs->sk.sk_wmem_alloc);
>  
> -	/* Phase 3: Build SKBs with packet data */
> +	/* Phase 3: Build SKBs with packet data. */
> +	struct xsk_buff_pool *pool = xs->pool;
> +	void *pool_addrs = pool->addrs;
> +	bool unaligned = pool->unaligned;
> +
>  	for (j = 0; j < alloc_descs; j++) {
> +		u64 addr = descs[j].addr;
> +		void *buffer;
> +
> +		if (unaligned)
> +			addr = xp_unaligned_add_offset_to_addr(addr);
> +		buffer = pool_addrs + addr;
> +
> +		if (j + 1 < alloc_descs)
> +			prefetch(pool_addrs + descs[j + 1].addr);

Does this prefetch use the raw AF_XDP descriptor address without extracting
the offset?

In unaligned mode, an AF_XDP descriptor addr field encodes a chunk offset
in its upper 16 bits. While xp_unaligned_add_offset_to_addr() is used to
decode the current descriptor address, the prefetch directly adds
descs[j + 1].addr to pool_addrs.

Could this result in an out-of-bounds pointer in unaligned mode, failing to
hide memory latency and causing cache pollution?

> +
>  		if (!xs->skb) {
>  			skb = skbs[skb_count - 1 - k];
>  			k++;
>  		}
>  
> -		skb = xsk_build_skb(xs, skb, &descs[j]);
> +		skb = xsk_build_skb(xs, skb, &descs[j], buffer);
>  		if (IS_ERR(skb)) {
>  			*err = PTR_ERR(skb);
>  			break;

Can this leak the newly assigned SKB if xsk_build_skb() returns an error?

If xsk_build_skb() fails, it returns ERR_PTR(err) and the loop breaks. Since
k was already incremented, the cleanup loop in Phase 4 (while (k < i)
kfree_skb(...)) reclaims unused SKBs starting from the current k, skipping
the failed SKB at index k - 1.

Could a malicious userspace program exploit this to systematically leak SKBs
by triggering metadata errors?

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