eth_type_trans() pulls ETH_HLEN unconditionally, but never checks that the skb has that many bytes in its linear part. Only skb->len is checked, by skb_pull_inline(), while the __skb_pull() it calls requires len <= skb_headlen(skb). The two are the same test for a linear skb and differ for a paged one. Drivers using hardware header split, such as amd-xgbe and dwc-xlgmac, can end up with a paged skb whose linear part is shorter than the header: on certain frames the controller reports a header length below ETH_HLEN. With 2, that gives len=56, headlen=2 and data_len=54. Nothing is wrong with that skb, but eth_type_trans() on it hits the BUG() in __skb_pull() from softirq context, so one received packet panics the machine. Drop the frame instead. Test skb_headlen() rather than skb->len, since that is what __skb_pull() requires. Returning 0 leaves skb->protocol unset, so the frame is counted in rx_dropped and freed with SKB_DROP_REASON_UNHANDLED_PROTO rather than discarded silently. Signed-off-by: Pengfei Zhang --- Notes for reviewers, not for the changelog: We ran into this on a platform built on the Synopsys DesignWare Core IP, where a single received frame is enough to panic the machine. Nothing was corrupted -- the skb was intact and every invariant held. What is missing is a check that the header being pulled is actually present. The length in question is reported by the MAC itself: with header split enabled it writes into the receive descriptor how far into the frame it cut the header. A MAC reporting less than ETH_HLEN there is misbehaving, and that much is the hardware's problem. But eth_type_trans() requires ETH_HLEN linear bytes and tests only skb->len, so the short length reaches the BUG() in softirq context, on a path fed by received traffic. The same failure mode was CVE-2024-41091 when it was reachable through tun_xdp_one(), and was fixed there by dropping the frame, in 049584807f1d ("tun: add missing verification for short frame"). amd-xgbe shows the same shape in-tree: /* On some frames the MAC reports a header length below ETH_HLEN in the * receive descriptor. The driver takes that value as-is; nothing bounds * it from below. */ xgbe-dev.c:1901 rdata->rx.hdr_len = XGMAC_GET_BITS_LE(rdesc->desc2, RX_NORMAL_DESC2, HL); /* The length is passed down unchanged and used to fill the skb, through * the ordinary core helpers. The numbers below are one instance of it, * hdr_len = 2 on a 56-byte frame: */ xgbe-drv.c:2355 skb = xgbe_create_skb(pdata, napi, rdata, buf1_len); napi_alloc_skb(napi, rdata->rx.hdr.dma_len) skb_copy_to_linear_data(skb, packet, len) /* len = 2 */ skb_put(skb, len) /* headlen = 2 */ xgbe-drv.c:2370 skb_add_rx_frag(...) /* the other 54 bytes, as a frag */ /* The skb is well formed here -- 56 == 2 + 54 -- and eth_type_trans() * pulls the MAC header without testing that it is in the linear part. * The patch adds that test in eth_type_trans(), just before the pull * marked below. Without it the pull takes the machine down: */ xgbe-drv.c:2435 skb->protocol = eth_type_trans(skb, netdev); eth_skb_pull_mac(skb) <- eth.c:164, unguarded skb_pull_inline(skb, ETH_HLEN) /* 14 > 56, false */ __skb_pull(skb, ETH_HLEN) skb->len -= len; /* 56 - 14 = 42 */ if (skb->len < skb->data_len) /* 42 < 54 */ BUG(); /* fatal in softirq */ Every step there uses the standard core APIs, and there is no skb memory corruption anywhere along the way: the BUG() fires on arithmetic that __skb_pull() just did itself. A driver doing nothing unusual walks past the one test there is and into it. A received packet should not be able to make the stack panic on purpose, so the check belongs where the requirement is -- the stack has everything it needs to reject the frame itself, and should not have to rely on the hardware or the driver reporting a sane length. The BUG() in __skb_pull() is deliberately left alone. It is the backstop for the many skb_pull() call sites that discard the return value, and for real corruption it should stay a panic. net/ethernet/eth.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index d9faadbe9..84914709a 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -161,6 +161,12 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev) skb->dev = dev; skb_reset_mac_header(skb); + if (unlikely(skb_headlen(skb) < ETH_HLEN)) { + net_warn_ratelimited("%s: dropping frame with a short linear part from %s\n", + __func__, dev->name); + return 0; + } + eth = eth_skb_pull_mac(skb); eth_skb_pkt_type(skb, dev); base-commit: 4fa4977a0d900f936bcae5cd2c510be5554e8dd6 -- 2.54.0