On 8/28/2026 9:09 PM, Christian König wrote: > On 8/28/26 11:53, Zhu Lingshan wrote: >> The life cycle of a user queue is managed by its >> kref. However when destroy a userq manager, >> the kref_put of its queues in amdgpu_userq_mgr_fini >> may not be the last put, therefore the queues >> could be still alive after the userq manager >> has been destroyed, resulting 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 > Clear NAK to that one as well, this is just nonsense.
It could be better to have some explanations. I am not sure how to guarantee the put_kref in amdgpu_userq_mgr_fini is the last put and result in kref == 0, if not the last one, there can be userq->userq_mgr UAF bugs. Thanks Lingshan > > Christian. > >> Signed-off-by: Zhu Lingshan <[email protected]> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 30 +++++++++++++++++++++++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 9 +++++++ >> 2 files changed, 39 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c >> index e0639f844a8e..f398986a61a5 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c >> @@ -27,6 +27,7 @@ >> #include <linux/pm_runtime.h> >> #include <linux/overflow.h> >> #include <drm/drm_drv.h> >> +#include <linux/wait_bit.h> >> >> #include "amdgpu.h" >> #include "amdgpu_reset.h" >> @@ -533,6 +534,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_var(&uq_mgr->refs); >> +} >> + >> static int >> amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, struct >> amdgpu_usermode_queue *queue) >> { >> @@ -594,6 +606,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) >> @@ -707,6 +721,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); >> >> @@ -819,6 +834,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; >> @@ -1331,6 +1347,7 @@ 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); >> userq_mgr->adev = adev; >> userq_mgr->file = file_priv; >> userq_mgr->proc_ctx_allocated = false; >> @@ -1380,6 +1397,19 @@ void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr >> *userq_mgr) >> amdgpu_userq_put(queue); >> } >> >> + /* >> + * The above amdgpu_userq_put() may not be the last put >> + * of the kref of a user queue, therefore there could >> + * be some queues still alive even when the userq manager >> + * has been destroyed. This wait_evet() blocks >> + * amdgpu_userq_mgr_fini(), so keep userq_mgr alive >> + * while any queues holding it. >> + * >> + * This prevents queue->userq_mgr use-after-free issues. >> + */ >> + wait_var_event(&userq_mgr->refs, >> + !atomic_read_acquire(&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 8fc73862f64e..a13d8d4dd5c7 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h >> @@ -126,6 +126,15 @@ struct amdgpu_userq_mgr { >> */ >> struct xarray userq_xa; >> struct mutex userq_mutex; >> + >> + /** >> + * @refs: >> + * >> + * Each queue increases this counter when join this manager, >> + * and decreases it when leave this manager. >> + */ >> + atomic_t refs; >> + >> struct amdgpu_device *adev; >> struct delayed_work resume_work; >> struct drm_file *file;
