[lkml]   [2006]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectMemory corruption in 8390.c ? (was Re: Possible leaks in network drivers)
    Ar Mer, 2006-06-21 am 18:28 +0200, ysgrifennodd Eric Sesterhenn:
    > of the driver. Where we call skb=skb_padto(skb, ETH_ZLEN),
    > and dont free the skb later when something goes wrong.

    skb_padto() returns either a new buffer or the existing one depending
    upon the space situation. If it returns a new buffer then it frees the
    old one.

    The sequence is

    ->hard_start_xmit(dev, skb)
    if (0)
    free skb

    Which I think means that the error path for a short packet would double
    free the skb buffer and leak nskb.

    So these drivers should indeed be checking their status before they
    clone the buffer. At the point they do an skb_padto they must not fail
    if the skb_padto succeeds.

    In the case of 8390.c this was broken in 2.6.9 when the efficient 8390
    padding code was replaced by something slower and it turns out broken -
    although nobody realised that last bit until the checker came along.


    Although reverting the change was proposed it was not actually done.

    The following should do the trick:

    Signed-off-by: Alan Cox <>

    --- drivers/net/8390.c~ 2006-06-21 17:41:12.006145536 +0100
    +++ drivers/net/8390.c 2006-06-21 17:41:12.007145384 +0100
    @@ -275,12 +275,14 @@
    struct ei_device *ei_local = (struct ei_device *) netdev_priv(dev);
    int send_length = skb->len, output_page;
    unsigned long flags;
    + char buf[64];
    + char *data = skb->data;

    if (skb->len < ETH_ZLEN) {
    - skb = skb_padto(skb, ETH_ZLEN);
    - if (skb == NULL)
    - return 0;
    + memset(buf, 0, ETH_ZLEN); /* more efficient than doing just the needed bits */
    + memcpy(buf, data, ETH_ZLEN);
    send_length = ETH_ZLEN;
    + data = buf;

    /* Mask interrupts from the ethercard.
    @@ -347,7 +349,7 @@
    * trigger the send later, upon receiving a Tx done interrupt.

    - ei_block_output(dev, send_length, skb->data, output_page);
    + ei_block_output(dev, send_length, data, output_page);

    if (! ei_local->txing)

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-06-21 19:00    [W:0.021 / U:1.428 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site