[PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Junrui Luo]

In mes_userq_mqd_create(), the memdup_user() allocations for
IP-specific MQD structs are not freed when subsequent VA validation
fails. The goto free_mqd label only cleans up the MQD BO object and
userq_props.

Fix by adding kfree() before each goto free_mqd on VA validation
failure in the COMPUTE, GFX, and SDMA branches.

Fixes: 9e46b8bb0539 ("drm/amdgpu: validate userq buffer virtual address and size")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
index 8c74894254f7..faac21ee5739 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
@@ -324,8 +324,10 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 
 		r = amdgpu_userq_input_va_validate(adev, queue, compute_mqd->eop_va,
 						   2048);
-		if (r)
+		if (r) {
+			kfree(compute_mqd);
 			goto free_mqd;
+		}
 
 		userq_props->eop_gpu_addr = compute_mqd->eop_va;
 		userq_props->hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_NORMAL;
@@ -365,12 +367,16 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 
 		r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11->shadow_va,
 						   shadow_info.shadow_size);
-		if (r)
+		if (r) {
+			kfree(mqd_gfx_v11);
 			goto free_mqd;
+		}
 		r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11->csa_va,
 						   shadow_info.csa_size);
-		if (r)
+		if (r) {
+			kfree(mqd_gfx_v11);
 			goto free_mqd;
+		}
 
 		kfree(mqd_gfx_v11);
 	} else if (queue->queue_type == AMDGPU_HW_IP_DMA) {
@@ -390,8 +396,10 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
 		}
 		r = amdgpu_userq_input_va_validate(adev, queue, mqd_sdma_v11->csa_va,
 						   32);
-		if (r)
+		if (r) {
+			kfree(mqd_sdma_v11);
 			goto free_mqd;
+		}
 
 		userq_props->csa_addr = mqd_sdma_v11->csa_va;
 		kfree(mqd_sdma_v11);

---
base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
change-id: 20260314-fixes-f4411ac85e22

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Markus Elfring]

…
> Fix by adding kfree() before each goto free_mqd on VA validation
> failure in the COMPUTE, GFX, and SDMA branches.

How do you think about to benefit any more from application of an attribute
like __free(kfree)?
https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/cleanup.h#L157-L161

Regards,
Markus

Re: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Junrui Luo]

On Sat, Mar 14, 2026 at 09:04:08PM +0100, Markus Elfring wrote:
> …
> > Fix by adding kfree() before each goto free_mqd on VA validation
> > failure in the COMPUTE, GFX, and SDMA branches.
> 
> How do you think about to benefit any more from application of an attribute
> like __free(kfree)?
> https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/cleanup.h#L157-L161

Hi Markus,

Thanks for the suggestion. I considered __free(kfree) but the cleanup.h
comment says scope-based cleanup and "goto" should not be mixed in the same
function. Since mes_userq_mqd_create() relies heavily on goto-based
unwinding, applying __free(kfree) only to the memdup pointers would
violate that guideline.

A full conversion to scope-based cleanup would require restructuring
the entire function, which seems beyond the scope of a bug fix.

Thanks,
Junrui Luo

Re: drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Markus Elfring]

> A full conversion to scope-based cleanup would require restructuring
> the entire function, which seems beyond the scope of a bug fix.

If you would like to stick to the usage of goto labels so far,
I see further possibilities to avoid also duplicate source code for
the affected implementation of the function “mes_userq_mqd_create”.
https://elixir.bootlin.com/linux/v7.0-rc3/source/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c#L275-L434

Regards,
Markus

RE: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Liang, Prike]

[Public]

Thanks for the fix. We could further refine this by wrapping a unified helper for fetching and validating the userq MQD raw data.

Reviewed-by: Prike Liang <Prike.Liang@amd.com>

Regards,
      Prike

