thr3ads.net - Nouveau - [PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init [Jun 2025]

If this information is useful, please help other people find it:
Share via:

Ben Skeggs

2025-Jun-17 04:00 UTC

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

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.

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);
-- 
2.49.0

Philipp Stanner

2025-Jun-17 11:29 UTC

head link

[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.
> 
> Reported-by: Danilo Krummrich <dakr at kernel.org>
> Fixes: 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting
> GSP-RM")
Needs to +Cc the stable kernel

But, see below

> 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
This seems to be based on a code move that is not yet in mainline.

Therefore, backporting the bugfix to stable seems difficult. Since that
code move is already in drm-misc-next, it would seem that it can only
be solved with two distinct patches for stable and for -next.

But this needs to be judged by a maintainer.
> @@ -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;
> +
This could be done immediately after the creation of argv4. That way
it's more difficult to have the leak again if something is inserted
later on.
> ?	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:
'done' seems like a bad name considering that the operations are
aborted with a WARN_ON above. Better 'abort' or sth like that.


P.
> ?	ACPI_FREE(obj);
> ?
> ?	kfree(argv4.buffer.pointer);

Philipp Stanner

2025-Jul-07 08:27 UTC

head link

[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:59 UTC

head link

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

On 6/17/25 6:00 AM, 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.
> 
> 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>
Applied to drm-misc-fixes, thanks!

Philipp Stanner

2025-Aug-05 09:16 UTC

head link

[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.
> 
> 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>
Tested-by: Philipp Stanner <phasta at kernel.org>


This

Closes: https://www.spinics.net/lists/nouveau/msg16319.html



P.
> ---
> ?.../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);

Nouveau - Jun 2025 - [PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init

[PATCH] drm/nouveau/gsp: fix potential leak of memory used during acpi init