Skip to contentSkip to conversation

i915: Read outside array bounds

PATH Roel Kluin 4 participants 4 messages Jul 25, 2009 – Aug 2, 2009 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

dev_priv->saveSWF1 is a 16 element array, but this reads up to index 22

---
save_state does not do this addition, can it be removed? please review.

drivers/gpu/drm/i915/i915_suspend.c +1 -1
index 9e1d16e..1d04e19 100644
@@ -598,7 +598,7 @@ int i915_restore_state(struct drm_device *dev)
 
 	for (i = 0; i < 16; i++) {
 		I915_WRITE(SWF00 + (i << 2), dev_priv->saveSWF0[i]);
-		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i+7]);
+		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i]);
 	}
 	for (i = 0; i < 3; i++)
 		I915_WRITE(SWF30 + (i << 2), dev_priv->saveSWF2[i]);
#1 lore raw

(cc jbarnes)

Quoted 11 lines · On Sun, 26 Jul 2009 00:50:38 +0200 Roel Kluin <[email protected]> wrote:

dev_priv->saveSWF1 is a 16 element array, but this reads up to index 22

---
save_state does not do this addition, can it be removed? please review.

drivers/gpu/drm/i915/i915_suspend.c +0 -0
index 9e1d16e..1d04e19 100644

drivers/gpu/drm/i915/i915_suspend.c int i915_restore_state(struct drm_device *dev) · line 604

Quoted 8 lines
drivers/gpu/drm/i915/i915_suspend.c +1 -1
@@ -598,7 +598,7 @@ int i915_restore_state(struct drm_device *dev)
 
 	for (i = 0; i < 16; i++) {
 		I915_WRITE(SWF00 + (i << 2), dev_priv->saveSWF0[i]);
-		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i+7]);
+		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i]);
 	}
 	for (i = 0; i < 3; i++)
 		I915_WRITE(SWF30 + (i << 2), dev_priv->saveSWF2[i]);

This looks rather correct and the original code looked rather wrong.

Someone please tell me that this might fix one of our splendid number of i915 bugs :(

#2 lore raw
Quoted 27 lines · Andrew Morton

(cc jbarnes)

Quoted 11 lines · On Sun, 26 Jul 2009 00:50:38 +0200 Roel Kluin <[email protected]> wrote:

dev_priv->saveSWF1 is a 16 element array, but this reads up to index 22

---
save_state does not do this addition, can it be removed? please review.

drivers/gpu/drm/i915/i915_suspend.c +0 -0
index 9e1d16e..1d04e19 100644

drivers/gpu/drm/i915/i915_suspend.c int i915_restore_state(struct drm_device *dev) · line 604

Quoted 8 lines
drivers/gpu/drm/i915/i915_suspend.c +1 -1
@@ -598,7 +598,7 @@ int i915_restore_state(struct drm_device *dev)
 
 	for (i = 0; i < 16; i++) {
 		I915_WRITE(SWF00 + (i << 2), dev_priv->saveSWF0[i]);
-		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i+7]);
+		I915_WRITE(SWF10 + (i << 2), dev_priv->saveSWF1[i]);
 	}
 	for (i = 0; i < 3; i++)
 		I915_WRITE(SWF30 + (i << 2), dev_priv->saveSWF2[i]);

This looks rather correct and the original code looked rather wrong.

Quoted 2 lines

Someone please tell me that this might fix one of our splendid number of i915 bugs :(

Hmm, Jesse, what's your opinion about this one? It looks correct to me.

Rafael

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 33 lines · On Wed, 29 Jul 2009 23:06:14 +0200 "Rafael J. Wysocki" <[email protected]> wrote:
Quoted 32 lines · Andrew Morton

(cc jbarnes)

Quoted 23 lines · On Sun, 26 Jul 2009 00:50:38 +0200 Roel Kluin <[email protected]> wrote:

dev_priv->saveSWF1 is a 16 element array, but this reads up to index 22

---
save_state does not do this addition, can it be removed? please review.

(quoted diff) +0 -0

b/drivers/gpu/drm/i915/i915_suspend.c index 9e1d16e..1d04e19 100644 --- a/drivers/gpu/drm/i915/i915_suspend.c +++ b/drivers/gpu/drm/i915/i915_suspend.c

(quoted diff) +1 -1
@@ -598,7 +598,7 @@ int i915_restore_state(struct drm_device *dev)
 
 	for (i = 0; i < 16; i++) {
 		I915_WRITE(SWF00 + (i << 2),
 dev_priv->saveSWF0[i]);
-		I915_WRITE(SWF10 + (i << 2),
 dev_priv->saveSWF1[i+7]);
+		I915_WRITE(SWF10 + (i << 2),
 dev_priv->saveSWF1[i]); }
(quoted diff) +0 -0
 	for (i = 0; i < 3; i++)
 		I915_WRITE(SWF30 + (i << 2),
Quoted 1 line

dev_priv->saveSWF2[i]);

This looks rather correct and the original code looked rather wrong.

Quoted 5 lines
Quoted 2 lines

Someone please tell me that this might fix one of our splendid number of i915 bugs :(

Hmm, Jesse, what's your opinion about this one? It looks correct to me.

Yeah, the fix looks fine. And yes, we're upstreaming bug fixes as quickly as we responsibly can.

Jesse