> -----Original Message-----
> From: Junrui Luo <moonafterrain@outlook.com>
> Sent: Saturday, March 14, 2026 11:34 PM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; Liang, Prike <Prike.Liang@amd.com>
> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; Yuhao Jiang <danisjiang@gmail.com>;
> stable@vger.kernel.org; Junrui Luo <moonafterrain@outlook.com>
> Subject: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths
>
> [Some people who received this message don't often get email from
> moonafterrain@outlook.com. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> In mes_userq_mqd_create(), the memdup_user() allocations for IP-specific MQD
> structs are not freed when subsequent VA validation fails. The goto free_mqd label
> only cleans up the MQD BO object and userq_props.
>
> Fix by adding kfree() before each goto free_mqd on VA validation failure in the
> COMPUTE, GFX, and SDMA branches.
>
> Fixes: 9e46b8bb0539 ("drm/amdgpu: validate userq buffer virtual address and size")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> index 8c74894254f7..faac21ee5739 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> @@ -324,8 +324,10 @@ static int mes_userq_mqd_create(struct
> amdgpu_usermode_queue *queue,
>
>                 r = amdgpu_userq_input_va_validate(adev, queue, compute_mqd-
> >eop_va,
>                                                    2048);
> -               if (r)
> +               if (r) {
> +                       kfree(compute_mqd);
>                         goto free_mqd;
> +               }
>
>                 userq_props->eop_gpu_addr = compute_mqd->eop_va;
>                 userq_props->hqd_pipe_priority =
> AMDGPU_GFX_PIPE_PRIO_NORMAL; @@ -365,12 +367,16 @@ static int
> mes_userq_mqd_create(struct amdgpu_usermode_queue *queue,
>
>                 r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11-
> >shadow_va,
>                                                    shadow_info.shadow_size);
> -               if (r)
> +               if (r) {
> +                       kfree(mqd_gfx_v11);
>                         goto free_mqd;
> +               }
>                 r = amdgpu_userq_input_va_validate(adev, queue, mqd_gfx_v11->csa_va,
>                                                    shadow_info.csa_size);
> -               if (r)
> +               if (r) {
> +                       kfree(mqd_gfx_v11);
>                         goto free_mqd;
> +               }
>
>                 kfree(mqd_gfx_v11);
>         } else if (queue->queue_type == AMDGPU_HW_IP_DMA) { @@ -390,8
> +396,10 @@ static int mes_userq_mqd_create(struct amdgpu_usermode_queue
> *queue,
>                 }
>                 r = amdgpu_userq_input_va_validate(adev, queue, mqd_sdma_v11-
> >csa_va,
>                                                    32);
> -               if (r)
> +               if (r) {
> +                       kfree(mqd_sdma_v11);
>                         goto free_mqd;
> +               }
>
>                 userq_props->csa_addr = mqd_sdma_v11->csa_va;
>                 kfree(mqd_sdma_v11);
>
> ---
> base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
> change-id: 20260314-fixes-f4411ac85e22
>
> Best regards,
> --
> Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Junrui Luo]

On Sun, Mar 15, 2026 at 10:50:44AM +0100, Markus Elfring wrote:
> If you would like to stick to the usage of goto labels so far,
> I see further possibilities to avoid also duplicate source code for
> the affected implementation of the function “mes_userq_mqd_create”.
> https://elixir.bootlin.com/linux/v7.0-rc3/source/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c#L275-L434


On Mon, Mar 16, 2026 at 03:32:34AM +0000, Liang, Prike wrote:
> Thanks for the fix. We could further refine this by wrapping a unified helper for fetching and validating the userq MQD raw data.
 
Thanks for the review and suggestions.

I'm thinking of a follow-up patch that splits the branches into separate
helper functions. Each function would use __free(kfree) to manage the
memdup lifetime internally. The main function would only dispatch and
forward errors to the existing goto chain.

For instance:

static void *mes_userq_mqd_read(struct drm_amdgpu_userq_in *mqd_user,
				size_t size, const char *ip_name)
{
	void *mqd;

	if (mqd_user->mqd_size != size || !mqd_user->mqd) {
		DRM_ERROR("Invalid %s MQD\n", ip_name);
		return ERR_PTR(-EINVAL);
	}

	mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd), size);
	if (IS_ERR(mqd)) {
		DRM_ERROR("Failed to read %s user MQD\n", ip_name);
		return ERR_PTR(-ENOMEM);
	}

	return mqd;
}

static int mes_userq_mqd_init_compute(struct amdgpu_device *adev,
				      struct amdgpu_usermode_queue *queue,
				      struct drm_amdgpu_userq_in *mqd_user,
				      struct amdgpu_mqd_prop *userq_props)
{
	struct drm_amdgpu_userq_mqd_compute_gfx11 *mqd __free(kfree) = NULL;
	int r;

	mqd = mes_userq_mqd_read(mqd_user, sizeof(*mqd), "compute");
	if (IS_ERR(mqd))
		return PTR_ERR(mqd);

	r = amdgpu_userq_input_va_validate(adev, queue, mqd->eop_va, 2048);
	if (r)
		return r;

	userq_props->eop_gpu_addr = mqd->eop_va;
	userq_props->hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_NORMAL;
	userq_props->hqd_queue_priority = AMDGPU_GFX_QUEUE_PRIORITY_MINIMUM;
	userq_props->hqd_active = false;
	userq_props->tmz_queue =
		mqd_user->flags & AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
	return 0;
}

/* similarly for mes_userq_mqd_init_gfx/sdma */

Then in mes_userq_mqd_create():

if (queue->queue_type == AMDGPU_HW_IP_COMPUTE)
	r = mes_userq_mqd_init_compute(adev, queue, mqd_user,
				       userq_props);
else if (queue->queue_type == AMDGPU_HW_IP_GFX)
	r = mes_userq_mqd_init_gfx(adev, queue, mqd_user, userq_props);
else if (queue->queue_type == AMDGPU_HW_IP_DMA)
	r = mes_userq_mqd_init_sdma(adev, queue, mqd_user, userq_props);

if (r)
	goto free_mqd;

Would this direction be acceptable as a follow-up?

Thanks,
Junrui Luo

Re: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Markus Elfring]

