The correct fix is to tie the global xa entry lifetime to the queue lifetime: insert in amdgpu_userq_create() and erase in amdgpu_userq_cleanup(), both at the well-defined doorbell_index key, making the operation O(1) and resolve the fence driver UAF problem by binding the userq driver fence to per queue.
Signed-off-by: Prike Liang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 ----- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 +--- .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 20 +------------------ drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 10 +++++----- drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 10 +++++----- drivers/gpu/drm/amd/amdgpu/gfx_v12_1.c | 11 +++++----- drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 10 +++++----- drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 10 +++++----- 8 files changed, 28 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 49e7881750fa..8bc591deb546 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1045,11 +1045,6 @@ struct amdgpu_device { struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM]; const struct amdgpu_userq_funcs *userq_funcs[AMDGPU_HW_IP_NUM]; - /* xarray used to retrieve the user queue fence driver reference - * in the EOP interrupt handler to signal the particular user - * queue fence. - */ - struct xarray userq_xa; /** * @userq_doorbell_xa: Global user queue map (doorbell index → queue) * Key: doorbell_index (unique global identifier for the queue) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 0c0489395edf..a7b519f670a5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -3757,15 +3757,13 @@ int amdgpu_device_init(struct amdgpu_device *adev, spin_lock_init(&adev->virt.rlcg_reg_lock); spin_lock_init(&adev->wb.lock); - xa_init_flags(&adev->userq_xa, XA_FLAGS_LOCK_IRQ); - INIT_LIST_HEAD(&adev->reset_list); INIT_LIST_HEAD(&adev->ras_list); INIT_LIST_HEAD(&adev->pm.od_kobj_list); - xa_init(&adev->userq_doorbell_xa); + xa_init_flags(&adev->userq_doorbell_xa, XA_FLAGS_LOCK_IRQ); INIT_DELAYED_WORK(&adev->delayed_init_work, amdgpu_device_delayed_init_work_handler); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c index 6b33c2428b2d..87560c1251d8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -81,7 +81,6 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, struct amdgpu_usermode_queue *userq) { struct amdgpu_userq_fence_driver *fence_drv; - unsigned long flags; int r; fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL); @@ -104,19 +103,10 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev, fence_drv->context = dma_fence_context_alloc(1); get_task_comm(fence_drv->timeline_name, current); - xa_lock_irqsave(&adev->userq_xa, flags); - r = xa_err(__xa_store(&adev->userq_xa, userq->doorbell_index, - fence_drv, GFP_KERNEL)); - xa_unlock_irqrestore(&adev->userq_xa, flags); - if (r) - goto free_seq64; - userq->fence_drv = fence_drv; return 0; -free_seq64: - amdgpu_seq64_free(adev, fence_drv->va); free_fence_drv: kfree(fence_drv); @@ -187,11 +177,9 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref) struct amdgpu_userq_fence_driver *fence_drv = container_of(ref, struct amdgpu_userq_fence_driver, refcount); - struct amdgpu_userq_fence_driver *xa_fence_drv; struct amdgpu_device *adev = fence_drv->adev; struct amdgpu_userq_fence *fence, *tmp; - struct xarray *xa = &adev->userq_xa; - unsigned long index, flags; + unsigned long flags; struct dma_fence *f; spin_lock_irqsave(&fence_drv->fence_list_lock, flags); @@ -208,12 +196,6 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref) } spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags); - xa_lock_irqsave(xa, flags); - xa_for_each(xa, index, xa_fence_drv) - if (xa_fence_drv == fence_drv) - __xa_erase(xa, index); - xa_unlock_irqrestore(xa, flags); - /* Free seq64 memory */ amdgpu_seq64_free(adev, fence_drv->va); kfree(fence_drv); diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index 78d1f3eb522e..c0921977b853 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c @@ -6488,14 +6488,14 @@ static int gfx_v11_0_eop_irq(struct amdgpu_device *adev, DRM_DEBUG("IH: CP EOP\n"); if (adev->enable_mes && doorbell_offset) { - struct amdgpu_userq_fence_driver *fence_drv = NULL; - struct xarray *xa = &adev->userq_xa; + struct amdgpu_usermode_queue *queue = NULL; + struct xarray *xa = &adev->userq_doorbell_xa; unsigned long flags; xa_lock_irqsave(xa, flags); - fence_drv = xa_load(xa, doorbell_offset); - if (fence_drv) - amdgpu_userq_fence_driver_process(fence_drv); + queue = xa_load(xa, doorbell_offset); + if (queue) + amdgpu_userq_fence_driver_process(queue->fence_drv); xa_unlock_irqrestore(xa, flags); } else { me_id = (entry->ring_id & 0x0c) >> 2; diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c index a418ae609c36..d4faceab8f88 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c @@ -4854,14 +4854,14 @@ static int gfx_v12_0_eop_irq(struct amdgpu_device *adev, DRM_DEBUG("IH: CP EOP\n"); if (adev->enable_mes && doorbell_offset) { - struct amdgpu_userq_fence_driver *fence_drv = NULL; - struct xarray *xa = &adev->userq_xa; + struct xarray *xa = &adev->userq_doorbell_xa; + struct amdgpu_usermode_queue *queue = NULL; unsigned long flags; xa_lock_irqsave(xa, flags); - fence_drv = xa_load(xa, doorbell_offset); - if (fence_drv) - amdgpu_userq_fence_driver_process(fence_drv); + queue = xa_load(xa, doorbell_offset); + if (queue) + amdgpu_userq_fence_driver_process(queue->fence_drv); xa_unlock_irqrestore(xa, flags); } else { me_id = (entry->ring_id & 0x0c) >> 2; diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_1.c index db49582a211f..642ddd9473cb 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_1.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_1.c @@ -3643,14 +3643,15 @@ static int gfx_v12_1_eop_irq(struct amdgpu_device *adev, DRM_DEBUG("IH: CP EOP\n"); if (adev->enable_mes && doorbell_offset) { - struct amdgpu_userq_fence_driver *fence_drv = NULL; - struct xarray *xa = &adev->userq_xa; + struct xarray *xa = &adev->userq_doorbell_xa; + struct amdgpu_usermode_queue *queue = NULL; unsigned long flags; xa_lock_irqsave(xa, flags); - fence_drv = xa_load(xa, doorbell_offset); - if (fence_drv) - amdgpu_userq_fence_driver_process(fence_drv); + queue = xa_load(xa, doorbell_offset); + if (queue) + amdgpu_userq_fence_driver_process(queue->fence_drv); + xa_unlock_irqrestore(xa, flags); } else { me_id = (entry->ring_id & 0x0c) >> 2; diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c index b005672f2f96..301e9364aff1 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c @@ -1662,16 +1662,16 @@ static int sdma_v6_0_process_fence_irq(struct amdgpu_device *adev, u32 doorbell_offset = entry->src_data[0]; if (adev->enable_mes && doorbell_offset) { - struct amdgpu_userq_fence_driver *fence_drv = NULL; - struct xarray *xa = &adev->userq_xa; + struct amdgpu_usermode_queue *queue = NULL; + struct xarray *xa = &adev->userq_doorbell_xa; unsigned long flags; doorbell_offset >>= SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT; xa_lock_irqsave(xa, flags); - fence_drv = xa_load(xa, doorbell_offset); - if (fence_drv) - amdgpu_userq_fence_driver_process(fence_drv); + queue = xa_load(xa, doorbell_offset); + if (queue) + amdgpu_userq_fence_driver_process(queue->fence_drv); xa_unlock_irqrestore(xa, flags); } diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c index 5679a94d0815..2660e8f08daa 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c @@ -1594,16 +1594,16 @@ static int sdma_v7_0_process_fence_irq(struct amdgpu_device *adev, u32 doorbell_offset = entry->src_data[0]; if (adev->enable_mes && doorbell_offset) { - struct amdgpu_userq_fence_driver *fence_drv = NULL; - struct xarray *xa = &adev->userq_xa; + struct xarray *xa = &adev->userq_doorbell_xa; + struct amdgpu_usermode_queue *queue = NULL; unsigned long flags; doorbell_offset >>= SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT; xa_lock_irqsave(xa, flags); - fence_drv = xa_load(xa, doorbell_offset); - if (fence_drv) - amdgpu_userq_fence_driver_process(fence_drv); + queue = xa_load(xa, doorbell_offset); + if (queue) + amdgpu_userq_fence_driver_process(queue->fence_drv); xa_unlock_irqrestore(xa, flags); } -- 2.34.1
