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