Re: [PATCH] lmc: Read outside array bounds

[Roel Kluin <roel.kluin@gmail.com>]

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>
---
> 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.

Forgot a bit about this one, but I hope this is what you meant?

diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
index 45b1822..b26fabb 100644
--- a/drivers/net/wan/lmc/lmc_main.c
+++ b/drivers/net/wan/lmc/lmc_main.c
@@ -1838,7 +1838,7 @@ void lmc_mii_writereg (lmc_softc_t * const sc, unsigned devaddr, unsigned regno,
 
 static void lmc_softreset (lmc_softc_t * const sc) /*fold00*/
 {
-    int i;
+    int i, j;
 
     lmc_trace(sc->lmc_device, "lmc_softreset in");
 
@@ -1897,24 +1897,27 @@ static void lmc_softreset (lmc_softc_t * const sc) /*fold00*/
     /*
      * Sets end of ring
      */
-    sc->lmc_rxring[i - 1].length |= 0x02000000; /* Set end of buffers flag */
-    sc->lmc_rxring[i - 1].buffer2 = virt_to_bus (&sc->lmc_rxring[0]); /* Point back to the start */
-    LMC_CSR_WRITE (sc, csr_rxlist, virt_to_bus (sc->lmc_rxring)); /* write base address */
+    if (i > 0) {
+        sc->lmc_rxring[i - 1].length |= 0x02000000; /* Set end of buffers flag */
+        sc->lmc_rxring[i - 1].buffer2 = virt_to_bus(&sc->lmc_rxring[0]); /* Point back to the start */
+        LMC_CSR_WRITE(sc, csr_rxlist, virt_to_bus(sc->lmc_rxring)); /* write base address */
+    }
 
 
     /* Initialize the transmit rings and buffers */
-    for (i = 0; i < LMC_TXDESCS; i++)
-    {
-        if (sc->lmc_txq[i] != NULL){		/* have buffer */
-            dev_kfree_skb(sc->lmc_txq[i]);	/* free it */
+    for (j = 0; j < i; j++) {
+        if (sc->lmc_txq[j] != NULL) {		/* have buffer */
+            dev_kfree_skb(sc->lmc_txq[j]);	/* free it */
 	    sc->lmc_device->stats.tx_dropped++;	/* We just dropped a packet */
         }
-        sc->lmc_txq[i] = NULL;
-        sc->lmc_txring[i].status = 0x00000000;
-        sc->lmc_txring[i].buffer2 = virt_to_bus (&sc->lmc_txring[i + 1]);
+        sc->lmc_txq[j] = NULL;
+        sc->lmc_txring[j].status = 0x00000000;
+        sc->lmc_txring[j].buffer2 = virt_to_bus(&sc->lmc_txring[j + 1]);
+    }
+    if (j > 0) {
+        sc->lmc_txring[j - 1].buffer2 = virt_to_bus(&sc->lmc_txring[0]);
+        LMC_CSR_WRITE(sc, csr_txlist, virt_to_bus(sc->lmc_txring));
     }
-    sc->lmc_txring[i - 1].buffer2 = virt_to_bus (&sc->lmc_txring[0]);
-    LMC_CSR_WRITE (sc, csr_txlist, virt_to_bus (sc->lmc_txring));
 
     lmc_trace(sc->lmc_device, "lmc_softreset out");
 }