diff mbox

lmc: Read outside array bounds

Message ID 4A8004B0.5000504@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

roel kluin Aug. 10, 2009, 11:29 a.m. UTC 
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?

--
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 Aug. 13, 2009, 11:27 p.m. UTC | #1 
From: Roel Kluin <roel.kluin@gmail.com>
Date: Mon, 10 Aug 2009 13:29:52 +0200

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

Applied to net-next-2.6
--
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..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++)
     {