i915: Read outside array bounds
Subsystems (2)
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
@@ -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]);
(cc jbarnes)
Quoted 11 lines · On Sun, 26 Jul 2009 00:50:38 +0200 Roel Kluin <[email protected]> wrote:
dev_priv->saveSWF1is a 16 element array, but this reads up to index 22Signed-off-by Roel Kluin <[email protected]>---
save_statedoes not do this addition, can it be removed? please review.drivers/gpu/drm/i915/i915_suspend.c +0
-0
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 :(
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->saveSWF1is a 16 element array, but this reads up to index 22Signed-off-by Roel Kluin <[email protected]>---
save_statedoes not do this addition, can it be removed? please review.drivers/gpu/drm/i915/i915_suspend.c +0
-0drivers/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]
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->saveSWF1is a 16 element array, but this reads up to index 22Signed-off-by Roel Kluin <[email protected]>---
save_statedoes not do this addition, can it be removed? please review.(quoted diff) +0
-0
b/drivers/gpu/drm/i915/i915_suspend.cindex 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
-0for (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