Re: [PATCH] 3c515: Write outside array bounds

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

if dev_alloc_skb() fails on the first iteration, a write to
cp->rx_ring[-1] occurs.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
>>>> can we error return like this?
>>>
>>> I doubt we can return here: there is a lot of cleaning missing.

>> I took drivers/net/3c59x.c as an example
>>
>> Is this going in the right direction?
> 
> The direction is right but a long and winding road... I guess it's for
> somebody with drivers knowhow. It seems most of the corkscrew_close()
> might be needed, including del_timer(). So, since this -1 case looks
> quite unlikely, it might be reasonable to only limit the most obvious
> damage with 'if (i != 0)' before [i - 1] write, like David advised
> in lmc case?
> 
> Cheers,
> Jarek P.

Thanks, here it is:

diff --git a/drivers/net/3c515.c b/drivers/net/3c515.c
index 3e00fa8..4a7c328 100644
--- a/drivers/net/3c515.c
+++ b/drivers/net/3c515.c
@@ -832,7 +832,9 @@ static int corkscrew_open(struct net_device *dev)
 			skb_reserve(skb, 2);	/* Align IP on 16 byte boundaries */
 			vp->rx_ring[i].addr = isa_virt_to_bus(skb->data);
 		}
-		vp->rx_ring[i - 1].next = isa_virt_to_bus(&vp->rx_ring[0]);	/* Wrap the ring. */
+		if (i != 0)
+			vp->rx_ring[i - 1].next =
+				isa_virt_to_bus(&vp->rx_ring[0]);	/* Wrap the ring. */
 		outl(isa_virt_to_bus(&vp->rx_ring[0]), ioaddr + UpListPtr);
 	}
 	if (vp->full_bus_master_tx) {	/* Boomerang bus master Tx. */