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

[Junrui Luo <moonafterrain@outlook.com>]

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