Am 12.11.20 um 15:20 schrieb Ruhl, Michael J:
-----Original Message-----
From: Ben Skeggs <skeggsb@xxxxxxxxx>
Sent: Wednesday, November 11, 2020 9:39 PM
To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>; bskeggs@xxxxxxxxxx;
airlied@xxxxxxxx; daniel@xxxxxxxx; christian.koenig@xxxxxxx; amd-
gfx@xxxxxxxxxxxxxxxxxxxxx; nouveau@xxxxxxxxxxxxxxxxxxxxx; dri-
devel@xxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; Roland
Scheidegger <sroland@xxxxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxx>;
Huang Rui <ray.huang@xxxxxxx>; VMware Graphics <linux-graphics-
maintainer@xxxxxxxxxx>; Gerd Hoffmann <kraxel@xxxxxxxxxx>; spice-
devel@xxxxxxxxxxxxxxxxxxxxx; Alex Deucher <alexander.deucher@xxxxxxx>;
Dave Airlie <airlied@xxxxxxxxxx>; Likun Gao <Likun.Gao@xxxxxxx>; Felix
Kuehling <Felix.Kuehling@xxxxxxx>; Hawking Zhang
<Hawking.Zhang@xxxxxxx>
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.ruhl@xxxxxxxxx>
wrote:
-----Original Message-----
From: Thomas Zimmermann <tzimmermann@xxxxxxx>
Sent: Wednesday, November 11, 2020 7:08 AM
To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; bskeggs@xxxxxxxxxx;
airlied@xxxxxxxx; daniel@xxxxxxxx; christian.koenig@xxxxxxx
Cc: nouveau@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard
<mripard@xxxxxxxxxx>; Dave Airlie <airlied@xxxxxxxxxx>; Gerd Hoffmann
<kraxel@xxxxxxxxxx>; Alex Deucher <alexander.deucher@xxxxxxx>;
VMware Graphics <linux-graphics-maintainer@xxxxxxxxxx>; Roland
Scheidegger <sroland@xxxxxxxxxx>; Huang Rui <ray.huang@xxxxxxx>;
Felix Kuehling <Felix.Kuehling@xxxxxxx>; Hawking Zhang
<Hawking.Zhang@xxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxx>; Likun
Gao
<Likun.Gao@xxxxxxx>; virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; spice-
devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
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 <tzimmermann@xxxxxxx>
Sent: Tuesday, November 10, 2020 8:37 AM
To: bskeggs@xxxxxxxxxx; airlied@xxxxxxxx; daniel@xxxxxxxx; Ruhl,
Michael J
<michael.j.ruhl@xxxxxxxxx>; christian.koenig@xxxxxxx
Cc: nouveau@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
Thomas
Zimmermann <tzimmermann@xxxxxxx>; Maarten Lankhorst
<maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard
<mripard@xxxxxxxxxx>; Dave Airlie <airlied@xxxxxxxxxx>; Gerd
Hoffmann
<kraxel@xxxxxxxxxx>; Alex Deucher <alexander.deucher@xxxxxxx>;
VMware Graphics <linux-graphics-maintainer@xxxxxxxxxx>; Roland
Scheidegger <sroland@xxxxxxxxxx>; Huang Rui
<ray.huang@xxxxxxx>;
Felix Kuehling <Felix.Kuehling@xxxxxxx>; Hawking Zhang
<Hawking.Zhang@xxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxx>; Likun
Gao
<Likun.Gao@xxxxxxx>; virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx;
spice-
devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
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.ruhl@xxxxxxxxx>
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.ruhl@xxxxxxxxx>
Feel free to add an Reviewed-by: Christian König
<christian.koenig@xxxxxxx> as well.
Regards,
Christian.
For this solution as well.
Mike
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization