In amdgpu_userq_create(), once a newly created user queue has been assigned a qid and published to userq_xa, a concurrent AMDGPU_USERQ_OP_FREE ioctl can free the queue, resulting in use-after-free issues in amdgpu_userq_create.
This is surely a user space bug, but kernel should not crash. This commit fixes this issue by taking an additional kref before xa_alloc() Signed-off-by: Zhu Lingshan <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index 21a9a2138fc8..83e6c87a5940 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c @@ -871,22 +871,32 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args) mutex_unlock(&uq_mgr->userq_mutex); + /* + * A concurrent AMDGPU_USERQ_OP_FREE ioctl can free the queue once + * xa_alloc() publishes it and assigne a qid to it. + * Take a kref to avoid use-after-free issues. + */ + kref_get(&queue->refcount); r = xa_alloc(&uq_mgr->userq_xa, &qid, queue, XA_LIMIT(1, AMDGPU_MAX_USERQ_COUNT), GFP_KERNEL); if (r) { /* - * This drops the last reference which should take care of + * This drops the last two references which should take care of * all cleanup. */ trace_amdgpu_userq_create_end(queue, r); amdgpu_userq_put(queue); + amdgpu_userq_put(queue); + return r; } amdgpu_debugfs_userq_init(filp, queue, qid); trace_amdgpu_userq_create_end(queue, 0); args->out.queue_id = qid; + amdgpu_userq_put(queue); + return 0; clean_mqd: -- 2.53.0
