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 <[email protected]> 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-9512b0fb9...@toxicpanda.com?part=4
