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. 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;