…
> static int mes_userq_mqd_init_compute(struct amdgpu_device *adev,
> 				      struct amdgpu_usermode_queue *queue,
> 				      struct drm_amdgpu_userq_in *mqd_user,
> 				      struct amdgpu_mqd_prop *userq_props)
> {
> 	struct drm_amdgpu_userq_mqd_compute_gfx11 *mqd __free(kfree) = NULL;
…

How do you think about to use a direct assignment without the variable initialisation “NULL”?

Regards,
Markus

RE: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error paths

[Liang, Prike]

[Public]

It seems reasonable to introduce a unified helper that encapsulates fetching and validating the raw MQD data for user queues, so that this logic is not duplicated across call sites.

Regards,
      Prike

> -----Original Message-----
> From: Junrui Luo <moonafterrain@outlook.com>
> Sent: Monday, March 16, 2026 6:10 PM
> To: Liang, Prike <Prike.Liang@amd.com>; Markus.Elfring@web.de
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; David Airlie <airlied@gmail.com>; Simona Vetter
> <simona@ffwll.ch>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> linux-kernel@vger.kernel.org; Yuhao Jiang <danisjiang@gmail.com>;
> stable@vger.kernel.org
> Subject: Re: [PATCH] drm/amdgpu/userq: fix memory leak in MQD creation error
> paths
>
> On Sun, Mar 15, 2026 at 10:50:44AM +0100, Markus Elfring wrote:
> > If you would like to stick to the usage of goto labels so far, I see
> > further possibilities to avoid also duplicate source code for the
> > affected implementation of the function “mes_userq_mqd_create”.
> > https://elixir.bootlin.com/linux/v7.0-rc3/source/drivers/gpu/drm/amd/a
> > mdgpu/mes_userqueue.c#L275-L434
>
>
> On Mon, Mar 16, 2026 at 03:32:34AM +0000, Liang, Prike wrote:
> > Thanks for the fix. We could further refine this by wrapping a unified helper for
> fetching and validating the userq MQD raw data.
>
> Thanks for the review and suggestions.
>
> I'm thinking of a follow-up patch that splits the branches into separate helper
> functions. Each function would use __free(kfree) to manage the memdup lifetime
> internally. The main function would only dispatch and forward errors to the existing
> goto chain.
>
> For instance:
>
> static void *mes_userq_mqd_read(struct drm_amdgpu_userq_in *mqd_user,
>                               size_t size, const char *ip_name)
> {
>       void *mqd;
>
>       if (mqd_user->mqd_size != size || !mqd_user->mqd) {
>               DRM_ERROR("Invalid %s MQD\n", ip_name);
>               return ERR_PTR(-EINVAL);
>       }
>
>       mqd = memdup_user(u64_to_user_ptr(mqd_user->mqd), size);
>       if (IS_ERR(mqd)) {
>               DRM_ERROR("Failed to read %s user MQD\n", ip_name);
>               return ERR_PTR(-ENOMEM);
>       }
>
>       return mqd;
> }
>
> static int mes_userq_mqd_init_compute(struct amdgpu_device *adev,
>                                     struct amdgpu_usermode_queue *queue,
>                                     struct drm_amdgpu_userq_in *mqd_user,
>                                     struct amdgpu_mqd_prop *userq_props) {
>       struct drm_amdgpu_userq_mqd_compute_gfx11 *mqd __free(kfree) = NULL;
>       int r;
>
>       mqd = mes_userq_mqd_read(mqd_user, sizeof(*mqd), "compute");
>       if (IS_ERR(mqd))
>               return PTR_ERR(mqd);
>
>       r = amdgpu_userq_input_va_validate(adev, queue, mqd->eop_va, 2048);
>       if (r)
>               return r;
>
>       userq_props->eop_gpu_addr = mqd->eop_va;
>       userq_props->hqd_pipe_priority = AMDGPU_GFX_PIPE_PRIO_NORMAL;
>       userq_props->hqd_queue_priority =
> AMDGPU_GFX_QUEUE_PRIORITY_MINIMUM;
>       userq_props->hqd_active = false;
>       userq_props->tmz_queue =
>               mqd_user->flags &
> AMDGPU_USERQ_CREATE_FLAGS_QUEUE_SECURE;
>       return 0;
> }
>
> /* similarly for mes_userq_mqd_init_gfx/sdma */
>
> Then in mes_userq_mqd_create():
>
> if (queue->queue_type == AMDGPU_HW_IP_COMPUTE)
>       r = mes_userq_mqd_init_compute(adev, queue, mqd_user,
>                                      userq_props);
> else if (queue->queue_type == AMDGPU_HW_IP_GFX)
>       r = mes_userq_mqd_init_gfx(adev, queue, mqd_user, userq_props); else if
> (queue->queue_type == AMDGPU_HW_IP_DMA)
>       r = mes_userq_mqd_init_sdma(adev, queue, mqd_user, userq_props);
>
> if (r)
>       goto free_mqd;
>
> Would this direction be acceptable as a follow-up?
>
> Thanks,
> Junrui Luo