Re: [PATCH net v5 3/8] xsk: handle NULL dereference of the skb without frags issue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Completion Queue (CQ) state corruption when handling -EOVERFLOW for the first descriptor.
--

commit 819a57cd3753db9f503ac093f83dccc180da574d
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: handle NULL dereference of the skb without frags issue

This commit fixes a NULL pointer dereference when a first descriptor
triggers -EOVERFLOW in xsk_build_skb_zerocopy() by adding a check for
xs->skb. It handles the first-descriptor case by manually canceling the CQ
slot and incrementing invalid_descs.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -976,9 +976,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
>  		kfree_skb(skb);
>  
>  	if (err == -EOVERFLOW) {
> -		/* Drop the packet */
> -		xsk_inc_num_desc(xs->skb);
> -		xsk_drop_skb(xs->skb);
> +		if (xs->skb) {
> +			/* Drop the packet */
> +			xsk_inc_num_desc(xs->skb);
> +			xsk_drop_skb(xs->skb);
> +		} else {
> +			xsk_cq_cancel_locked(xs->pool, 1);

Does this cause a Completion Queue (CQ) state corruption?

When a first descriptor triggers -EOVERFLOW in xsk_build_skb_zerocopy(), the
free_err path first calls kfree_skb(skb) just above this code. Because
xsk_skb_init_misc() was called earlier, the SKB's destructor is set to
xsk_destruct_skb(). The kfree_skb() call invokes this destructor, which
submits a completion and increments the ring's producer index.

If we then call xsk_cq_cancel_locked() here, it decrements the reservation
index while the producer index was already incremented, breaking the core
invariant cached_prod >= ring->producer. 

This causes space calculations to underflow and can overwrite unconsumed
entries in the CQ.

I see this was addressed later in the series by commit 88b75ff1d12b ("xsk:
prevent CQ desync when freeing half-built skbs in xsk_build_skb()").

> +			xs->tx->invalid_descs++;
> +		}
>  		xskq_cons_release(xs->tx);
>  	} else {
>  		/* Let application retry */

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