On Fri, 2010-08-13 at 23:59 +0200, Luca Tettamanti wrote:

On Fri, Aug 13, 2010 at 11:39 PM, Dan Carpenter error27@gmail.com wrote:

...

Smatch thinks there is a buffer overflow in nvc0_instmem_suspend() and I've looked at it, but I don't understand the code.

drivers/gpu/drm/nouveau/nvc0_instmem.c +152 nvc0_instmem_suspend(10) error: buffer overflow 'dev_priv->susres.ramin_copy' 16384 <= 1835008

141 int 142 nvc0_instmem_suspend(struct drm_device *dev) 143 { 144 struct drm_nouveau_private *dev_priv = dev->dev_private; 145 int i; 146 147 dev_priv->susres.ramin_copy = vmalloc(65536);

   dev_priv->susres.ramin_copy is an array of 16384 u32 elements
   (65536 bytes).

148 if (!dev_priv->susres.ramin_copy) 149 return -ENOMEM; 150 151 for (i = 0x700000; i < 0x710000; i += 4) 152 dev_priv->susres.ramin_copy[i/4] = nv_rd32(dev, i);

   0x700000 / 4 is 1835008 so we're way past the end of the array
   and then we get larger.

I guess that it should be something like:

base = 0x700000;
for (i = 0; i < 0x10000; i += 4)
    dev_priv->susres.ramin_copy[i/4] = nv_rd32(dev, base + i);

Oops, what a thinko. I've pushed a fix to nouveau git, I'll send it on for inclusion in 2.6.36.

Ben.

Luca