Re: [PATCH RFC net-next v4 02/14] xsk: extend xsk_build_skb() to support passing an already allocated skb

sashiko-bot
2026-04-15T08:52:04Z

Sashiko AI review found 1 potential issue(s):
- [High] Inconsistent ownership semantics of `allocated_skb` on error paths lead to a memory leak in the zero-copy path.
--

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

xsk: extend xsk_build_skb() to support passing an already allocated skb

This patch extends xsk_build_skb() to accept an allocated_skb parameter,
allowing the batch feature to reuse an already allocated skb instead of
allocating a new one.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 1194268ae8676..becc8ebfd82f8 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> @@ -793,15 +797,16 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
> return skb;
> }
>
> -static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> - struct xdp_desc *desc)
> +struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> + struct sk_buff *allocated_skb,
> + struct xdp_desc *desc)
> {
> struct net_device *dev = xs->dev;
> struct sk_buff *skb = xs->skb;
> int err;
>
> if (dev->priv_flags & IFF_TX_SKB_NO_LINEAR) {
> - skb = xsk_build_skb_zerocopy(xs, desc);
> + skb = xsk_build_skb_zerocopy(xs, allocated_skb, desc);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> skb = NULL;
> goto free_err;

Does this leak allocated_skb on the zero-copy error path?

If xsk_build_skb_zerocopy() fails on the first descriptor (e.g., if
xsk_skb_metadata() returns an error due to invalid metadata), it returns an
ERR_PTR without freeing the SKB.

Because skb is then explicitly set to NULL before jumping to free_err,
kfree_skb() is skipped.

While the original code also had a leak here for internally allocated SKBs,
this patch extends the issue to caller-provided SKBs. In copy mode,
xsk_build_skb() correctly frees allocated_skb on error, but the zero-copy
path does not.

Could this lead to memory exhaustion if a caller assumes xsk_build_skb()
always consumes or frees the provided SKB on error?

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