>-----Original Message----- >From: Ben Skeggs <[email protected]> >Sent: Wednesday, November 11, 2020 9:39 PM >To: Ruhl, Michael J <[email protected]> >Cc: Thomas Zimmermann <[email protected]>; [email protected]; >[email protected]; [email protected]; [email protected]; amd- >[email protected]; [email protected]; dri- >[email protected]; [email protected]; Roland >Scheidegger <[email protected]>; Jason Gunthorpe <[email protected]>; >Huang Rui <[email protected]>; VMware Graphics <linux-graphics- >[email protected]>; Gerd Hoffmann <[email protected]>; spice- >[email protected]; Alex Deucher <[email protected]>; >Dave Airlie <[email protected]>; Likun Gao <[email protected]>; Felix >Kuehling <[email protected]>; Hawking Zhang ><[email protected]> >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 <[email protected]> >wrote: >> >> >-----Original Message----- >> >From: Thomas Zimmermann <[email protected]> >> >Sent: Wednesday, November 11, 2020 7:08 AM >> >To: Ruhl, Michael J <[email protected]>; [email protected]; >> >[email protected]; [email protected]; [email protected] >> >Cc: [email protected]; [email protected]; >> >Maarten Lankhorst <[email protected]>; Maxime Ripard >> ><[email protected]>; Dave Airlie <[email protected]>; Gerd Hoffmann >> ><[email protected]>; Alex Deucher <[email protected]>; >> >VMware Graphics <[email protected]>; Roland >> >Scheidegger <[email protected]>; Huang Rui <[email protected]>; >> >Felix Kuehling <[email protected]>; Hawking Zhang >> ><[email protected]>; Jason Gunthorpe <[email protected]>; Likun >Gao >> ><[email protected]>; [email protected]; spice- >> >[email protected]; [email protected] >> >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 <[email protected]> >> >>> Sent: Tuesday, November 10, 2020 8:37 AM >> >>> To: [email protected]; [email protected]; [email protected]; Ruhl, >Michael J >> >>> <[email protected]>; [email protected] >> >>> Cc: [email protected]; [email protected]; >> >Thomas >> >>> Zimmermann <[email protected]>; Maarten Lankhorst >> >>> <[email protected]>; Maxime Ripard >> >>> <[email protected]>; Dave Airlie <[email protected]>; Gerd >Hoffmann >> >>> <[email protected]>; Alex Deucher <[email protected]>; >> >>> VMware Graphics <[email protected]>; Roland >> >>> Scheidegger <[email protected]>; Huang Rui ><[email protected]>; >> >>> Felix Kuehling <[email protected]>; Hawking Zhang >> >>> <[email protected]>; Jason Gunthorpe <[email protected]>; Likun >> >Gao >> >>> <[email protected]>; [email protected]; >spice- >> >>> [email protected]; [email protected] >> >>> 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 <[email protected]> >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 <[email protected]> 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 <[email protected]> >> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to >ttm_bus_placement") >> >>> Cc: Christian König <[email protected]> >> >>> Cc: Michael J. Ruhl <[email protected]> >> >>> Cc: Maarten Lankhorst <[email protected]> >> >>> Cc: Maxime Ripard <[email protected]> >> >>> Cc: Thomas Zimmermann <[email protected]> >> >>> Cc: David Airlie <[email protected]> >> >>> Cc: Daniel Vetter <[email protected]> >> >>> Cc: Ben Skeggs <[email protected]> >> >>> Cc: Dave Airlie <[email protected]> >> >>> Cc: Gerd Hoffmann <[email protected]> >> >>> Cc: Alex Deucher <[email protected]> >> >>> Cc: "Christian König" <[email protected]> >> >>> Cc: VMware Graphics <[email protected]> >> >>> Cc: Roland Scheidegger <[email protected]> >> >>> Cc: Huang Rui <[email protected]> >> >>> Cc: Felix Kuehling <[email protected]> >> >>> Cc: Hawking Zhang <[email protected]> >> >>> Cc: Jason Gunthorpe <[email protected]> >> >>> Cc: Likun Gao <[email protected]> >> >>> Cc: [email protected] >> >>> Cc: [email protected] >> >>> Cc: [email protected] >> >>> Cc: [email protected] >> >>> Cc: [email protected] >> >>> --- >> >>> 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 >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
