drm/xe: fix UAF around queue destruction

We currently do stuff like queuing the final destruction step on a
random system wq, which will outlive the driver instance. With bad
timing we can teardown the driver with one or more work workqueue still
being alive leading to various UAF splats. Add a fini step to ensure
user queues are properly torn down. At this point GuC should already be
nuked so queue itself should no longer be referenced from hw pov.

v2 (Matt B)
 - Looks much safer to use a waitqueue and then just wait for the
   xa_array to become empty before triggering the drain.

Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2317
Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: <stable@vger.kernel.org> # v6.8+
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240923145647.77707-2-matthew.auld@intel.com
(cherry picked from commit 861108666cc0e999cffeab6aff17b662e68774e3)
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 70d4e4d46c3c86592f0e984d725778090af99222..74e593caf87c7bc53ed9759c60f26847e36d3d11 100644 (file)
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -298,6 +298,9 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy)
        if (xe->unordered_wq)
                destroy_workqueue(xe->unordered_wq);
 
+       if (xe->destroy_wq)
+               destroy_workqueue(xe->destroy_wq);
+
        ttm_device_fini(&xe->ttm);
 }
 
@@ -363,8 +366,9 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
        xe->preempt_fence_wq = alloc_ordered_workqueue("xe-preempt-fence-wq", 0);
        xe->ordered_wq = alloc_ordered_workqueue("xe-ordered-wq", 0);
        xe->unordered_wq = alloc_workqueue("xe-unordered-wq", 0, 0);
+       xe->destroy_wq = alloc_workqueue("xe-destroy-wq", 0, 0);
        if (!xe->ordered_wq || !xe->unordered_wq ||
-           !xe->preempt_fence_wq) {
+           !xe->preempt_fence_wq || !xe->destroy_wq) {
                /*
                 * Cleanup done in xe_device_destroy via
                 * drmm_add_action_or_reset register above

diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
index ec7eb781112649b87dcc4541741c34faf8c5d2b5..24c8c2d20676ff0da80ff87a5ad05e0573d88c44 100644 (file)
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -396,6 +396,9 @@ struct xe_device {
        /** @unordered_wq: used to serialize unordered work, mostly display */
        struct workqueue_struct *unordered_wq;
 
+       /** @destroy_wq: used to serialize user destroy work, like queue */
+       struct workqueue_struct *destroy_wq;
+
        /** @tiles: device tiles */
        struct xe_tile tiles[XE_MAX_TILES_PER_DEVICE];
 

diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
index 715c761dc7d62f746acb5ea9ac40c149e5d57a3f..98a6a385a79646842dcbe8af5c0b2c65619cfe90 100644 (file)
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -276,10 +276,26 @@ static struct workqueue_struct *get_submit_wq(struct xe_guc *guc)
 }
 #endif
 
+static void xe_guc_submit_fini(struct xe_guc *guc)
+{
+       struct xe_device *xe = guc_to_xe(guc);
+       struct xe_gt *gt = guc_to_gt(guc);
+       int ret;
+
+       ret = wait_event_timeout(guc->submission_state.fini_wq,
+                                xa_empty(&guc->submission_state.exec_queue_lookup),
+                                HZ * 5);
+
+       drain_workqueue(xe->destroy_wq);
+
+       xe_gt_assert(gt, ret);
+}
+
 static void guc_submit_fini(struct drm_device *drm, void *arg)
 {
        struct xe_guc *guc = arg;
 
+       xe_guc_submit_fini(guc);
        xa_destroy(&guc->submission_state.exec_queue_lookup);
        free_submit_wq(guc);
 }
@@ -351,6 +367,8 @@ int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
 
        xa_init(&guc->submission_state.exec_queue_lookup);
 
+       init_waitqueue_head(&guc->submission_state.fini_wq);
+
        primelockdep(guc);
 
        return drmm_add_action_or_reset(&xe->drm, guc_submit_fini, guc);
@@ -367,6 +385,9 @@ static void __release_guc_id(struct xe_guc *guc, struct xe_exec_queue *q, u32 xa
 
        xe_guc_id_mgr_release_locked(&guc->submission_state.idm,
                                     q->guc->id, q->width);
+
+       if (xa_empty(&guc->submission_state.exec_queue_lookup))
+               wake_up(&guc->submission_state.fini_wq);
 }
 
 static int alloc_guc_id(struct xe_guc *guc, struct xe_exec_queue *q)
@@ -1274,13 +1295,16 @@ static void __guc_exec_queue_fini_async(struct work_struct *w)
 
 static void guc_exec_queue_fini_async(struct xe_exec_queue *q)
 {
+       struct xe_guc *guc = exec_queue_to_guc(q);
+       struct xe_device *xe = guc_to_xe(guc);
+
        INIT_WORK(&q->guc->fini_async, __guc_exec_queue_fini_async);
 
        /* We must block on kernel engines so slabs are empty on driver unload */
        if (q->flags & EXEC_QUEUE_FLAG_PERMANENT || exec_queue_wedged(q))
                __guc_exec_queue_fini_async(&q->guc->fini_async);
        else
-               queue_work(system_wq, &q->guc->fini_async);
+               queue_work(xe->destroy_wq, &q->guc->fini_async);
 }
 
 static void __guc_exec_queue_fini(struct xe_guc *guc, struct xe_exec_queue *q)

diff --git a/drivers/gpu/drm/xe/xe_guc_types.h b/drivers/gpu/drm/xe/xe_guc_types.h
index 546ac6350a31ffce73680cd217b9b8500aebeeb0..69046f698271747915d652feb336e9154dfd3aff 100644 (file)
--- a/drivers/gpu/drm/xe/xe_guc_types.h
+++ b/drivers/gpu/drm/xe/xe_guc_types.h
@@ -81,6 +81,8 @@ struct xe_guc {
 #endif
                /** @submission_state.enabled: submission is enabled */
                bool enabled;
+               /** @submission_state.fini_wq: submit fini wait queue */
+               wait_queue_head_t fini_wq;
        } submission_state;
        /** @hwconfig: Hardware config state */
        struct {