[lkml]   [1998]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectFatal bug in Boomerang driver found
    The boomerang driver (3c59x.c) has two fatal bugs in
    boomerang_rx() that will cause a kernel panic.

    Bug #1 will affect *all* machines once the 'signed int'
    counter vp->cur_rx goes greater than MAX_INT. At this
    point vp->cur_rx becomes a negative number and the for()
    loop used to fill the ring buffer always fails. The skbs
    in the ring buffer will never be allocated (all will be
    set to NULL). The driver does not test for a NULL skb
    and calls skb_put with a NULL value, generating a kernel
    panic. The solution is to test for the wrap and adjust
    both vp->cur_rx and vp->dirty_rx accordingly.

    Bug #2 can affect any machine, but machines that have
    plenty of RAM are less likely to be affected. If the
    driver fails to replenish one of the skbs in the ring
    buffers (because DEV_ALLOC_SKB returned NULL) then the
    buffer will point to NULL (or 0x00). The driver does
    not test for a NULL skb and calls skb_put with a NULL
    value, generating a kernel panic. I beleive the correct
    solution here is to break from the read loop and try to
    replenish the ring buffer. There may be other issues
    involved here that I am not aware of, but Donald will
    need to check this out.

    There is one other issue that I have with the driver.
    In boomerang_rx(), rx_work_limit is set and is not
    checked until *after* it has tried to put at least
    one packet into an skb. However, it is possible that
    the we do not have any skbs ready in the ring buffer
    (as described above). It would be better to check this
    variable *before* trying to fill the NULL skb. In fact,
    now that I think about it, that is probably the best
    way to deal with Bug #2.

    The enclosed patch is against .99E and should fix both
    #1 and #2 (with the caveat noted above), but does not
    deal with the last issue. Please test and report results.

    (Please note that I follow the above lists through the
    various archives, rather that subscribing directly.
    Should you have comments for me, please cc: me directly.)


    Rob Riggs Devil's Thumb Entertainment
    Network Administrator Boulder, CO - (303) 938-1200
    rob@DevilsThumb.COM http://www.DevilsThumb.COM/~rob
    "The notion of errors is ill-defined." - IRIX 'netstat' man page--- 3c59x.c.orig Mon Jul 6 12:16:25 1998
    +++ 3c59x.c Mon Jul 6 17:31:17 1998
    @@ -1801,6 +1801,11 @@
    void *temp;
    /* Pass up the skbuff already on the Rx ring. */
    skb = vp->rx_skbuff[entry];
    + if (skb == NULL) {
    + printk(KERN_ERR "boomerang: Tried to fill a NULL skb!\n");
    + break;
    + }
    vp->rx_skbuff[entry] = NULL;
    #if LINUX_VERSION_CODE >= 0x10300
    temp = skb_put(skb, pkt_len);
    @@ -1833,6 +1838,11 @@
    entry = (++vp->cur_rx) % RX_RING_SIZE;
    + /* Wrap our counters -- otherwise the next for() loop fails */
    + if (vp->cur_rx < 0) {
    + vp->cur_rx = 0;
    + vp->dirty_rx -= INT_MAX;
    + }
    if (--rx_work_limit < 0)
    @@ -1842,8 +1852,10 @@
    entry = vp->dirty_rx % RX_RING_SIZE;
    if (vp->rx_skbuff[entry] == NULL) {
    - if (skb == NULL)
    + if (skb == NULL) {
    + printk(KERN_ERR "boomerang: Could not allocate skb on ring buffer!\n");
    break; /* Bad news! */
    + }
    skb->dev = dev; /* Mark as being used by this device. */
    #if LINUX_VERSION_CODE > 0x10300
    skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
     \ /
      Last update: 2005-03-22 13:43    [W:0.021 / U:8.068 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site