drm/amdgpu: rework userq fence driver alloc/destroy

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.

v2: clean up the local variables initialization. (Christian)

Signed-off-by: Prike Liang <Prike.Liang@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 49e7881750fa7cfe96af51d344f673fc5000c12e..8bc591deb546d10ce5f7c5bb37a8fa9f17153e63 100644 (file)
--- 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 824ac0a0549f7809ca75f68fc3f281413ae8a289..584c9ec28bf110280d2ee19267f176db23e2dc91 100644 (file)
--- 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 fe6d83e859a076ad03fc6fdc6edbec7afffc256a..5e1336e993e0e15da13d3d885646333d6813535e 100644 (file)
--- 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_obj(*fence_drv);
@@ -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 ae39b9e1f7d65a56522ec1ae03d93743671e7654..5097de940a1954877a971a2ab740eda98f5d6107 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -6502,14 +6502,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;
+               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 a418ae609c3634aadb0635c4b24ad776bfd0e79f..65c33823a688fcbc4ad2d9b82312f35ea0cad6b1 100644 (file)
--- 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;
                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 db49582a211f76229b7267feaf1f8880271b1aad..68fd3c04134d8e2258d4a1c898092275553caa2e 100644 (file)
--- 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;
                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 b005672f2f96b6b0eacd447c83e261195ff436da..0f530bb8a9a36044c705dfc3501f0fa62079a107 100644 (file)
--- 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;
+               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 5679a94d0815eae42d76d4b8b65ebbec2315aa51..9ed817b69a3b74f803d0545bab22c6160cc98cee 100644 (file)
--- 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;
                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);
        }