net: tulip: xircom_cb: drop runt frames before skb copy
Subsystems (3)
PATCH 1/1 net: tulip: xircom_cb: drop runt frames before skb copy
investigate_read_descriptor() takes the 11-bit packet length straight from the device-written descriptor and subtracts 4 for the CRC. A length below 4 underflows pkt_len to a negative value, which is then passed to skb_put_data() as a huge unsigned length and corrupts memory past the end of the freshly allocated skb.
Drop the frame as a length error if pkt_len is negative after CRC removal.
--- drivers/net/ethernet/dec/tulip/xircom_cb.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/net/ethernet/dec/tulip/xircom_cb.c +5 -0
@@ -1110,6 +1110,11 @@ investigate_read_descriptor(struct net_device *dev, struct xircom_private *card,/* minus 4, we don't want the CRC */ struct sk_buff *skb; +if (pkt_len <0 ) { + dev->stats.rx_length_errors++; + goto out; + } +if (pkt_len >1518 ) {netdev_err (dev,"Packet length %i is bogus\n" , pkt_len); pkt_len =1518 ;
Signature · 1 line
2.55.0
Quoted 5 lines · On Fri, 31 Jul 2026 20:33:34 +0200 Pablo Vallespín Aranguren…
investigate_read_descriptor()takes the 11-bit packet length straight from the device-written descriptor and subtracts 4 for the CRC. A length below 4 underflowspkt_lento a negative value, which is then passed toskb_put_data()as a huge unsigned length and corrupts memory past the end of the freshly allocated skb.
Have you checked that that hardware can actually set a short value? I'd expect that packets shorter than 64 bytes (including the crc) are dropped as 'runts' and probably don't even use a ring entry.
Of course, if you think the device might lie all bets are off. (Without an iommu is can write anywhere in host memory...)
David
Quoted 26 lines
Drop the frame as a length error if
pkt_lenis negative after CRC removal.Signed-off-by Pablo Vallespín Aranguren <[email protected]>Assisted-by gkh_clanker_t1000--- drivers/net/ethernet/dec/tulip/xircom_cb.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/net/ethernet/dec/tulip/xircom_cb.c +5
-0@@ -1110,6 +1110,11 @@ investigate_read_descriptor(struct net_device *dev, struct xircom_private *card,/* minus 4, we don't want the CRC */ struct sk_buff *skb; +if (pkt_len <0 ) { + dev->stats.rx_length_errors++; + goto out; + } +if (pkt_len >1518 ) {netdev_err (dev,"Packet length %i is bogus\n" , pkt_len); pkt_len =1518 ;
Quoted 6 lines · David Laight
On Fri, 31 Jul 2026 20:33:34 +0200
Pablo Vallespín Aranguren <[email protected]> wrote:Have you checked that that hardware can actually set a short value? I'd expect that packets shorter than 64 bytes (including the crc) are dropped as 'runts' and probably don't even use a ring entry.
I haven't verified how the real Xircom firmware handles runt frames (I don't own this hardware). I tested with Qemu's emulated NIC, the device itself does validate and reject negative-size values before sending them to the driver.
Quoted 2 lines
Of course, if you think the device might lie all bets are off. (Without an iommu is can write anywhere in host memory...)
I took into account that the hardware could glitch or a misbehaving/fake card could be used. I modified the emulated NIC to mimic this behaviour, and given that the driver does not perform a check on its own (to validate that pkt_len is not negative) it does lead to a kernel panic.
Best,
Pablo
Quoted 23 lines
Quoted 22 lines
Signed-off-by Pablo Vallespín Aranguren <[email protected]>Assisted-by gkh_clanker_t1000--- drivers/net/ethernet/dec/tulip/xircom_cb.c | 5 +++++ 1 file changed, 5 insertions(+)drivers/net/ethernet/dec/tulip/xircom_cb.c +5
-0@@ -1110,6 +1110,11 @@ investigate_read_descriptor(struct net_device *dev, struct xircom_private *card,/* minus 4, we don't want the CRC */ struct sk_buff *skb; +if (pkt_len <0 ) { + dev->stats.rx_length_errors++; + goto out; + } +if (pkt_len >1518 ) {netdev_err (dev,"Packet length %i is bogus\n" , pkt_len); pkt_len =1518 ;
Jakub Kicinski
maintainerQuoted 8 lines · On Fri, 31 Jul 2026 22:56:02 +0200 Pablo Vallespín Aranguren wrote:
Quoted 3 lines
Have you checked that that hardware can actually set a short value? I'd expect that packets shorter than 64 bytes (including the crc) are dropped as 'runts' and probably don't even use a ring entry.
I haven't verified how the real Xircom firmware handles runt frames (I don't own this hardware). I tested with Qemu's emulated NIC, the device itself does validate and reject negative-size values before sending them to the driver.
Are you related to Greg KH in any way? You use the same slop tag. Just curious.
Please send a patch to delete this driver if you want to help. It's ancient.
Quoted 5 lines · Jakub Kicinski
Quoted 4 lines · On Fri, 31 Jul 2026 22:56:02 +0200 Pablo Vallespín Aranguren wrote:
Quoted 3 lines
Have you checked that that hardware can actually set a short value? I'd expect that packets shorter than 64 bytes (including the crc) are dropped as 'runts' and probably don't even use a ring entry.
Quoted 7 lines
Quoted 4 lines
I haven't verified how the real Xircom firmware handles runt frames (I don't own this hardware). I tested with Qemu's emulated NIC, the device itself does validate and reject negative-size values before sending them to the driver.
Are you related to Greg KH in any way? You use the same slop tag. Just curious.
Pablo is part of an CS masters intern project through vu.nl that I'm running to help audit and submit LLM-found issues against the kernel. Hence the gkh_clanker tag, as my bot is the thing that originally "found" this report.
Quoted 2 lines
Please send a patch to delete this driver if you want to help. It's ancient.
No objection from me, that fixes all of the issues!
Pablo, want to submit a patch for that?
thanks,
greg k-h
Quoted 9 lines · Greg KH
Quoted 4 lines · Jakub Kicinski
On Fri, 31 Jul 2026 22:56:02 +0200 Pablo Vallespín Aranguren wrote:
Please send a patch to delete this driver if you want to help. It's ancient.
No objection from me, that fixes all of the issues!
Pablo, want to submit a patch for that?
Sounds good, I will submit a patch to delete it.
Best,
Pablo