Re: out-of-bounds array index

Previous thread: Re: Gadget driver repository? by Robin Getz on Thursday, February 7, 2008 - 2:56 pm. (1 message)

Next thread: [PATCH] Use global TLB flushes in MTRR code by Andi Kleen on Thursday, February 7, 2008 - 3:02 pm. (10 messages)
To: <linux-kernel@...>, <jesse.barnes@...>
Date: Thursday, February 7, 2008 - 2:56 pm

Hi,

Just saw this from gcc:

drivers/char/drm/i915_drv.c: In function ?i915_suspend?:
drivers/char/drm/i915_drv.c:173: warning: array subscript is above array
bounds
CC [M] drivers/char/drm/i915_dma.o
drivers/char/drm/i915_drv.c: In function ?i915_resume?:
drivers/char/drm/i915_drv.c:220: warning: array subscript is above array
bounds

It's this code:

dev_priv->saveGR[0x18] =
i915_read_indexed(VGA_GR_INDEX, VGA_GR_DATA, 0x18);

which looks legit, since saveGR is

u8 saveGR[24];

It has been introduced by commit
ba8bbcf6ff4650712f64c0ef61139c73898e2165, which seems to be you Jesse.

--
Jens Axboe

--

To: Jens Axboe <jens.axboe@...>
Cc: <linux-kernel@...>, <jesse.barnes@...>
Date: Thursday, February 7, 2008 - 3:21 pm

It is not legit at all. 0x18 is the 25th position in the array,
but it is only 24 big. (Excluding play-hide-and-seek games like
allocating more in case of malloc or char *foo[0].)

--

To: Jan Engelhardt <jengelh@...>
Cc: Jens Axboe <jens.axboe@...>, <linux-kernel@...>
Date: Thursday, February 7, 2008 - 3:28 pm

I think he was saying that the warning was legit. Anyway, my gcc isn't smart
enough to emit warnings like this, maybe it's time to ugprade...

Jesse
--

To: Jesse Barnes <jesse.barnes@...>
Cc: Jan Engelhardt <jengelh@...>, <linux-kernel@...>
Date: Thursday, February 7, 2008 - 4:01 pm

of course, the reference was to the warning (I do know C and array
indexing 101 :-)

--
Jens Axboe

--

To: Jens Axboe <jens.axboe@...>
Cc: <linux-kernel@...>, <torvalds@...>
Date: Thursday, February 7, 2008 - 3:15 pm

Just a silly off by one, don't know why I didn't catch it earlier. I'll push
the fix to the drm tree. Linus, you may want to take it in parallel.

Jesse

Make sure we have enough room for all the GR registers or we'll end up
clobbering the AR index register (which should actually be harmless unless
the BIOS is making an assumption about it).

Signed-off-by: Jesse Barnes <jesse.barnes@intel.com>

diff --git a/drivers/char/drm/i915_drv.h b/drivers/char/drm/i915_drv.h
index 37bbf67..f8308bf 100644
--- a/drivers/char/drm/i915_drv.h
+++ b/drivers/char/drm/i915_drv.h
@@ -187,7 +187,7 @@ typedef struct drm_i915_private {
u32 saveSWF2[3];
u8 saveMSR;
u8 saveSR[8];
- u8 saveGR[24];
+ u8 saveGR[25];
u8 saveAR_INDEX;
u8 saveAR[20];
u8 saveDACMASK;
--

To: Jens Axboe <jens.axboe@...>
Cc: <linux-kernel@...>
Date: Thursday, February 7, 2008 - 3:03 pm

I'll take a look, thanks.

Jesse
--

Previous thread: Re: Gadget driver repository? by Robin Getz on Thursday, February 7, 2008 - 2:56 pm. (1 message)

Next thread: [PATCH] Use global TLB flushes in MTRR code by Andi Kleen on Thursday, February 7, 2008 - 3:02 pm. (10 messages)