diff mbox

powerpc: Read buffer overflow

Message ID 4A76DB06.7090405@gmail.com (mailing list archive)
State Accepted, archived
Commit fb2881a7134576a6e95a63e3d2f34ea5629db4a1
Delegated to: Benjamin Herrenschmidt
Headers show

Commit Message

roel kluin Aug. 3, 2009, 12:41 p.m. UTC 
Check whether index is within bounds before grabbing the element.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---

Comments

Paul Mackerras Aug. 3, 2009, 12:57 p.m. UTC | #1 
Roel Kluin writes:

> Check whether index is within bounds before grabbing the element.

The change seems unnecessary since we only compute the address of the
element before the bounds check, we don't actually access the
element.  I believe that is legal in C.

Paul.
Wolfram Sang Aug. 3, 2009, 1:12 p.m. UTC | #2 
On Mon, Aug 03, 2009 at 10:57:17PM +1000, Paul Mackerras wrote:
> Roel Kluin writes:
> 
> > Check whether index is within bounds before grabbing the element.
> 
> The change seems unnecessary since we only compute the address of the
> element before the bounds check, we don't actually access the
> element.  I believe that is legal in C.

I've got this strange feeling of deja vu :)

http://thread.gmane.org/gmane.linux.ports.arm.kernel/63507

(I'd vote for applying it but won't mind if not)

Regards,

   Wolfram
Segher Boessenkool Aug. 3, 2009, 8:15 p.m. UTC | #3 
> The change seems unnecessary since we only compute the address of the
> element before the bounds check, we don't actually access the
> element.  I believe that is legal in C.

If you have an array a[N], taking &a[0] .. &a[N] are legal C, everything
else is not.


Segher
diff mbox

Patch

diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index a0f6838..588a5b0 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -294,10 +294,11 @@  static void macio_setup_interrupts(struct macio_dev *dev)
 	int i = 0, j = 0;
 
 	for (;;) {
-		struct resource *res = &dev->interrupt[j];
+		struct resource *res;
 
 		if (j >= MACIO_DEV_COUNT_IRQS)
 			break;
+		res = &dev->interrupt[j];
 		irq = irq_of_parse_and_map(np, i++);
 		if (irq == NO_IRQ)
 			break;
@@ -321,9 +322,10 @@  static void macio_setup_resources(struct macio_dev *dev,
 	int index;
 
 	for (index = 0; of_address_to_resource(np, index, &r) == 0; index++) {
-		struct resource *res = &dev->resource[index];
+		struct resource *res;
 		if (index >= MACIO_DEV_COUNT_RESOURCES)
 			break;
+		res = &dev->resource[index];
 		*res = r;
 		res->name = dev_name(&dev->ofdev.dev);