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,
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?
> 
> It's not correct to limit the TX ring loop by how many RX
> ring buffers we've been able to successfully allocate,
> that doesn't make any sense.
> 
> I'm talking about the second loop which you unexplainably
> changed to "for (j = 0; j < i; ..."

Ok, I apparently completely misunderstood. One remaining question is whether
the base address should still be written if i is 0?

diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
index 45b1822..8b61660 100644
--- a/drivers/net/wan/lmc/lmc_main.c
+++ b/drivers/net/wan/lmc/lmc_main.c
@@ -1897,11 +1897,12 @@ 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 */
+    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++)
     {