diff mbox

lmc: Read outside array bounds

Message ID 4A6B84A9.7020506@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

roel kluin July 25, 2009, 10:18 p.m. UTC
if dev_alloc_skb() fails on the first iteration of the allocation loop, and we
break out of the loop, then we end up writing before the start of the array.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
                 sc->lmc_rxq[i] = skb;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller July 27, 2009, 1:33 a.m. UTC | #1
From: Roel Kluin <roel.kluin@gmail.com>
Date: Sun, 26 Jul 2009 00:18:17 +0200

> if dev_alloc_skb() fails on the first iteration of the allocation loop, and we
> break out of the loop, then we end up writing before the start of the array.
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
...
>                  sc->failed_ring = 1;
> -                break;
> +                return;

This won't work.

First of all, if we allocated at least one buffer we should
mark the last one in the code right after this loop.

Second of all, we should purge the TX skbs in the next
loop even if we could not allocate even one RX buffer.

The thing to do is probably to guard the set of "[i-1]" RX ring
accesses with a "if (i != 0)" check.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
index 45b1822..ac8d5b2 100644
--- a/drivers/net/wan/lmc/lmc_main.c
+++ b/drivers/net/wan/lmc/lmc_main.c
@@ -1865,7 +1865,7 @@  static void lmc_softreset (lmc_softc_t * const sc) /*fold00*/
             if(skb == NULL){
                 printk(KERN_WARNING "%s: Failed to allocate receiver ring, will try again
", sc->name);
                 sc->failed_ring = 1;
-                break;
+                return;
             }
             else{