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

sashiko-bot
2026-04-15T09:47:21Z

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