[PATCH net] xen/netfront: drop RX packets with a short Ethernet header

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

handle_incoming_queue() pulls pull_to bytes into the head before
calling eth_type_trans().  pull_to is the length of the first RX slot,
capped at RX_COPY_THRESHOLD, and that length comes from the backend.
Nothing checks it against ETH_HLEN.

If the first slot is shorter than ETH_HLEN and more slots follow, the
head ends up shorter than an Ethernet header while skb->len is longer,
and eth_type_trans() BUG()s in __skb_pull().  If the whole packet is
shorter than ETH_HLEN, eth_type_trans() reads the header past the end
of the data instead.

Pull at least ETH_HLEN, and drop the packet if that fails, which also
drops packets too short to hold an Ethernet header.  This also checks
the return value of the pull, which was ignored.

Fixes: 0d160211965b ("xen: add virtual network device driver")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
I found this reading the code while going through the callers of
__pskb_pull_tail().  To check it I ran a guest under QEMU's KVM Xen
emulation with QEMU's xen_nic backend changed to split some frames
across several RX slots.  Without this patch, a frame with an 8 byte
first slot followed by a second slot hits the BUG() at
include/linux/skbuff.h in eth_type_trans(), called from xennet_poll().
With it, frames with a 1-13 byte first slot arrive intact, frames
shorter than an Ethernet header are dropped and counted in rx_errors,
and ordinary traffic is unaffected.

There's a net-next series converting the other __pskb_pull_tail()
callers in drivers, including the one in xennet_fill_frags().  It
doesn't touch handle_incoming_queue(), so the two don't conflict.

Thanks,
Josef
---
 drivers/net/xen-netfront.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 2ed673649c48..d269457e839e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1234,8 +1234,16 @@ static int handle_incoming_queue(struct netfront_queue 
*queue,
        while ((skb = __skb_dequeue(rxq)) != NULL) {
                int pull_to = NETFRONT_SKB_CB(skb)->pull_to;
 
-               if (pull_to > skb_headlen(skb))
-                       __pskb_pull_tail(skb, pull_to - skb_headlen(skb));
+               /* pull_to comes from the first slot's length, which the
+                * backend controls.  Make sure the head holds at least an
+                * Ethernet header for eth_type_trans().
+                */
+               if (!pskb_may_pull(skb, max(pull_to, ETH_HLEN))) {
+                       kfree_skb(skb);
+                       packets_dropped++;
+                       queue->info->netdev->stats.rx_errors++;
+                       continue;
+               }
 
                /* Ethernet work: Delayed to here as it peeks the header. */
                skb->protocol = eth_type_trans(skb, queue->info->netdev);

---
base-commit: d5a007b9b457c915ab1a53227e8939e4018aa97a
change-id: 20261006-b4-xen-netfront-short-head-4589eed26166