>-----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.
If you would like:
Reviewed-by: Michael J. Ruhl <michael.j.r...@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 <tzimmerm...@suse.de>
>> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to
>ttm_bus_placement")
>> >>> Cc: Christian König <christian.koe...@amd.com>
>> >>> Cc: Michael J. Ruhl <michael.j.r...@intel.com>
>> >>> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
>> >>> Cc: Maxime Ripard <mrip...@kernel.org>
>> >>> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
>> >>> Cc: David Airlie <airl...@linux.ie>
>> >>> Cc: Daniel Vetter <dan...@ffwll.ch>
>> >>> Cc: Ben Skeggs <bske...@redhat.com>
>> >>> Cc: Dave Airlie <airl...@redhat.com>
>> >>> Cc: Gerd Hoffmann <kra...@redhat.com>
>> >>> Cc: Alex Deucher <alexander.deuc...@amd.com>
>> >>> Cc: "Christian König" <christian.koe...@amd.com>
>> >>> Cc: VMware Graphics <linux-graphics-maintai...@vmware.com>
>> >>> Cc: Roland Scheidegger <srol...@vmware.com>
>> >>> Cc: Huang Rui <ray.hu...@amd.com>
>> >>> Cc: Felix Kuehling <felix.kuehl...@amd.com>
>> >>> Cc: Hawking Zhang <hawking.zh...@amd.com>
>> >>> Cc: Jason Gunthorpe <j...@ziepe.ca>
>> >>> Cc: Likun Gao <likun....@amd.com>
>> >>> Cc: dri-de...@lists.freedesktop.org
>> >>> Cc: nouveau@lists.freedesktop.org
>> >>> Cc: virtualizat...@lists.linux-foundation.org
>> >>> Cc: spice-de...@lists.freedesktop.org
>> >>> Cc: amd-...@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-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
