Ethernet frame padding bug in several drivers - patch(es) included

Ethernet frame padding bug in several drivers - patch(es) included

Post by Jesper Juh » Sat, 11 Jan 2003 18:55:07



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.


(http://www.atstake.com/research/advisories/2003/atstake_etherleak_rep...),
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.

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

        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

        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

        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,


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in

More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/