From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Skeggs <bskeggs@redhat.com>
Subject: Re: question regarding nvc0_instmem_suspend()
Date: Mon, 16 Aug 2010 08:24:03 +1000
Message-ID: <1281911043.2415.0.camel@nisroch>
References: <20100813213953.GU645@bicker>
	<AANLkTi=2UMXQXe1E1VTLofx613bVN1KNoOebZcPzonXC@mail.gmail.com>
Reply-To: bskeggs@redhat.com
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
	by gabe.freedesktop.org (Postfix) with ESMTP id 09F509E7A4
	for <dri-devel@lists.freedesktop.org>;
	Sun, 15 Aug 2010 15:24:36 -0700 (PDT)
In-Reply-To: <AANLkTi=2UMXQXe1E1VTLofx613bVN1KNoOebZcPzonXC@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org
To: Luca Tettamanti <kronos.it@gmail.com>
Cc: Dan Carpenter <error27@gmail.com>, dri-devel@lists.freedesktop.org
List-Id: dri-devel@lists.freedesktop.org

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