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

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

[Thomas Zimmermann]

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[]:

  [   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@suse.de>
Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
Cc: Christian König <christian.koenig@amd.com>
Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Roland Scheidegger <sroland@vmware.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Hawking Zhang <Hawking.Zhang@amd.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Likun Gao <Likun.Gao@amd.com>
Cc: dri-devel@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
Cc: amd-gfx@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

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

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

[Christian König]

Am 10.11.20 um 14:36 schrieb Thomas Zimmermann:
> 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[]:
>
>    [   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@suse.de>
> Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Roland Scheidegger <sroland@vmware.com>
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Likun Gao <Likun.Gao@amd.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org

Acked-by: Christian König <christian.koenig@amd.com>

> ---
>   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) {

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

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

[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?

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@suse.de>
>Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
>Cc: Christian König <christian.koenig@amd.com>
>Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>Cc: Maxime Ripard <mripard@kernel.org>
>Cc: Thomas Zimmermann <tzimmermann@suse.de>
>Cc: David Airlie <airlied@linux.ie>
>Cc: Daniel Vetter <daniel@ffwll.ch>
>Cc: Ben Skeggs <bskeggs@redhat.com>
>Cc: Dave Airlie <airlied@redhat.com>
>Cc: Gerd Hoffmann <kraxel@redhat.com>
>Cc: Alex Deucher <alexander.deucher@amd.com>
>Cc: "Christian König" <christian.koenig@amd.com>
>Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>Cc: Roland Scheidegger <sroland@vmware.com>
>Cc: Huang Rui <ray.huang@amd.com>
>Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>Cc: Hawking Zhang <Hawking.Zhang@amd.com>
>Cc: Jason Gunthorpe <jgg@ziepe.ca>
>Cc: Likun Gao <Likun.Gao@amd.com>
>Cc: dri-devel@lists.freedesktop.org
>Cc: nouveau@lists.freedesktop.org
>Cc: virtualization@lists.linux-foundation.org
>Cc: spice-devel@lists.freedesktop.org
>Cc: amd-gfx@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

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

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

[Thomas Zimmermann]

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.

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.

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@suse.de>
>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: "Christian König" <christian.koenig@amd.com>
>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>> Cc: Roland Scheidegger <sroland@vmware.com>
>> Cc: Huang Rui <ray.huang@amd.com>
>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>> Cc: Likun Gao <Likun.Gao@amd.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: nouveau@lists.freedesktop.org
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: spice-devel@lists.freedesktop.org
>> Cc: amd-gfx@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Ruhl, Michael J]

>-----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>

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@suse.de>
>>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Maxime Ripard <mripard@kernel.org>
>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>> Cc: David Airlie <airlied@linux.ie>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Ben Skeggs <bskeggs@redhat.com>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>>> Cc: Roland Scheidegger <sroland@vmware.com>
>>> Cc: Huang Rui <ray.huang@amd.com>
>>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>>> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
>>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>>> Cc: Likun Gao <Likun.Gao@amd.com>
>>> Cc: dri-devel@lists.freedesktop.org
>>> Cc: nouveau@lists.freedesktop.org
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: spice-devel@lists.freedesktop.org
>>> Cc: amd-gfx@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Ben Skeggs]

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/4590f7120c2f1f4aea9d8b93a2dae43b312d35ad

>
> 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@suse.de>
> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to ttm_bus_placement")
> >>> Cc: Christian König <christian.koenig@amd.com>
> >>> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Cc: Maxime Ripard <mripard@kernel.org>
> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>> Cc: David Airlie <airlied@linux.ie>
> >>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>> Cc: Ben Skeggs <bskeggs@redhat.com>
> >>> Cc: Dave Airlie <airlied@redhat.com>
> >>> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>> Cc: "Christian König" <christian.koenig@amd.com>
> >>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> >>> Cc: Roland Scheidegger <sroland@vmware.com>
> >>> Cc: Huang Rui <ray.huang@amd.com>
> >>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
> >>> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> >>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> >>> Cc: Likun Gao <Likun.Gao@amd.com>
> >>> Cc: dri-devel@lists.freedesktop.org
> >>> Cc: nouveau@lists.freedesktop.org
> >>> Cc: virtualization@lists.linux-foundation.org
> >>> Cc: spice-devel@lists.freedesktop.org
> >>> Cc: amd-gfx@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[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.

If you would like:

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@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@suse.de>
>> >>> Fixes: 1cf65c45183a ("drm/ttm: add caching state to
>ttm_bus_placement")
>> >>> Cc: Christian König <christian.koenig@amd.com>
>> >>> Cc: Michael J. Ruhl <michael.j.ruhl@intel.com>
>> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> >>> Cc: Maxime Ripard <mripard@kernel.org>
>> >>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> >>> Cc: David Airlie <airlied@linux.ie>
>> >>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> >>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> >>> Cc: Dave Airlie <airlied@redhat.com>
>> >>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> >>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> >>> Cc: "Christian König" <christian.koenig@amd.com>
>> >>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
>> >>> Cc: Roland Scheidegger <sroland@vmware.com>
>> >>> Cc: Huang Rui <ray.huang@amd.com>
>> >>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>> >>> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
>> >>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>> >>> Cc: Likun Gao <Likun.Gao@amd.com>
>> >>> Cc: dri-devel@lists.freedesktop.org
>> >>> Cc: nouveau@lists.freedesktop.org
>> >>> Cc: virtualization@lists.linux-foundation.org
>> >>> Cc: spice-devel@lists.freedesktop.org
>> >>> Cc: amd-gfx@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Christian König]

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