Skip to content

Commit 2d2be27

Browse files
matt-auldlucasdemarchi
authored andcommitted
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: dd08ebf ("drm/xe: Introduce a new DRM driver for Intel GPUs") Signed-off-by: Matthew Auld <[email protected]> Cc: Matthew Brost <[email protected]> Cc: <[email protected]> # v6.8+ Reviewed-by: Matthew Brost <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 8611086) Signed-off-by: Lucas De Marchi <[email protected]>
1 parent 790533e commit 2d2be27

File tree

4 files changed

+35
-2
lines changed

4 files changed

+35
-2
lines changed

drivers/gpu/drm/xe/xe_device.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,9 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy)
298298
if (xe->unordered_wq)
299299
destroy_workqueue(xe->unordered_wq);
300300

301+
if (xe->destroy_wq)
302+
destroy_workqueue(xe->destroy_wq);
303+
301304
ttm_device_fini(&xe->ttm);
302305
}
303306

@@ -363,8 +366,9 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
363366
xe->preempt_fence_wq = alloc_ordered_workqueue("xe-preempt-fence-wq", 0);
364367
xe->ordered_wq = alloc_ordered_workqueue("xe-ordered-wq", 0);
365368
xe->unordered_wq = alloc_workqueue("xe-unordered-wq", 0, 0);
369+
xe->destroy_wq = alloc_workqueue("xe-destroy-wq", 0, 0);
366370
if (!xe->ordered_wq || !xe->unordered_wq ||
367-
!xe->preempt_fence_wq) {
371+
!xe->preempt_fence_wq || !xe->destroy_wq) {
368372
/*
369373
* Cleanup done in xe_device_destroy via
370374
* drmm_add_action_or_reset register above

drivers/gpu/drm/xe/xe_device_types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,9 @@ struct xe_device {
396396
/** @unordered_wq: used to serialize unordered work, mostly display */
397397
struct workqueue_struct *unordered_wq;
398398

399+
/** @destroy_wq: used to serialize user destroy work, like queue */
400+
struct workqueue_struct *destroy_wq;
401+
399402
/** @tiles: device tiles */
400403
struct xe_tile tiles[XE_MAX_TILES_PER_DEVICE];
401404

drivers/gpu/drm/xe/xe_guc_submit.c

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,26 @@ static struct workqueue_struct *get_submit_wq(struct xe_guc *guc)
276276
}
277277
#endif
278278

279+
static void xe_guc_submit_fini(struct xe_guc *guc)
280+
{
281+
struct xe_device *xe = guc_to_xe(guc);
282+
struct xe_gt *gt = guc_to_gt(guc);
283+
int ret;
284+
285+
ret = wait_event_timeout(guc->submission_state.fini_wq,
286+
xa_empty(&guc->submission_state.exec_queue_lookup),
287+
HZ * 5);
288+
289+
drain_workqueue(xe->destroy_wq);
290+
291+
xe_gt_assert(gt, ret);
292+
}
293+
279294
static void guc_submit_fini(struct drm_device *drm, void *arg)
280295
{
281296
struct xe_guc *guc = arg;
282297

298+
xe_guc_submit_fini(guc);
283299
xa_destroy(&guc->submission_state.exec_queue_lookup);
284300
free_submit_wq(guc);
285301
}
@@ -351,6 +367,8 @@ int xe_guc_submit_init(struct xe_guc *guc, unsigned int num_ids)
351367

352368
xa_init(&guc->submission_state.exec_queue_lookup);
353369

370+
init_waitqueue_head(&guc->submission_state.fini_wq);
371+
354372
primelockdep(guc);
355373

356374
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
367385

368386
xe_guc_id_mgr_release_locked(&guc->submission_state.idm,
369387
q->guc->id, q->width);
388+
389+
if (xa_empty(&guc->submission_state.exec_queue_lookup))
390+
wake_up(&guc->submission_state.fini_wq);
370391
}
371392

372393
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)
12741295

12751296
static void guc_exec_queue_fini_async(struct xe_exec_queue *q)
12761297
{
1298+
struct xe_guc *guc = exec_queue_to_guc(q);
1299+
struct xe_device *xe = guc_to_xe(guc);
1300+
12771301
INIT_WORK(&q->guc->fini_async, __guc_exec_queue_fini_async);
12781302

12791303
/* We must block on kernel engines so slabs are empty on driver unload */
12801304
if (q->flags & EXEC_QUEUE_FLAG_PERMANENT || exec_queue_wedged(q))
12811305
__guc_exec_queue_fini_async(&q->guc->fini_async);
12821306
else
1283-
queue_work(system_wq, &q->guc->fini_async);
1307+
queue_work(xe->destroy_wq, &q->guc->fini_async);
12841308
}
12851309

12861310
static void __guc_exec_queue_fini(struct xe_guc *guc, struct xe_exec_queue *q)

drivers/gpu/drm/xe/xe_guc_types.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ struct xe_guc {
8181
#endif
8282
/** @submission_state.enabled: submission is enabled */
8383
bool enabled;
84+
/** @submission_state.fini_wq: submit fini wait queue */
85+
wait_queue_head_t fini_wq;
8486
} submission_state;
8587
/** @hwconfig: Hardware config state */
8688
struct {

0 commit comments

Comments
 (0)