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

[sashiko-bot@kernel.org]

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