The userq manager is embedded in fpriv, so any code paths trigger kfree(fpriv) would destroy the userq_mgr, even some of its queues are still alive, resuting in userq->userq_mgr use-after-free issues.
This commit fixes this problem by introduce a new counter refs representing for the number of its queues, and only free the userq_manager when refs == 0 Signed-off-by: Zhu Lingshan <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 27 +++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 19 ++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index 24adad7be251..434384b53b85 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c @@ -533,6 +533,17 @@ amdgpu_userq_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr, return r; } +static void amdgpu_userq_mgr_inc_refs(struct amdgpu_userq_mgr *uq_mgr) +{ + atomic_inc(&uq_mgr->refs); +} + +static void amdgpu_userq_mgr_dec_refs(struct amdgpu_userq_mgr *uq_mgr) +{ + if (atomic_dec_and_test(&uq_mgr->refs)) + wake_up(&uq_mgr->refs_wq); +} + static int amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue) { @@ -594,6 +605,8 @@ static void amdgpu_userq_kref_destroy(struct kref *kref) r = amdgpu_userq_destroy(uq_mgr, queue); if (r) drm_file_err(uq_mgr->file, "Failed to destroy usermode queue %d\n", r); + + amdgpu_userq_mgr_dec_refs(uq_mgr); } struct amdgpu_usermode_queue *amdgpu_userq_get(struct amdgpu_userq_mgr *uq_mgr, u32 qid) @@ -677,6 +690,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args) queue->xcp_id = (fpriv->xcp_id != AMDGPU_XCP_NO_PARTITION) ? fpriv->xcp_id : 0; queue->userq_mgr = uq_mgr; + amdgpu_userq_mgr_inc_refs(uq_mgr); INIT_DELAYED_WORK(&queue->hang_detect_work, amdgpu_userq_hang_detect_work); @@ -789,6 +803,7 @@ amdgpu_userq_create(struct drm_file *filp, union drm_amdgpu_userq *args) free_queue: trace_amdgpu_userq_create_end(queue, r); kfree(queue); + amdgpu_userq_mgr_dec_refs(uq_mgr); err_pm_runtime: pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); return r; @@ -1277,6 +1292,8 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *f { mutex_init(&userq_mgr->userq_mutex); xa_init_flags(&userq_mgr->userq_xa, XA_FLAGS_ALLOC); + atomic_set(&userq_mgr->refs, 0); + init_waitqueue_head(&userq_mgr->refs_wq); userq_mgr->adev = adev; userq_mgr->file = file_priv; userq_mgr->proc_ctx_allocated = false; @@ -1326,6 +1343,16 @@ void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr) amdgpu_userq_put(queue); } + /* + * userq_mgr is embedded in struct amdgpu_fpriv and get + * freed when kfree(fpriv). This wait_evet() blocks + * amdgpu_userq_mgr_fini() which is drm_release path, + * so keep userq_mgr alive while any queues holding it. + * + * This prevents queue->userq_mgr use-after-free issues. + */ + wait_event(userq_mgr->refs_wq, !atomic_read(&userq_mgr->refs)); + xa_destroy(&userq_mgr->userq_xa); /* diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h index 6412a7f7b6ef..7141383ea2ae 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h @@ -24,6 +24,8 @@ #ifndef AMDGPU_USERQ_H_ #define AMDGPU_USERQ_H_ +#include <linux/wait.h> + #include "amdgpu_eviction_fence.h" #define AMDGPU_MAX_USERQ_COUNT 512 @@ -126,6 +128,23 @@ struct amdgpu_userq_mgr { */ struct xarray userq_xa; struct mutex userq_mutex; + + /** + * @refs: + * + * Each queue increase this counter when join this manager, + * and decreash this counter when leave this manager. + */ + atomic_t refs; + + /** + * @refs_wq: + * + * Waited on by amdgpu_userq_mgr_fini() until @refs drops to zero. + * This wait prevents userq->userq_mgr UAF + */ + wait_queue_head_t refs_wq; + struct amdgpu_device *adev; struct delayed_work resume_work; struct drm_file *file; -- 2.53.0
