Re: [PATCH net-next 4/9] xen/netfront: check for failure when pulling in xennet_fill_frags()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: xen-netfront: memory leak of head SKB on xennet_fill_frags() error path
--

commit 35a098cdf4bebe9adc4115a04a7fd029b02400ea
Author: Josef Bacik <josef@toxicpanda.com>

xen/netfront: check for failure when pulling in xennet_fill_frags()

When the skb already has MAX_SKB_FRAGS frags, xennet_fill_frags() pulls
the start of the packet into the head to try to free up a frag slot. This
commit explicitly uses pskb_may_pull() and takes the error path if it
fails.

> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 2ed673649c488..6e01a883cc64a 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
[ ... ]
> @@ -1198,6 +1191,11 @@ static int xennet_fill_frags(struct netfront_queue *queue,
>  	xennet_set_rx_rsp_cons(queue, cons);
>  
>  	return 0;
> +
> +err:
> +	xennet_set_rx_rsp_cons(queue, ++cons + skb_queue_len(list));
> +	kfree_skb(nskb);
> +	return -ENOENT;
>  }

[Severity: High]
Does this code leak the head skb on the error path?

When xennet_fill_frags() returns -ENOENT on failure, the error path handles
cleanup for the individual fragment (nskb) but does not free or requeue the
main head skb.

Back in xennet_poll(), upon failure, it jumps to the err label without
placing the head skb back into tmpq, unlike the xennet_set_skb_gso() failure
path just above it:

drivers/net/xen-netfront.c:xennet_poll() {
    ...
		skb = __skb_dequeue(&tmpq);
    ...
		if (unlikely(xennet_fill_frags(queue, skb, &tmpq)))
			goto err;
    ...
err:
			while ((skb = __skb_dequeue(&tmpq)))
				__skb_queue_tail(&errq, skb);
    ...
}

This loop overwrites the skb pointer, causing the head skb to be permanently
lost.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-b4-pskb-pull-tail-drivers-v1-0-9512b0fb977b@toxicpanda.com?part=4