Messages in this thread Patches in this message | | | Subject | Ethernet frame padding bug in several drivers - patch(es) included | From | Jesper Juhl <> | Date | 10 Jan 2003 17:31:32 +0100 |
| |
Hi,
I send this mail once before (about 16 hours ago), but it seems it never reached the list, so I'm resending it. If you recieve it twice I appologize.
---[ original mail below ]---
Hi,
First of all, please CC me on replies as I'm not subscribed to the list.
I just read a paper by Ofir Arkin and Josh Anderson from @stake (http://www.atstake.com/research/advisories/2003/atstake_etherleak_report.pdf), that documents a bug related to incorrect padding of ethernet frames in many Linux (and other OS) device drivers.
Briefly, the problem is that when a frame is less than the minimum required size, the frame *should* be padded with 0 bytes, but is instead padded with kernel memory. This happens since the drivers don't take care to zero the padding of the buffer before transmitting it. I won't repeat the paper here as it does quite a good job explaining the issue.
After reading the paper I took a look at the source for my 2.4.20 kernel, and saw that the problem was indeed present in many drivers. I then desided that I might as well fix it as anyone else and started on the task.
So far I've only patched 3 drivers (randomly selected), and I would like to get some feedback on the patches before I do the rest of them. I would like to make sure the fix is correct before spending time implementing a flawed fix in all the affected drivers. The fix is more or less straight from the paper from @stake, so those people should get all the credit. I'm just doing the legwork of actually fixing up the driver files (patches are against vanilla 2.4.20).
If I get positive feedback on these 3 small patches, then I'll go ahead and do the rest of the drivers tomorrow and submit the new patches.
Here are the patches:
patch for drivers/net/3c507.c
--- 3c507.c.orig 2003-01-10 01:51:13.000000000 +0100 +++ 3c507.c 2003-01-10 02:05:30.000000000 +0100 @@ -494,9 +494,16 @@ struct net_local *lp = (struct net_local *) dev->priv; int ioaddr = dev->base_addr; unsigned long flags; - short length = ETH_ZLEN < skb->len ? skb->len : ETH_ZLEN; + short length; unsigned char *buf = skb->data;
+ if (skb->len < ETH_ZLEN) { + length = ETH_ZLEN; + memset(buf + skb->len, 0, length - skb->len); + } else { + length = skb->len; + } + netif_stop_queue (dev);
spin_lock_irqsave (&lp->lock, flags);
patch for drivers/net/atp.c
--- atp.c.orig 2003-01-10 01:51:13.000000000 +0100 +++ atp.c 2003-01-10 02:03:35.000000000 +0100 @@ -553,7 +553,12 @@ int length; long flags;
- length = ETH_ZLEN < skb->len ? skb->len : ETH_ZLEN; + if (skb->len < ETH_ZLEN) { + length = ETH_ZLEN; + memset(&skb->data + skb->len, 0, length - skb->len); + } else { + length = skb->len; + }
netif_stop_queue(dev);
patch for drivers/net/eepro.c
--- eepro.c.orig 2003-01-10 01:51:13.000000000 +0100 +++ eepro.c 2003-01-10 02:17:00.000000000 +0100 @@ -1138,9 +1138,16 @@ spin_lock_irqsave(&lp->lock, flags);
{ - short length = ETH_ZLEN < skb->len ? skb->len : ETH_ZLEN; + short length; unsigned char *buf = skb->data;
+ if (skb->len < ETH_ZLEN) { + length = ETH_ZLEN; + memset(buf + skb->len, 0, length - skb->len); + } else { + length = skb->len; + } + if (hardware_send_packet(dev, buf, length)) /* we won't wake queue here because we're out of space */ lp->stats.tx_dropped++;
Best regards,
Jesper Juhl <jju@dif.dk>
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |