[PATCH] drm/nouveau: Fix out-of-bounds access when deferencing MMU type
Ruhl, Michael J
michael.j.ruhl at intel.com
Thu Nov 12 14:20:53 UTC 2020
>-----Original Message-----
>From: Ben Skeggs <skeggsb at gmail.com>
>Sent: Wednesday, November 11, 2020 9:39 PM
>To: Ruhl, Michael J <michael.j.ruhl at intel.com>
>Cc: Thomas Zimmermann <tzimmermann at suse.de>; bskeggs at redhat.com;
>airlied at linux.ie; daniel at ffwll.ch; christian.koenig at amd.com; amd-
>gfx at lists.freedesktop.org; nouveau at lists.freedesktop.org; dri-
>devel at lists.freedesktop.org; virtualization at lists.linux-foundation.org; Roland
>Scheidegger <sroland at vmware.com>; Jason Gunthorpe <jgg at ziepe.ca>;
>Huang Rui <ray.huang at amd.com>; VMware Graphics <linux-graphics-
>maintainer at vmware.com>; Gerd Hoffmann <kraxel at redhat.com>; spice-
>devel at lists.freedesktop.org; Alex Deucher <alexander.deucher at amd.com>;
>Dave Airlie <airlied at redhat.com>; Likun Gao <Likun.Gao at amd.com>; Felix
>Kuehling <Felix.Kuehling at amd.com>; Hawking Zhang
><Hawking.Zhang at 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 at intel.com>
>wrote:
>>
>> >-----Original Message-----
>> >From: Thomas Zimmermann <tzimmermann at suse.de>
>> >Sent: Wednesday, November 11, 2020 7:08 AM
>> >To: Ruhl, Michael J <michael.j.ruhl at intel.com>; bskeggs at redhat.com;
>> >airlied at linux.ie; daniel at ffwll.ch; christian.koenig at amd.com
>> >Cc: nouveau at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
>> >Maarten Lankhorst <maarten.lankhorst at linux.intel.com>; Maxime Ripard
>> ><mripard at kernel.org>; Dave Airlie <airlied at redhat.com>; Gerd Hoffmann
>> ><kraxel at redhat.com>; Alex Deucher <alexander.deucher at amd.com>;
>> >VMware Graphics <linux-graphics-maintainer at vmware.com>; Roland
>> >Scheidegger <sroland at vmware.com>; Huang Rui <ray.huang at amd.com>;
>> >Felix Kuehling <Felix.Kuehling at amd.com>; Hawking Zhang
>> ><Hawking.Zhang at amd.com>; Jason Gunthorpe <jgg at ziepe.ca>; Likun
>Gao
>> ><Likun.Gao at amd.com>; virtualization at lists.linux-foundation.org; spice-
>> >devel at lists.freedesktop.org; amd-gfx at 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 at suse.de>
>> >>> Sent: Tuesday, November 10, 2020 8:37 AM
>> >>> To: bskeggs at redhat.com; airlied at linux.ie; daniel at ffwll.ch; Ruhl,
>Michael J
>> >>> <michael.j.ruhl at intel.com>; christian.koenig at amd.com
>> >>> Cc: nouveau at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
>> >Thomas
>> >>> Zimmermann <tzimmermann at suse.de>; Maarten Lankhorst
>> >>> <maarten.lankhorst at linux.intel.com>; Maxime Ripard
>> >>> <mripard at kernel.org>; Dave Airlie <airlied at redhat.com>; Gerd
>Hoffmann
>> >>> <kraxel at redhat.com>; Alex Deucher <alexander.deucher at amd.com>;
>> >>> VMware Graphics <linux-graphics-maintainer at vmware.com>; Roland
>> >>> Scheidegger <sroland at vmware.com>; Huang Rui
><ray.huang at amd.com>;
>> >>> Felix Kuehling <Felix.Kuehling at amd.com>; Hawking Zhang
>> >>> <Hawking.Zhang at amd.com>; Jason Gunthorpe <jgg at ziepe.ca>; Likun
>> >Gao
>> >>> <Likun.Gao at amd.com>; virtualization at lists.linux-foundation.org;
>spice-
>> >>> devel at lists.freedesktop.org; amd-gfx at 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 at 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.
If you would like:
Reviewed-by: Michael J. Ruhl <michael.j.ruhl at intel.com>
For this solution as well.
Mike
>>
>> Thanks,
>> Mike
>>
>> >Best regards
>> >Thomas
>> >
>> >>
>> >> Mike
>> >>
>> >>>
>> >>> [ 18.304116]
>> >>>
>>
>>==========================================================
>=
>> >>> =======
>> >>> [ 18.311649] BUG: KASAN: slab-out-of-bounds in
>> >>> nouveau_ttm_io_mem_reserve+0x17a/0x7e0 [nouveau]
>> >>> [ 18.320415] Read of size 1 at addr ffff88810ffac1fe by task systemd-
>> >>> udevd/342
>> >>> [ 18.327681]
>> >>> [ 18.329208] CPU: 1 PID: 342 Comm: systemd-udevd Tainted: G E
>> >>> 5.10.0-rc2-1-default+ #581
>> >>> [ 18.338681] Hardware name: Dell Inc. OptiPlex 9020/0N4YC8, BIOS A24
>> >>> 10/24/2018
>> >>> [ 18.346032] Call Trace:
>> >>> [ 18.348536] dump_stack+0xae/0xe5
>> >>> [ 18.351919] print_address_description.constprop.0+0x17/0xf0
>> >>> [ 18.357787] ? nouveau_ttm_io_mem_reserve+0x17a/0x7e0
>[nouveau]
>> >>> [ 18.363818] __kasan_report.cold+0x20/0x38
>> >>> [ 18.368099] ? nouveau_ttm_io_mem_reserve+0x17a/0x7e0
>[nouveau]
>> >>> [ 18.374133] kasan_report+0x3a/0x50
>> >>> [ 18.377789] nouveau_ttm_io_mem_reserve+0x17a/0x7e0 [nouveau]
>> >>> <...>
>> >>> [ 18.767690] Allocated by task 342:
>> >>> [ 18.773087] kasan_save_stack+0x1b/0x40
>> >>> [ 18.778890] __kasan_kmalloc.constprop.0+0xbf/0xd0
>> >>> [ 18.785646] __kmalloc_track_caller+0x1be/0x390
>> >>> [ 18.792165] kstrdup_const+0x46/0x70
>> >>> [ 18.797686] kobject_set_name_vargs+0x2f/0xb0
>> >>> [ 18.803992] kobject_init_and_add+0x9d/0xf0
>> >>> [ 18.810117] ttm_mem_global_init+0x12c/0x210 [ttm]
>> >>> [ 18.816853] ttm_bo_global_init+0x4a/0x160 [ttm]
>> >>> [ 18.823420] ttm_bo_device_init+0x39/0x220 [ttm]
>> >>> [ 18.830046] nouveau_ttm_init+0x2c3/0x830 [nouveau]
>> >>> [ 18.836929] nouveau_drm_device_init+0x1b4/0x3f0 [nouveau]
>> >>> <...>
>> >>> [ 19.105336]
>> >>>
>>
>>==========================================================
>=
>> >>> =======
>> >>>
>> >>> Fix this error, by not using type_vram as an index if it's negative.
>> >>> Assume default values instead.
>> >>>
>> >>> The error was seen on Nvidia G72 hardware.
>> >>>
>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to
>ttm_bus_placement")
>> >>> Cc: Christian König <christian.koenig at amd.com>
>> >>> Cc: Michael J. Ruhl <michael.j.ruhl at intel.com>
>> >>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> >>> Cc: Maxime Ripard <mripard at kernel.org>
>> >>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>> >>> Cc: David Airlie <airlied at linux.ie>
>> >>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> >>> Cc: Ben Skeggs <bskeggs at redhat.com>
>> >>> Cc: Dave Airlie <airlied at redhat.com>
>> >>> Cc: Gerd Hoffmann <kraxel at redhat.com>
>> >>> Cc: Alex Deucher <alexander.deucher at amd.com>
>> >>> Cc: "Christian König" <christian.koenig at amd.com>
>> >>> Cc: VMware Graphics <linux-graphics-maintainer at vmware.com>
>> >>> Cc: Roland Scheidegger <sroland at vmware.com>
>> >>> Cc: Huang Rui <ray.huang at amd.com>
>> >>> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
>> >>> Cc: Hawking Zhang <Hawking.Zhang at amd.com>
>> >>> Cc: Jason Gunthorpe <jgg at ziepe.ca>
>> >>> Cc: Likun Gao <Likun.Gao at amd.com>
>> >>> Cc: dri-devel at lists.freedesktop.org
>> >>> Cc: nouveau at lists.freedesktop.org
>> >>> Cc: virtualization at lists.linux-foundation.org
>> >>> Cc: spice-devel at lists.freedesktop.org
>> >>> Cc: amd-gfx at lists.freedesktop.org
>> >>> ---
>> >>> drivers/gpu/drm/nouveau/nouveau_bo.c | 5 ++++-
>> >>> 1 file changed, 4 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> >>> b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> >>> index 8133377d865d..fe15299d417e 100644
>> >>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> >>> @@ -1142,9 +1142,12 @@ nouveau_ttm_io_mem_reserve(struct
>> >>> ttm_bo_device *bdev, struct ttm_resource *reg)
>> >>> struct nvkm_device *device = nvxx_device(&drm->client.device);
>> >>> struct nouveau_mem *mem = nouveau_mem(reg);
>> >>> struct nvif_mmu *mmu = &drm->client.mmu;
>> >>> - const u8 type = mmu->type[drm->ttm.type_vram].type;
>> >>> + u8 type = 0;
>> >>> int ret;
>> >>>
>> >>> + if (drm->ttm.type_vram >= 0)
>> >>> + type = mmu->type[drm->ttm.type_vram].type;
>> >>> +
>> >>> mutex_lock(&drm->ttm.io_reserve_mutex);
>> >>> retry:
>> >>> switch (reg->mem_type) {
>> >>> --
>> >>> 2.29.2
>> >>
>> >
>> >--
>> >Thomas Zimmermann
>> >Graphics Driver Developer
>> >SUSE Software Solutions Germany GmbH
>> >Maxfeldstr. 5, 90409 Nürnberg, Germany
>> >(HRB 36809, AG Nürnberg)
>> >Geschäftsführer: Felix Imendörffer
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the amd-gfx
mailing list