Re: [PATCH] drm/nouveau: Fix out-of-bounds access when deferencing MMU type

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 12.11.20 um 15:20 schrieb Ruhl, Michael J:
>> -----Original Message-----
>> From: Ben Skeggs <skeggsb@gmail.com>
>> Sent: Wednesday, November 11, 2020 9:39 PM
>> To: Ruhl, Michael J <michael.j.ruhl@intel.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>; bskeggs@redhat.com;
>> airlied@linux.ie; daniel@ffwll.ch; christian.koenig@amd.com; amd-
>> gfx@lists.freedesktop.org; nouveau@lists.freedesktop.org; dri-
>> devel@lists.freedesktop.org; virtualization@lists.linux-foundation.org; Roland
>> Scheidegger <sroland@vmware.com>; Jason Gunthorpe <jgg@ziepe.ca>;
>> Huang Rui <ray.huang@amd.com>; VMware Graphics <linux-graphics-
>> maintainer@vmware.com>; Gerd Hoffmann <kraxel@redhat.com>; spice-
>> devel@lists.freedesktop.org; Alex Deucher <alexander.deucher@amd.com>;
>> Dave Airlie <airlied@redhat.com>; Likun Gao <Likun.Gao@amd.com>; Felix
>> Kuehling <Felix.Kuehling@amd.com>; Hawking Zhang
>> <Hawking.Zhang@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.ruhl@intel.com>
>> wrote:
>>>> -----Original Message-----
>>>> From: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Sent: Wednesday, November 11, 2020 7:08 AM
>>>> To: Ruhl, Michael J <michael.j.ruhl@intel.com>; bskeggs@redhat.com;
>>>> airlied@linux.ie; daniel@ffwll.ch; christian.koenig@amd.com
>>>> Cc: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>>>> Maarten Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard
>>>> <mripard@kernel.org>; Dave Airlie <airlied@redhat.com>; Gerd Hoffmann
>>>> <kraxel@redhat.com>; Alex Deucher <alexander.deucher@amd.com>;
>>>> VMware Graphics <linux-graphics-maintainer@vmware.com>; Roland
>>>> Scheidegger <sroland@vmware.com>; Huang Rui <ray.huang@amd.com>;
>>>> Felix Kuehling <Felix.Kuehling@amd.com>; Hawking Zhang
>>>> <Hawking.Zhang@amd.com>; Jason Gunthorpe <jgg@ziepe.ca>; Likun
>> Gao
>>>> <Likun.Gao@amd.com>; virtualization@lists.linux-foundation.org; spice-
>>>> devel@lists.freedesktop.org; amd-gfx@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 <tzimmermann@suse.de>
>>>>>> Sent: Tuesday, November 10, 2020 8:37 AM
>>>>>> To: bskeggs@redhat.com; airlied@linux.ie; daniel@ffwll.ch; Ruhl,
>> Michael J
>>>>>> <michael.j.ruhl@intel.com>; christian.koenig@amd.com
>>>>>> Cc: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>>>> Thomas
>>>>>> Zimmermann <tzimmermann@suse.de>; Maarten Lankhorst
>>>>>> <maarten.lankhorst@linux.intel.com>; Maxime Ripard
>>>>>> <mripard@kernel.org>; Dave Airlie <airlied@redhat.com>; Gerd
>> Hoffmann
>>>>>> <kraxel@redhat.com>; Alex Deucher <alexander.deucher@amd.com>;
>>>>>> VMware Graphics <linux-graphics-maintainer@vmware.com>; Roland
>>>>>> Scheidegger <sroland@vmware.com>; Huang Rui
>> <ray.huang@amd.com>;
>>>>>> Felix Kuehling <Felix.Kuehling@amd.com>; Hawking Zhang
>>>>>> <Hawking.Zhang@amd.com>; Jason Gunthorpe <jgg@ziepe.ca>; Likun
>>>> Gao
>>>>>> <Likun.Gao@amd.com>; virtualization@lists.linux-foundation.org;
>> spice-
>>>>>> devel@lists.freedesktop.org; amd-gfx@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.ruhl@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.ruhl@intel.com>

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

Regards,
Christian.

>
> For this solution as well.
>
> Mike
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx