From: Matthew Auld <matthew.auld@intel.com>
To: intel-xe@lists.freedesktop.org
Cc: Matthew Brost <matthew.brost@intel.com>, stable@vger.kernel.org
Subject: [PATCH] drm/xe: fix UAF around queue destruction
Date: Fri, 20 Sep 2024 18:26:00 +0100 [thread overview]
Message-ID: <20240920172559.208358-2-matthew.auld@intel.com> (raw)
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.
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+
---
drivers/gpu/drm/xe/xe_device.c | 6 +++++-
drivers/gpu/drm/xe/xe_device_types.h | 3 +++
drivers/gpu/drm/xe/xe_guc_submit.c | 32 +++++++++++++++++++++++++++-
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index cb5a9fd820cf..90b3478ed7cd 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -297,6 +297,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);
}
@@ -360,8 +363,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 5ad96d283a71..515385b916cc 100644
--- a/drivers/gpu/drm/xe/xe_device_types.h
+++ b/drivers/gpu/drm/xe/xe_device_types.h
@@ -422,6 +422,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 fbbe6a487bbb..66441efa0bcd 100644
--- a/drivers/gpu/drm/xe/xe_guc_submit.c
+++ b/drivers/gpu/drm/xe/xe_guc_submit.c
@@ -276,10 +276,37 @@ static struct workqueue_struct *get_submit_wq(struct xe_guc *guc)
}
#endif
+static void guc_exec_queue_fini_async(struct xe_exec_queue *q);
+
+static void xe_guc_submit_fini(struct xe_guc *guc)
+{
+ struct xe_device *xe = guc_to_xe(guc);
+ struct xe_exec_queue *q;
+ unsigned long index;
+
+ mutex_lock(&guc->submission_state.lock);
+ xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
+ struct xe_gpu_scheduler *sched = &q->guc->sched;
+
+ xe_assert(xe, !kref_read(&q->refcount));
+
+ xe_sched_submission_stop(sched);
+
+ if (exec_queue_registered(q) && !exec_queue_wedged(q))
+ guc_exec_queue_fini_async(q);
+ }
+ mutex_unlock(&guc->submission_state.lock);
+
+ drain_workqueue(xe->destroy_wq);
+
+ xe_assert(xe, xa_empty(&guc->submission_state.exec_queue_lookup));
+}
+
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);
}
@@ -1268,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)
--
2.46.0
next reply other threads:[~2024-09-20 17:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-20 17:26 Matthew Auld [this message]
2024-09-20 17:58 ` ✓ CI.Patch_applied: success for drm/xe: fix UAF around queue destruction Patchwork
2024-09-20 17:58 ` ✓ CI.checkpatch: " Patchwork
2024-09-20 17:59 ` ✓ CI.KUnit: " Patchwork
2024-09-20 18:11 ` ✓ CI.Build: " Patchwork
2024-09-20 18:13 ` ✓ CI.Hooks: " Patchwork
2024-09-20 18:15 ` ✓ CI.checksparse: " Patchwork
2024-09-20 18:33 ` ✓ CI.BAT: " Patchwork
2024-09-20 19:27 ` [PATCH] " Matthew Brost
2024-09-23 8:51 ` Matthew Auld
2024-09-20 23:57 ` ✗ CI.FULL: failure for " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240920172559.208358-2-matthew.auld@intel.com \
--to=matthew.auld@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=matthew.brost@intel.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox