Philipp Stanner
2025-Jul-07 08:27 UTC
[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init
On Tue, 2025-06-17 at 14:00 +1000, Ben Skeggs wrote:> If any of the ACPI calls fail, memory allocated for the input buffer > would be leaked.? Fix failure paths to free allocated memory. > > Also add checks to ensure the allocations succeeded in the first > place.If you've got a kmemleak trace, it would also be good to put it here in the commit message so that we can distinguish this bug from potential other leaks. P.> > Reported-by: Danilo Krummrich <dakr at kernel.org> > Fixes: 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting > GSP-RM") > Signed-off-by: Ben Skeggs <bskeggs at nvidia.com> > --- > ?.../drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c | 20 +++++++++++++---- > -- > ?1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c > b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c > index baf42339f93e..b098a7555fde 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gsp.c > @@ -719,7 +719,6 @@ r535_gsp_acpi_caps(acpi_handle handle, > CAPS_METHOD_DATA *caps) > ? union acpi_object argv4 = { > ? .buffer.type??? = ACPI_TYPE_BUFFER, > ? .buffer.length? = 4, > - .buffer.pointer = kmalloc(argv4.buffer.length, > GFP_KERNEL), > ? }, *obj; > ? > ? caps->status = 0xffff; > @@ -727,17 +726,22 @@ r535_gsp_acpi_caps(acpi_handle handle, > CAPS_METHOD_DATA *caps) > ? if (!acpi_check_dsm(handle, &NVOP_DSM_GUID, NVOP_DSM_REV, > BIT_ULL(0x1a))) > ? return; > ? > + argv4.buffer.pointer = kmalloc(argv4.buffer.length, > GFP_KERNEL); > + if (!argv4.buffer.pointer) > + return; > + > ? obj = acpi_evaluate_dsm(handle, &NVOP_DSM_GUID, > NVOP_DSM_REV, 0x1a, &argv4); > ? if (!obj) > - return; > + goto done; > ? > ? if (WARN_ON(obj->type != ACPI_TYPE_BUFFER) || > ? ??? WARN_ON(obj->buffer.length != 4)) > - return; > + goto done; > ? > ? caps->status = 0; > ? caps->optimusCaps = *(u32 *)obj->buffer.pointer; > ? > +done: > ? ACPI_FREE(obj); > ? > ? kfree(argv4.buffer.pointer); > @@ -754,24 +758,28 @@ r535_gsp_acpi_jt(acpi_handle handle, > JT_METHOD_DATA *jt) > ? union acpi_object argv4 = { > ? .buffer.type??? = ACPI_TYPE_BUFFER, > ? .buffer.length? = sizeof(caps), > - .buffer.pointer = kmalloc(argv4.buffer.length, > GFP_KERNEL), > ? }, *obj; > ? > ? jt->status = 0xffff; > ? > + argv4.buffer.pointer = kmalloc(argv4.buffer.length, > GFP_KERNEL); > + if (!argv4.buffer.pointer) > + return; > + > ? obj = acpi_evaluate_dsm(handle, &JT_DSM_GUID, JT_DSM_REV, > 0x1, &argv4); > ? if (!obj) > - return; > + goto done; > ? > ? if (WARN_ON(obj->type != ACPI_TYPE_BUFFER) || > ? ??? WARN_ON(obj->buffer.length != 4)) > - return; > + goto done; > ? > ? jt->status = 0; > ? jt->jtCaps = *(u32 *)obj->buffer.pointer; > ? jt->jtRevId = (jt->jtCaps & 0xfff00000) >> 20; > ? jt->bSBIOSCaps = 0; > ? > +done: > ? ACPI_FREE(obj); > ? > ? kfree(argv4.buffer.pointer);
Danilo Krummrich
2025-Jul-07 14:31 UTC
[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init
On 7/7/25 10:27 AM, Philipp Stanner wrote:> On Tue, 2025-06-17 at 14:00 +1000, Ben Skeggs wrote: >> If any of the ACPI calls fail, memory allocated for the input buffer >> would be leaked.? Fix failure paths to free allocated memory. >> >> Also add checks to ensure the allocations succeeded in the first >> place. > > If you've got a kmemleak trace, it would also be good to put it here in > the commit message so that we can distinguish this bug from potential > other leaks.unreferenced object 0xffff8ed5029bfb28 (size 8): comm "(udev-worker)", pid 464, jiffies 4294672444 hex dump (first 8 bytes): 7c b4 d4 79 01 59 36 6c |..y.Y6l backtrace (crc 4461fdb7): __kmalloc_cache_noprof+0x31b/0x410 r535_gsp_acpi_jt+0x7c/0x110 [nouveau] r535_gsp_set_system_info+0x153/0x390 [nouveau] r535_gsp_oneinit+0xa4d/0xf50 [nouveau] tu102_gsp_oneinit+0x124/0x440 [nouveau] nvkm_subdev_oneinit_+0x3f/0x90 [nouveau] nvkm_subdev_init_+0x33/0xa0 [nouveau] nvkm_subdev_init+0x46/0x60 [nouveau] nvkm_device_init+0x167/0x1f0 [nouveau] nvkm_udevice_init+0x4b/0x70 [nouveau] nvkm_object_init+0x3a/0x110 [nouveau] nvkm_ioctl_new+0x13a/0x200 [nouveau] nvkm_ioctl+0x9f/0x140 [nouveau] nvif_object_ctor+0x11a/0x1a0 [nouveau] nvif_device_ctor+0x2a/0x80 [nouveau] nouveau_drm_device_new+0x157/0x2e0 [nouveau] unreferenced object 0xffff8ed502a37580 (size 32): comm "(udev-worker)", pid 464, jiffies 4294672444 hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace (crc f1da05aa): __kmalloc_noprof+0x3ac/0x500 acpi_ut_initialize_buffer+0x67/0xc0 acpi_evaluate_object+0x272/0x2c0 acpi_evaluate_dsm+0xb4/0x120 r535_gsp_acpi_jt+0xa3/0x110 [nouveau] r535_gsp_set_system_info+0x153/0x390 [nouveau] r535_gsp_oneinit+0xa4d/0xf50 [nouveau] tu102_gsp_oneinit+0x124/0x440 [nouveau] nvkm_subdev_oneinit_+0x3f/0x90 [nouveau] nvkm_subdev_init_+0x33/0xa0 [nouveau] nvkm_subdev_init+0x46/0x60 [nouveau] nvkm_device_init+0x167/0x1f0 [nouveau] nvkm_udevice_init+0x4b/0x70 [nouveau] nvkm_object_init+0x3a/0x110 [nouveau] nvkm_ioctl_new+0x13a/0x200 [nouveau] nvkm_ioctl+0x9f/0x140 [nouveau] unreferenced object 0xffff8ed5029bf1c0 (size 8): comm "(udev-worker)", pid 464, jiffies 4294672446 hex dump (first 8 bytes): cc bb d4 79 01 59 3c 84 ...y.Y<. backtrace (crc 30e1d939): __kmalloc_cache_noprof+0x31b/0x410 r535_gsp_acpi_caps+0x7e/0x120 [nouveau] r535_gsp_set_system_info+0x162/0x390 [nouveau] r535_gsp_oneinit+0xa4d/0xf50 [nouveau] tu102_gsp_oneinit+0x124/0x440 [nouveau] nvkm_subdev_oneinit_+0x3f/0x90 [nouveau] nvkm_subdev_init_+0x33/0xa0 [nouveau] nvkm_subdev_init+0x46/0x60 [nouveau] nvkm_device_init+0x167/0x1f0 [nouveau] nvkm_udevice_init+0x4b/0x70 [nouveau] nvkm_object_init+0x3a/0x110 [nouveau] nvkm_ioctl_new+0x13a/0x200 [nouveau] nvkm_ioctl+0x9f/0x140 [nouveau] nvif_object_ctor+0x11a/0x1a0 [nouveau] nvif_device_ctor+0x2a/0x80 [nouveau] nouveau_drm_device_new+0x157/0x2e0 [nouveau]