Am 12.11.20 um 15:20 schrieb Ruhl, Michael J:
-----Original Message-----
From: Ben Skeggs <skeg...@gmail.com>
Sent: Wednesday, November 11, 2020 9:39 PM
To: Ruhl, Michael J <michael.j.r...@intel.com>
Cc: Thomas Zimmermann <tzimmerm...@suse.de>; bske...@redhat.com;
airl...@linux.ie; dan...@ffwll.ch; christian.koe...@amd.com; amd-
g...@lists.freedesktop.org; nouveau@lists.freedesktop.org; dri-
de...@lists.freedesktop.org; virtualizat...@lists.linux-foundation.org; Roland
Scheidegger <srol...@vmware.com>; Jason Gunthorpe <j...@ziepe.ca>;
Huang Rui <ray.hu...@amd.com>; VMware Graphics <linux-graphics-
maintai...@vmware.com>; Gerd Hoffmann <kra...@redhat.com>; spice-
de...@lists.freedesktop.org; Alex Deucher <alexander.deuc...@amd.com>;
Dave Airlie <airl...@redhat.com>; Likun Gao <likun....@amd.com>; Felix
Kuehling <felix.kuehl...@amd.com>; Hawking Zhang
<hawking.zh...@amd.com>
Subject: Re: [PATCH] drm/nouveau: Fix out-of-bounds access when
deferencing MMU type

On Thu, 12 Nov 2020 at 02:27, Ruhl, Michael J <michael.j.r...@intel.com>
wrote:
-----Original Message-----
From: Thomas Zimmermann <tzimmerm...@suse.de>
Sent: Wednesday, November 11, 2020 7:08 AM
To: Ruhl, Michael J <michael.j.r...@intel.com>; bske...@redhat.com;
airl...@linux.ie; dan...@ffwll.ch; christian.koe...@amd.com
Cc: nouveau@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
Maarten Lankhorst <maarten.lankho...@linux.intel.com>; Maxime Ripard
<mrip...@kernel.org>; Dave Airlie <airl...@redhat.com>; Gerd Hoffmann
<kra...@redhat.com>; Alex Deucher <alexander.deuc...@amd.com>;
VMware Graphics <linux-graphics-maintai...@vmware.com>; Roland
Scheidegger <srol...@vmware.com>; Huang Rui <ray.hu...@amd.com>;
Felix Kuehling <felix.kuehl...@amd.com>; Hawking Zhang
<hawking.zh...@amd.com>; Jason Gunthorpe <j...@ziepe.ca>; Likun
Gao
<likun....@amd.com>; virtualizat...@lists.linux-foundation.org; spice-
de...@lists.freedesktop.org; amd-...@lists.freedesktop.org
Subject: Re: [PATCH] drm/nouveau: Fix out-of-bounds access when
deferencing MMU type

Hi

Am 10.11.20 um 16:27 schrieb Ruhl, Michael J:

-----Original Message-----
From: Thomas Zimmermann <tzimmerm...@suse.de>
Sent: Tuesday, November 10, 2020 8:37 AM
To: bske...@redhat.com; airl...@linux.ie; dan...@ffwll.ch; Ruhl,
Michael J
<michael.j.r...@intel.com>; christian.koe...@amd.com
Cc: nouveau@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
Thomas
Zimmermann <tzimmerm...@suse.de>; Maarten Lankhorst
<maarten.lankho...@linux.intel.com>; Maxime Ripard
<mrip...@kernel.org>; Dave Airlie <airl...@redhat.com>; Gerd
Hoffmann
<kra...@redhat.com>; Alex Deucher <alexander.deuc...@amd.com>;
VMware Graphics <linux-graphics-maintai...@vmware.com>; Roland
Scheidegger <srol...@vmware.com>; Huang Rui
<ray.hu...@amd.com>;
Felix Kuehling <felix.kuehl...@amd.com>; Hawking Zhang
<hawking.zh...@amd.com>; Jason Gunthorpe <j...@ziepe.ca>; Likun
Gao
<likun....@amd.com>; virtualizat...@lists.linux-foundation.org;
spice-
de...@lists.freedesktop.org; amd-...@lists.freedesktop.org
Subject: [PATCH] drm/nouveau: Fix out-of-bounds access when
deferencing
MMU type

The value of struct drm_device.ttm.type_vram can become -1 for
unknown
types of memory (see nouveau_ttm_init()). This leads to an out-of-
bounds
error when accessing struct nvif_mmu.type[]:
Would this make more sense to just set the type_vram = 0 instead of -1?
>From what I understand, these indices refer to an internal type of MMU,
rsp the MMU's capabilities. However, my hardware (pre-NV50) does not
have an MMU at all.
Yeah, and upon further review I see that my comment was completely
wrong
(value vs. index).

A better suggestion would have been, create an entry in the array that
means,
"unsupported type" with a value of 0, but...

I agree that it would be nice to have a cleaner design that incorporates
this case, but resolving that would apparently require more than a bugfix.
I agree.  The -1 index is a special case for the platform path
(platform != NV_DEVICE_INFO_V0_SOC).  This is a fix for the issue, but not
a complete solution.

If you need it:
Reviewed-by: Michael J. Ruhl <michael.j.r...@intel.com>
I've put an alternate fix for this here[1], and will get it into
drm-fixes later today.

Ben.

[1]
https://github.com/skeggsb/nouveau/commit/4590f7120c2f1f4aea9d8b93a2d
ae43b312d35ad
This makes a lot of sense.  I spent some time trying to reconcile the platform 
info
that was not being used in this case, but didn't see the solution like this.   
This is
pretty clean.

I was already wondering why the old code never hit that problem, but this explains it properly and also fixes it up cleanly.


If you would like:

Reviewed-by: Michael J. Ruhl <michael.j.r...@intel.com>

Feel free to add an Reviewed-by: Christian König <christian.koe...@amd.com> as well.

Regards,
Christian.


For this solution as well.

Mike


_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Reply via email to