net/ethernet/eth.c | 6 ++++++ 1 file changed, 6 insertions(+)
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 <zhangfeionline@gmail.com>
---
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.0On Fri, Aug 7, 2026 at 12:28 PM Pengfei Zhang <zhangfeionline@gmail.com> wrote:
>
> 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 <zhangfeionline@gmail.com>
> ---
> 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);
>
This is obviously a wrong patch. Please fix faulty drivers instead of
adding more code in our fast path.
If you use header split, you are supposed to use modern NAPI api, like
napi_gro_frags() which will take care of that.
pw-bot: cr
On Fri, Aug 7, 2026 at 6:52 PM Eric Dumazet <edumazet@google.com> wrote: > This is obviously a wrong patch. Please fix faulty drivers instead of > adding more code in our fast path. > > If you use header split, you are supposed to use modern NAPI api, like > napi_gro_frags() which will take care of that. Thanks for the review. I put the test in eth_type_trans() because I thought the stack should not be brought down by a driver handing it a malformed skb. I take your point about the cost on the fast path, and that napi_gro_frags() already takes care of this. I will drop the patch. I do not have amd-xgbe or dwc-xlgmac hardware, so I will not send an untested driver change; if I can get access to a device that behaves this way, I will fix and verify the driver there. Thanks, Pengfei
On Mon, Aug 10, 2026 at 9:59 AM Pengfei Zhang <zhangfeionline@gmail.com> wrote: > > On Fri, Aug 7, 2026 at 6:52 PM Eric Dumazet <edumazet@google.com> wrote: > > This is obviously a wrong patch. Please fix faulty drivers instead of > > adding more code in our fast path. > > > > If you use header split, you are supposed to use modern NAPI api, like > > napi_gro_frags() which will take care of that. > > Thanks for the review. > > I put the test in eth_type_trans() because I thought the stack should not > be brought down by a driver handing it a malformed skb. I take your point > about the cost on the fast path, and that napi_gro_frags() already takes > care of this. > > I will drop the patch. I do not have amd-xgbe or dwc-xlgmac hardware, so > I will not send an untested driver change; if I can get access to a device > that behaves this way, I will fix and verify the driver there. This function is primarily used by Ethernet drivers. An Ethernet frame must at least contain an Ethernet header. A frame smaller than that should be dropped early by the NIC. If a NIC and/or driver provides the upper stack a packet without an Ethernet header, the bug is on them. Let's not add checks in our fast path for a situation that should never happen.
© 2016 - 2026 Red Hat, Inc.