dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name
@ 2026-09-04  8:06 Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  8:06 UTC (permalink / raw)
  To: phasta, christian.koenig, tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, Jonghyuk Kim(MalHyuk)

drm_sched_fence_get_timeline_name() dereferences fence->sched->name. A driver
that allocates a drm_gpu_scheduler per context, queue or VM frees that scheduler
on context teardown, but the finished fence can outlive it: unprivileged
userspace holds the exported fence via a sync_file or drm_syncobj and later
queries its timeline name (e.g. SYNC_IOC_FILE_INFO), reading the freed
scheduler. Same class as CVE-2025-38703 (drm/xe) and CVE-2025-71302
(drm/panthor); amdxdna, nouveau and msm (VM_BIND) are still affected in
mainline, so this fixes it in the core.

v3 tried to drop the finished fence's ->release so that dma_fence detaches the
ops on signalling. That turned out not to be viable:

 - amdgpu dereferences to_drm_sched_fence() unconditionally
   (amdgpu_cs_p2_dependencies(), amdgpu_ctx_fence_time()), and ops-detach makes
   the helper return NULL for a signalled fence - a deterministic NULL deref
   reachable by an unprivileged process;
 - drm/imagination uses the ops pointer as an identity test in
   pvr_queue_fence_is_native(), which Philipp showed would then race; and
 - Christian pointed out that the reference must go from the finished to the
   scheduled fence, not the other way around, so v3's refcount rework was wrong.

So v4 goes back to the minimal caching fix: get_timeline_name() returns a name
cached at fence init and never dereferences ->sched. Both .release callbacks,
the shared allocation, the call_rcu() free and to_drm_sched_fence() all stay
exactly as they are today, so there is no amdgpu/pvr regression and nothing new
for the backend to reason about.

Detaching the ops is still the better fix in the long run - it is what the
dma-fence rules ask for, and it would also cover get_driver_name(), which can
return a string literal from a module that has since been unloaded. Patch 2
records that as a TODO entry (and a comment next to the fence ops) with the
three blockers that have to be solved first, so the cleanup is not lost.

There is no unrelated formatting churn in this version; the kerneldoc reflow
that was mixed into v3 is gone.

Tested with KUnit under KASAN and kmemleak (kunit.py --arch=x86_64), matched
pair:

  - unfixed (get_timeline_name() dereferencing ->sched):
      [FAILED] drm_sched_dma_fence_uaf
      BUG: KASAN: slab-use-after-free in
          drm_sched_fence_get_timeline_name+0x9c/0xb0
  - fixed (this series):
      [PASSED] drm_sched_dma_fence_uaf
      Testing complete. Ran 43 tests: passed: 43

No kmemleak reports, and the whole drm_sched suite passes with no regressions.

Link to v3 (ops-detach):
https://lore.kernel.org/lkml/20260902144204.1843670-1-malhyuk97@gmail.com/
Link to v2 (caching):
https://lore.kernel.org/lkml/20260902105808.1541063-1-malhyuk97@gmail.com/

v4:
 - Drop the ops-detach/refcount rework; return to caching the timeline name
   (per the amdgpu/pvr regression above and Christian's reference-direction
   point).
 - Add a TODO comment and a Documentation/gpu/todo.rst entry for the
   ops-detach cleanup (per Philipp).
 - Keep Cc: stable with "we don't know since when" (per Philipp).
 - No formatting-only hunks in the fix patch.
 - Test suite renamed to drm_sched_dma_fence_uaf_tests for consistency with the
   sibling suites; re-verified under kmemleak as well as KASAN.

Jonghyuk Kim(MalHyuk) (3):
  drm/sched: cache the timeline name to fix a use-after-free
  drm/sched: add the fence ops-detach cleanup to the TODO list
  drm/sched/tests: add a UAF regression test for the timeline name

 Documentation/gpu/todo.rst                    | 39 ++++++++
 drivers/gpu/drm/scheduler/sched_fence.c       | 24 ++++-
 drivers/gpu/drm/scheduler/tests/Makefile      |  1 +
 .../drm/scheduler/tests/tests_integration.c   | 88 +++++++++++++++++++
 include/drm/gpu_scheduler.h                   | 18 +++-
 5 files changed, 168 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/scheduler/tests/tests_integration.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:06 [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:06 ` Jonghyuk Kim(MalHyuk)
  2026-09-04  8:18   ` sashiko-bot
                     ` (2 more replies)
  2026-09-04  8:06 ` [PATCH v4 2/3] drm/sched: add the fence ops-detach cleanup to the TODO list Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name Jonghyuk Kim(MalHyuk)
  2 siblings, 3 replies; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  8:06 UTC (permalink / raw)
  To: phasta, christian.koenig, tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, Jonghyuk Kim(MalHyuk), stable

drm_sched_fence_get_timeline_name() returns fence->sched->name, and the
drm_sched_fence ops keep a .release callback, so the fence is not
ops-detached on signalling (dma_fence_signal_timestamp_locked() only
clears ->ops for fences without .release/.wait). The callback therefore
stays reachable on a long-signalled, userspace-held finished fence and
unconditionally dereferences fence->sched.

A driver that allocates a drm_gpu_scheduler at per-context/per-queue/per-VM
granularity and frees it on an unprivileged context/fd close, while
exporting the resulting finished fence to userspace (drm_syncobj /
sync_file / dma_resv), leaves fence->sched dangling after the free. A
subsequent SYNC_IOC_FILE_INFO ioctl (which calls get_timeline_name()) then
reads the freed scheduler:

  BUG: KASAN: slab-use-after-free in drm_sched_fence_get_timeline_name

This is the same class as CVE-2025-38703 (drm/xe) and CVE-2025-71302
(drm/panthor), which were fixed per-driver. amdxdna, nouveau and msm
(VM_BIND) are still affected in mainline, so fix it in the core to cover
any per-context-scheduler driver at once.

Cache the scheduler's name pointer in the fence at init time, while the
scheduler is guaranteed alive, and return the cached value from
get_timeline_name() without dereferencing fence->sched. The timeline name
is not guaranteed by the contract to outlive the scheduler, so document in
struct drm_sched_init_args that the @name passed to drm_sched_init() must
follow the dma-fence safe access rules and outlive any exported fence.
Every in-tree driver passes a string literal, which satisfies this;
commit 299bc6d50b1b ("drm/xe/guc: Keep scheduler timeline name alive")
keeps drm/xe's dynamically-allocated name alive across the RCU grace and
can be simplified on top of this.

Fixes: 506aa8b02a8d ("dma-fence: Add safe access helpers and document the rules")
Cc: stable@vger.kernel.org # we don't know since when
Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 drivers/gpu/drm/scheduler/sched_fence.c | 24 +++++++++++++++++++++++-
 include/drm/gpu_scheduler.h             | 18 +++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
index 096fe28aa9c9..b2a842a1c9ba 100644
--- a/drivers/gpu/drm/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/scheduler/sched_fence.c
@@ -92,7 +92,13 @@ static const char *drm_sched_fence_get_driver_name(struct dma_fence *fence)
 static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
 {
 	struct drm_sched_fence *fence = to_drm_sched_fence(f);
-	return (const char *)fence->sched->name;
+
+	/*
+	 * Do not dereference fence->sched here: a userspace-held finished
+	 * fence can outlive a per-context scheduler. Return the name cached
+	 * in drm_sched_fence_init() instead.
+	 */
+	return fence->sched_name;
 }
 
 static void drm_sched_fence_free_rcu(struct rcu_head *rcu)
@@ -180,6 +186,14 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f,
 		dma_fence_set_deadline(parent, deadline);
 }
 
+/*
+ * TODO: Both fences implement .release, so dma_fence keeps their ops attached
+ * after signalling. Dropping the callbacks would let dma_fence detach the ops,
+ * after which neither get_timeline_name() nor get_driver_name() can run against
+ * a freed scheduler or an unloaded module - the complete fix. It first requires
+ * auditing every to_drm_sched_fence() caller, since ops-detach makes the helper
+ * return NULL for a signalled fence. See Documentation/gpu/todo.rst.
+ */
 static const struct dma_fence_ops drm_sched_fence_ops_scheduled = {
 	.get_driver_name = drm_sched_fence_get_driver_name,
 	.get_timeline_name = drm_sched_fence_get_timeline_name,
@@ -228,6 +242,14 @@ void drm_sched_fence_init(struct drm_sched_fence *fence,
 	unsigned seq;
 
 	fence->sched = entity->rq->sched;
+	/*
+	 * Cache the scheduler's timeline name. The finished fence may be
+	 * exported to userspace and outlive @sched (per-context schedulers are
+	 * freed on context teardown), so get_timeline_name() must not
+	 * dereference @sched. The name is required to outlive any exported
+	 * fence (see @name in struct drm_sched_init_args).
+	 */
+	fence->sched_name = fence->sched->name;
 	seq = atomic_inc_return(&entity->fence_seq);
 	dma_fence_init(&fence->scheduled, &drm_sched_fence_ops_scheduled,
 		       &fence->lock, entity->fence_context, seq);
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 7a64cc11de08..412b8c4643f1 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -322,6 +322,17 @@ struct drm_sched_fence {
          * belongs to.
          */
 	struct drm_gpu_scheduler	*sched;
+	/**
+	 * @sched_name: the timeline name of @sched, cached at init time.
+	 *
+	 * &drm_sched_fence.finished may be exported to userspace (via a
+	 * sync_file or drm_syncobj) and can outlive @sched: a driver using a
+	 * per-context scheduler frees it on context teardown while a
+	 * userspace-held finished fence still references it. The
+	 * get_timeline_name() callback must therefore not dereference @sched;
+	 * it returns this cached name instead.
+	 */
+	const char			*sched_name;
         /**
          * @lock: the lock used by the scheduled and the finished fences.
          */
@@ -646,7 +657,12 @@ struct drm_gpu_scheduler {
  * @timeout: timeout value in jiffies for submitted jobs.
  * @timeout_wq: workqueue to use for timeout work. If NULL, the system_wq is used.
  * @score: score atomic shared with other schedulers. May be NULL.
- * @name: name (typically the driver's name). Used for debugging
+ * @name: name (typically the driver's name). Used for debugging, and as the
+ *	dma-fence timeline name of the scheduler's fences. It must follow the
+ *	dma-fence safe access rules: a &drm_sched_fence.finished exported to
+ *	userspace can outlive the scheduler, so @name has to outlive any such
+ *	fence - use a string literal, or free it only after an RCU grace period
+ *	past the last exported fence. See drm_sched_fence_get_timeline_name().
  * @dev: associated device. Used for debugging
  */
 struct drm_sched_init_args {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 2/3] drm/sched: add the fence ops-detach cleanup to the TODO list
  2026-09-04  8:06 [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:06 ` Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name Jonghyuk Kim(MalHyuk)
  2 siblings, 0 replies; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  8:06 UTC (permalink / raw)
  To: phasta, christian.koenig, tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, Jonghyuk Kim(MalHyuk)

The previous patch caches the timeline name so that get_timeline_name() no
longer dereferences a scheduler that a userspace-held fence has outlived.
That is a targeted fix: the underlying reason the callback is reachable at
all is that both drm_sched fences implement .release, so dma_fence never
detaches their ops on signalling. get_driver_name() has the same exposure
for module unload.

Dropping the .release callbacks is the complete fix, but it requires
auditing every to_drm_sched_fence() caller (ops-detach makes it return NULL
for signalled fences), a different identity mechanism for
pvr_queue_fence_is_native(), and a rework of the shared allocation's
reference handling. Record that as a TODO entry so the cleanup is not lost.

Suggested-by: Philipp Stanner <phasta@kernel.org>
Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 Documentation/gpu/todo.rst | 39 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 14cf37590fc7..284aeba3c752 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -990,6 +990,45 @@ Contact:
 
 Level: Beginner
 
+Detach the scheduler fence ops on signalling
+--------------------------------------------
+
+The dma-fence contract forbids touching driver-provided data - everything
+reachable through &dma_fence.ops - once a fence is signalled. dma_fence enforces
+that by detaching a fence's ops on signalling, but only for fences that carry
+neither a .release nor a .wait callback (see
+dma_fence_signal_timestamp_locked()).
+
+Both drm_sched fences implement .release, so their ops stay attached forever.
+That leaves the callbacks reachable on a long-signalled fence that userspace
+still holds through a sync_file or drm_syncobj, even after the scheduler is
+gone: get_timeline_name() used to dereference the freed &drm_sched_fence.sched
+(fixed by caching the name), and get_driver_name() can still return a string
+literal belonging to a module that has since been unloaded.
+
+Dropping the .release callbacks so that the ops are detached on signalling is
+the complete fix, and it is what the dma-fence rules ask for. It is not
+straightforward:
+
+Tasks:
+
+- Audit every to_drm_sched_fence() caller. Detaching the ops makes the helper
+  return NULL for a signalled fence, and callers such as
+  amdgpu_cs_p2_dependencies() and amdgpu_ctx_fence_time() dereference the result
+  unconditionally.
+- drm/imagination uses the ops pointer as an identity test in
+  pvr_queue_fence_is_native(); that needs a different mechanism.
+- Rework the reference handling. The scheduled and the finished fence share one
+  allocation, and the finished fence's .release currently drops the scheduled
+  fence's reference, so the callbacks cannot simply be deleted.
+
+Contact:
+
+- Philipp Stanner <phasta@kernel.org>
+- Christian König <christian.koenig@amd.com>
+
+Level: Advanced
+
 Outside DRM
 ===========
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name
  2026-09-04  8:06 [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
  2026-09-04  8:06 ` [PATCH v4 2/3] drm/sched: add the fence ops-detach cleanup to the TODO list Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:06 ` Jonghyuk Kim(MalHyuk)
  2026-09-04  8:15   ` sashiko-bot
  2 siblings, 1 reply; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  8:06 UTC (permalink / raw)
  To: phasta, christian.koenig, tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, Jonghyuk Kim(MalHyuk)

Add a KUnit test that reproduces the drm_sched_fence timeline-name
use-after-free fixed by the previous patch. It submits a job on the mock
scheduler, takes an independent reference on the finished fence (standing
in for a userspace sync_file), lets the job finish, frees the scheduler,
and then queries the timeline name through dma_fence_timeline_name().

Without the fix get_timeline_name() dereferences fence->sched of the freed
scheduler and KASAN reports a slab-use-after-free read in
drm_sched_fence_get_timeline_name(); with the fix the name was cached at
init and the freed scheduler is never touched.

The test needs no hardware - it exercises the drm_sched core through the
existing mock scheduler under KASAN. Per review it lives in a new
tests_integration.c rather than in tests_basic.c, since it is about the
scheduler's interaction with the dma-fence API rather than scheduler
behaviour in isolation.

Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 drivers/gpu/drm/scheduler/tests/Makefile      |  1 +
 .../drm/scheduler/tests/tests_integration.c   | 88 +++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100644 drivers/gpu/drm/scheduler/tests/tests_integration.c

diff --git a/drivers/gpu/drm/scheduler/tests/Makefile b/drivers/gpu/drm/scheduler/tests/Makefile
index 9ec185fbbc15..10abe07c06d2 100644
--- a/drivers/gpu/drm/scheduler/tests/Makefile
+++ b/drivers/gpu/drm/scheduler/tests/Makefile
@@ -3,6 +3,7 @@
 drm-sched-tests-y := \
         mock_scheduler.o \
         tests_basic.o \
+        tests_integration.o \
         tests_scheduler.o
 
 obj-$(CONFIG_DRM_SCHED_KUNIT_TEST) += drm-sched-tests.o
diff --git a/drivers/gpu/drm/scheduler/tests/tests_integration.c b/drivers/gpu/drm/scheduler/tests/tests_integration.c
new file mode 100644
index 000000000000..4a2d5571440d
--- /dev/null
+++ b/drivers/gpu/drm/scheduler/tests/tests_integration.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/dma-fence.h>
+#include <linux/rcupdate.h>
+
+#include "sched_tests.h"
+
+/*
+ * Integration-style regression tests that exercise the interaction between the
+ * DRM scheduler and the dma-fence API, rather than scheduler behaviour in
+ * isolation.
+ */
+
+/*
+ * Reproduce the drm_sched_fence timeline-name use-after-free.
+ *
+ * drm_sched_fence_get_timeline_name() used to dereference fence->sched->name.
+ * A driver may free a per-context/per-queue/per-VM drm_gpu_scheduler while
+ * userspace still holds the exported ->finished fence (via sync_file /
+ * drm_syncobj). Querying the timeline name afterwards must not touch the freed
+ * scheduler.
+ *
+ * Without the fix this reads fence->sched->name from freed slab memory and
+ * KASAN reports a slab-use-after-free in drm_sched_fence_get_timeline_name();
+ * with the fix the name is cached at init and the freed scheduler is never
+ * dereferenced. Same class as CVE-2025-38703 (drm/xe) and CVE-2025-71302
+ * (drm/panthor).
+ */
+static void drm_sched_dma_fence_uaf(struct kunit *test)
+{
+	struct drm_mock_sched_entity *entity;
+	struct drm_mock_scheduler *sched;
+	struct drm_mock_sched_job *job;
+	struct dma_fence *finished;
+	const char __rcu *name;
+	bool done;
+
+	sched = drm_mock_sched_new(test, MAX_SCHEDULE_TIMEOUT);
+	entity = drm_mock_sched_entity_new(test, DRM_SCHED_PRIORITY_NORMAL,
+					   sched);
+	job = drm_mock_sched_job_new(test, entity);
+
+	/* The s_fence is only created by drm_sched_job_arm(). */
+	drm_mock_sched_job_submit(job);
+
+	/* Independent reference on the finished fence == userspace sync_file. */
+	finished = dma_fence_get(&job->base.s_fence->finished);
+
+	/* Let the job get scheduled (hw fence created), then signal + finish. */
+	done = drm_mock_sched_job_wait_scheduled(job, HZ);
+	KUNIT_ASSERT_TRUE(test, done);
+	drm_mock_sched_advance(sched, 1);
+	done = drm_mock_sched_job_wait_finished(job, HZ);
+	KUNIT_ASSERT_TRUE(test, done);
+
+	/*
+	 * Free the per-context scheduler while the finished fence is held.
+	 * kunit_kfree() releases the backing memory immediately (rather than at
+	 * test teardown) so that fence->sched becomes a dangling pointer now.
+	 */
+	drm_mock_sched_entity_free(entity);
+	drm_mock_sched_fini(sched);
+	kunit_kfree(test, sched);
+
+	/*
+	 * Query the timeline name of the now-stale fence. With the fix the name
+	 * was cached at init, so the freed scheduler is not dereferenced;
+	 * without it this is a use-after-free read of the freed scheduler.
+	 */
+	rcu_read_lock();
+	name = dma_fence_timeline_name(finished);
+	KUNIT_EXPECT_NOT_NULL(test, name);
+	rcu_read_unlock();
+
+	dma_fence_put(finished);
+}
+
+static struct kunit_case drm_sched_dma_fence_tests[] = {
+	KUNIT_CASE(drm_sched_dma_fence_uaf),
+	{}
+};
+
+static struct kunit_suite drm_sched_dma_fence = {
+	.name = "drm_sched_dma_fence_uaf_tests",
+	.test_cases = drm_sched_dma_fence_tests,
+};
+
+kunit_test_suite(drm_sched_dma_fence);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name
  2026-09-04  8:06 ` [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:15   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-04  8:15 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk); +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KUnit assertion failure aborts the test without tearing down the DRM scheduler, leading to a use-after-free crash.
--

commit 5e3b2d1526bb334b5c71995b67ba48f0b45424d6
Author: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>

drm/sched/tests: add a UAF regression test for the timeline name

This patch introduces a KUnit test to reproduce a use-after-free issue 
related to the drm_sched_fence timeline name. It submits a job on a mock 
scheduler and verifies the timeline name can be safely queried after the 
scheduler is freed.

> diff --git a/drivers/gpu/drm/scheduler/tests/tests_integration.c b/drivers/gpu/drm/scheduler/tests/tests_integration.c
> new file mode 100644
> index 0000000000000..4a2d5571440d2
> --- /dev/null
> +++ b/drivers/gpu/drm/scheduler/tests/tests_integration.c

[ ... ]

> +static void drm_sched_dma_fence_uaf(struct kunit *test)
> +{
> +	struct drm_mock_sched_entity *entity;
> +	struct drm_mock_scheduler *sched;
> +	struct drm_mock_sched_job *job;
> +	struct dma_fence *finished;
> +	const char __rcu *name;
> +	bool done;
> +
> +	sched = drm_mock_sched_new(test, MAX_SCHEDULE_TIMEOUT);
> +	entity = drm_mock_sched_entity_new(test, DRM_SCHED_PRIORITY_NORMAL,
> +					   sched);
> +	job = drm_mock_sched_job_new(test, entity);
> +
> +	/* The s_fence is only created by drm_sched_job_arm(). */
> +	drm_mock_sched_job_submit(job);
> +
> +	/* Independent reference on the finished fence == userspace sync_file. */
> +	finished = dma_fence_get(&job->base.s_fence->finished);
> +
> +	/* Let the job get scheduled (hw fence created), then signal + finish. */
> +	done = drm_mock_sched_job_wait_scheduled(job, HZ);
> +	KUNIT_ASSERT_TRUE(test, done);

[Severity: Medium]
Will a failure here in drm_sched_dma_fence_uaf() abort the test and skip the
drm_mock_sched_fini() teardown?

If this assertion fails, the test thread aborts immediately. Because 
drm_mock_sched_fini() is bypassed, any active kernel mechanisms from the DRM 
scheduler might still hold references to sched->base. 

When KUnit's automated cleanup runs (since sched was allocated with 
kunit_kzalloc), could those mechanisms access the freed memory and cause a 
use-after-free crash in the test runner?

> +	drm_mock_sched_advance(sched, 1);
> +	done = drm_mock_sched_job_wait_finished(job, HZ);
> +	KUNIT_ASSERT_TRUE(test, done);

[Severity: Medium]
Similarly, if this job finish assertion fails, does aborting the test also 
bypass the manual cleanup phase below, causing the same use-after-free issue
during KUnit teardown?

> +
> +	/*
> +	 * Free the per-context scheduler while the finished fence is held.
> +	 * kunit_kfree() releases the backing memory immediately (rather than at
> +	 * test teardown) so that fence->sched becomes a dangling pointer now.
> +	 */
> +	drm_mock_sched_entity_free(entity);
> +	drm_mock_sched_fini(sched);
> +	kunit_kfree(test, sched);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904080618.2098450-1-malhyuk97@gmail.com?part=3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:18   ` sashiko-bot
  2026-09-04  8:20   ` Christian König
  2026-09-04  9:57   ` Danilo Krummrich
  2 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-04  8:18 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk); +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The patch attempts to fix a Use-After-Free by caching the timeline name in `fence->sched_name`, but it introduces a direct Use-After-Free of the string memory itself. The commit message incorrectly assumes all drivers pass a string literal or keep the string alive via RCU. However, `panthor` dynamically allocates the timeline name (`kasprintf`) and synchronously frees it (`kfree(queue->name)`) when the queue is destroyed. Furthermore, for `xe`, `kfree_rcu` does not protect against a `sync_file` fd held open by userspace for an arbitrary duration (userspace holding an fd does not equate to holding an RCU read lock). When userspace queries the fence info via the `SYNC_IOC_FILE_INFO` ioctl after the string is freed, `drm_sched_fence_get_timeline_name()` returns the cached pointer to the freed memory, leading to an info leak to userspace.

Pre-existing issues:
- [High] The AMDGPU driver's `amdgpu_sync_same_dev()` function incorrectly assumes that any `drm_sched_fence` belongs to an AMDGPU ring. `to_drm_sched_fence()` returns non-NULL for *any* DRM scheduler fence globally (e.g., from `panthor`, `xe`, `nouveau`). If a foreign fence is passed via a `dma-buf` or `syncobj`, `amdgpu_sync_same_dev()` unconditionally calculates `ring = container_of(s_fence->sched, struct amdgpu_ring, sched)` and dereferences `ring->adev`. This results in an out-of-bounds read (Type Confusion). Additionally, if the foreign fence originated from a per-context scheduler that has been freed (like a destroyed `panthor` queue), this results in a Use-After-Free read.
- [High] The PowerVR driver's `pvr_queue_fence_is_native()` accesses `sched_fence->sched->ops` to determine if a fence is native to the driver. However, if the `drm_sched_fence` originated from a per-context scheduler (whether from `pvr` or another driver like `panthor`) and that scheduler has been destroyed, `sched_fence->sched` is a dangling pointer. Dereferencing it to check `->ops` causes a Use-After-Free read.
--

commit 99163b915ba8e6d549d23c04536ae0f2ac030b70
Author: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>

drm/sched: cache the timeline name to fix a use-after-free

This commit caches the scheduler's timeline name inside drm_sched_fence to
prevent a use-after-free when a userspace-held finished fence outlives a
per-context scheduler. It documents that the name must outlive the fence,
assuming all in-tree drivers pass string literals or keep the name alive.

>     Every in-tree driver passes a string literal, which satisfies this;

[Severity: High]
Is this assumption correct for all in-tree drivers?

For example, panthor dynamically allocates the timeline name and synchronously
frees it when the queue is destroyed:

drivers/gpu/drm/panthor/panthor_sched.c:group_free_queue() {
    ...
    kfree(queue->name);
    ...
}

If a user creates a panthor queue, submits a job to get a fence, exports it
to a sync_file, and destroys the queue, the string memory is freed. When
userspace then queries the fence info via the SYNC_IOC_FILE_INFO ioctl,
wouldn't the cached pointer point to freed memory, leading to an info leak?

> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
> index 096fe28aa9c9e..b2a842a1c9ba9 100644
> --- a/drivers/gpu/drm/scheduler/sched_fence.c
> +++ b/drivers/gpu/drm/scheduler/sched_fence.c
> @@ -92,7 +92,13 @@ static const char *drm_sched_fence_get_driver_name(struct dma_fence *fence)
>  static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
>  {
>  	struct drm_sched_fence *fence = to_drm_sched_fence(f);
> -	return (const char *)fence->sched->name;
> +
> +	/*
> +	 * Do not dereference fence->sched here: a userspace-held finished
> +	 * fence can outlive a per-context scheduler. Return the name cached
> +	 * in drm_sched_fence_init() instead.
> +	 */
> +	return fence->sched_name;

[Severity: High]
Does returning this cached pointer just replace the scheduler use-after-free
with a string memory use-after-free?

Because drivers like panthor can free the string memory synchronously during
queue destruction, this cached pointer will point to freed memory when read
by the caller.

[ ... ]

> @@ -228,6 +242,14 @@ void drm_sched_fence_init(struct drm_sched_fence *fence,
>  	unsigned seq;
>  
>  	fence->sched = entity->rq->sched;
> +	/*
> +	 * Cache the scheduler's timeline name. The finished fence may be
> +	 * exported to userspace and outlive @sched (per-context schedulers are
> +	 * freed on context teardown), so get_timeline_name() must not
> +	 * dereference @sched. The name is required to outlive any exported
> +	 * fence (see @name in struct drm_sched_init_args).
> +	 */
> +	fence->sched_name = fence->sched->name;

[Severity: High]
Does caching the string pointer here without taking ownership or extending
its lifetime erroneously assume the string will inherently outlive the fence?

This is a pre-existing issue, but the dangling fence->sched pointer causes
problems in other drivers as well when foreign fences are imported.

[Severity: High]
Does the AMDGPU driver's amdgpu_sync_same_dev() incorrectly assume any
drm_sched_fence globally belongs to an AMDGPU ring?

drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c:amdgpu_sync_same_dev() {
    ...
    ring = container_of(s_fence->sched, struct amdgpu_ring, sched);
    return ring->adev == adev;
}

If a foreign fence from another driver (e.g., panthor) is passed via a dma-buf
or syncobj and used as an input dependency to the AMDGPU command submission
ioctl, this unconditional container_of() calculation will dereference an
invalid or freed pointer. Can this lead to an out-of-bounds type confusion
read, or a use-after-free read if the original per-context scheduler was
already destroyed?

This is also a pre-existing issue, but similar to the AMDGPU issue above,
does the PowerVR driver also dereference a freed scheduler pointer?

[Severity: High]
drivers/gpu/drm/imagination/pvr_queue.c:pvr_queue_fence_is_native() {
    ...
    if (sched_fence &&
        sched_fence->sched->ops == &pvr_queue_sched_ops)
    ...
}

If a user exports a drm_sched_fence from a queue, destroys that queue (freeing
the scheduler), and then passes the fence as an input dependency to the PVR
job submission ioctl, sched_fence->sched will be a dangling pointer. Will
dereferencing it to check the ops field cause a use-after-free read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904080618.2098450-1-malhyuk97@gmail.com?part=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
  2026-09-04  8:18   ` sashiko-bot
@ 2026-09-04  8:20   ` Christian König
  2026-09-04  8:31     ` Philipp Stanner
  2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
  2026-09-04  9:57   ` Danilo Krummrich
  2 siblings, 2 replies; 16+ messages in thread
From: Christian König @ 2026-09-04  8:20 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk), phasta, tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, stable



On 9/4/26 10:06, Jonghyuk Kim(MalHyuk) wrote:
> drm_sched_fence_get_timeline_name() returns fence->sched->name, and the
> drm_sched_fence ops keep a .release callback, so the fence is not
> ops-detached on signalling (dma_fence_signal_timestamp_locked() only
> clears ->ops for fences without .release/.wait). The callback therefore
> stays reachable on a long-signalled, userspace-held finished fence and
> unconditionally dereferences fence->sched.
> 
> A driver that allocates a drm_gpu_scheduler at per-context/per-queue/per-VM
> granularity and frees it on an unprivileged context/fd close, while
> exporting the resulting finished fence to userspace (drm_syncobj /
> sync_file / dma_resv), leaves fence->sched dangling after the free. A
> subsequent SYNC_IOC_FILE_INFO ioctl (which calls get_timeline_name()) then
> reads the freed scheduler:
> 
>   BUG: KASAN: slab-use-after-free in drm_sched_fence_get_timeline_name
> 
> This is the same class as CVE-2025-38703 (drm/xe) and CVE-2025-71302
> (drm/panthor), which were fixed per-driver. amdxdna, nouveau and msm
> (VM_BIND) are still affected in mainline, so fix it in the core to cover
> any per-context-scheduler driver at once.
> 
> Cache the scheduler's name pointer in the fence at init time, while the
> scheduler is guaranteed alive, and return the cached value from
> get_timeline_name() without dereferencing fence->sched. The timeline name
> is not guaranteed by the contract to outlive the scheduler, so document in
> struct drm_sched_init_args that the @name passed to drm_sched_init() must
> follow the dma-fence safe access rules and outlive any exported fence.
> Every in-tree driver passes a string literal, which satisfies this;
> commit 299bc6d50b1b ("drm/xe/guc: Keep scheduler timeline name alive")
> keeps drm/xe's dynamically-allocated name alive across the RCU grace and
> can be simplified on top of this.
> 
> Fixes: 506aa8b02a8d ("dma-fence: Add safe access helpers and document the rules")
> Cc: stable@vger.kernel.org # we don't know since when
> Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
> ---
>  drivers/gpu/drm/scheduler/sched_fence.c | 24 +++++++++++++++++++++++-
>  include/drm/gpu_scheduler.h             | 18 +++++++++++++++++-
>  2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
> index 096fe28aa9c9..b2a842a1c9ba 100644
> --- a/drivers/gpu/drm/scheduler/sched_fence.c
> +++ b/drivers/gpu/drm/scheduler/sched_fence.c
> @@ -92,7 +92,13 @@ static const char *drm_sched_fence_get_driver_name(struct dma_fence *fence)
>  static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
>  {
>  	struct drm_sched_fence *fence = to_drm_sched_fence(f);
> -	return (const char *)fence->sched->name;
> +
> +	/*
> +	 * Do not dereference fence->sched here: a userspace-held finished
> +	 * fence can outlive a per-context scheduler. Return the name cached
> +	 * in drm_sched_fence_init() instead.
> +	 */
> +	return fence->sched_name;

I don't think that this actually solves the problem, the sched_name still needs to be kept alive until all fences are destroyed and that is something drivers don't want/can do.

>  }
>  
>  static void drm_sched_fence_free_rcu(struct rcu_head *rcu)
> @@ -180,6 +186,14 @@ static void drm_sched_fence_set_deadline_finished(struct dma_fence *f,
>  		dma_fence_set_deadline(parent, deadline);
>  }
>  
> +/*
> + * TODO: Both fences implement .release, so dma_fence keeps their ops attached
> + * after signalling. Dropping the callbacks would let dma_fence detach the ops,
> + * after which neither get_timeline_name() nor get_driver_name() can run against
> + * a freed scheduler or an unloaded module - the complete fix. It first requires
> + * auditing every to_drm_sched_fence() caller, since ops-detach makes the helper
> + * return NULL for a signalled fence. See Documentation/gpu/todo.rst.
> + */

That sounds like a bad idea as well.

Dropping the fence->ops is to detach the fence from the module which originally issued it and not solve lifetime problems between the scheduler and the driver.

I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):

Here we changed the check in dma_fence_driver_name() and dma_fence_timeline_name():

@@ -1167,7 +1167,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
 
        /* RCU protection is required for safe access to returned string */
        ops = rcu_dereference(fence->ops);
-       if (!dma_fence_test_signaled_flag(fence))
+       if (ops)
                return (const char __rcu *)ops->get_driver_name(fence);
        else
                return (const char __rcu *)"detached-driver";

The problem is that we didn't considered that there a fence implementations which still have a release or wait callbacks but rely on not needing to return a string for a signaled fence.

Regards,
Christian.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:20   ` Christian König
@ 2026-09-04  8:31     ` Philipp Stanner
  2026-09-04 12:49       ` Christian König
  2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
  1 sibling, 1 reply; 16+ messages in thread
From: Philipp Stanner @ 2026-09-04  8:31 UTC (permalink / raw)
  To: Christian König, Jonghyuk Kim(MalHyuk), phasta, tursulin,
	matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, stable

On Fri, 2026-09-04 at 10:20 +0200, Christian König wrote:
> 

[…]

> >  
> > +/*
> > + * TODO: Both fences implement .release, so dma_fence keeps their ops attached
> > + * after signalling. Dropping the callbacks would let dma_fence detach the ops,
> > + * after which neither get_timeline_name() nor get_driver_name() can run against
> > + * a freed scheduler or an unloaded module - the complete fix. It first requires
> > + * auditing every to_drm_sched_fence() caller, since ops-detach makes the helper
> > + * return NULL for a signalled fence. See Documentation/gpu/todo.rst.
> > + */
> 
> That sounds like a bad idea as well.
> 
> Dropping the fence->ops is to detach the fence from the module which originally issued it and not solve lifetime problems between the scheduler and the driver.

It can be used to solve that problem though, can it not?

The underlying problem is that the driver has no chance to figure out
when the scheduler is actually done with all the sched_fences.

Remember our lengthy discussions about drm_sched_fini(). Maybe we want
to reconsider providing a function with which the driver can wait until
the scheduler is done with all finished_fences?

> 
> I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):
> 
> Here we changed the check in dma_fence_driver_name() and dma_fence_timeline_name():
> 
> @@ -1167,7 +1167,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
>  
>         /* RCU protection is required for safe access to returned string */
>         ops = rcu_dereference(fence->ops);
> -       if (!dma_fence_test_signaled_flag(fence))
> +       if (ops)
>                 return (const char __rcu *)ops->get_driver_name(fence);
>         else
>                 return (const char __rcu *)"detached-driver";
> 
> The problem is that we didn't considered that there a fence implementations which still have a release or wait callbacks but rely on not needing to return a string for a signaled fence.
> 

Could we move the signaled check to amdgpu and pvr?

IOW, we keep the solution presented here (removing ops->release for
finished-fence) and the few drivers that check whether a fence is their
own first do a locked dma_fence_is_signaled() check?


P.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:20   ` Christian König
  2026-09-04  8:31     ` Philipp Stanner
@ 2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
  2026-09-04  8:39       ` Philipp Stanner
  2026-09-04  9:07       ` Tvrtko Ursulin
  1 sibling, 2 replies; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  8:31 UTC (permalink / raw)
  To: christian.koenig, phasta, tursulin, matthew.brost, dakr
  Cc: Jonghyuk Kim(MalHyuk), dri-devel, linux-kernel, mdaenzer,
	alessio.belle, luigi.santivetti

On 9/4/26 10:20, Christian König wrote:
>> +	return fence->sched_name;
>
> I don't think that this actually solves the problem, the sched_name still
> needs to be kept alive until all fences are destroyed and that is something
> drivers don't want/can do.

Agreed, and that is the same objection Tvrtko raised against v1. Caching the
pointer only moves the lifetime requirement from the scheduler to the string,
and the documentation hunk I added just pushes that requirement onto drivers.
I will drop that patch.

>> +/*
>> + * TODO: Both fences implement .release, so dma_fence keeps their ops attached
>> + * after signalling. Dropping the callbacks would let dma_fence detach the ops,
>
> That sounds like a bad idea as well.
>
> Dropping the fence->ops is to detach the fence from the module which
> originally issued it and not solve lifetime problems between the scheduler
> and the driver.

Understood - ops-detach is about producer/module decoupling, not about the
scheduler's lifetime relative to the driver, so framing it as "the complete
fix" for this bug was wrong. I will drop the TODO patch as well.

> I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b
> ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):
> [...]
> The problem is that we didn't considered that there a fence implementations
> which still have a release or wait callbacks but rely on not needing to
> return a string for a signaled fence.

That matches what I see in the code, thanks - this is the actual root cause and
it is not drm/sched specific.

dma_fence_signal_timestamp_locked() only clears the ops pointer for fences that
carry neither .release nor .wait:

	ops = rcu_dereference_protected(fence->ops, true);
	if (!ops->release && !ops->wait)
		RCU_INIT_POINTER(fence->ops, NULL);

drm_sched_fence implements .release, so its ops survive signalling. Before
035219a760ed the helpers gated on the signaled bit, so such a fence returned
the static string and the producer callback was never reached. Since that
commit they gate on the ops pointer alone, so get_timeline_name() /
get_driver_name() are called on a long-signalled fence - which is exactly the
window my report hits, with ->sched already freed.

To be clear, I am not suggesting a revert: 035219a760ed fixes a real problem,
namely that "set signaled bit, then NULL the ops" and "load ops, then check the
signaled bit" can be reordered on weakly ordered platforms, and using the ops
pointer as the synchronization point solves that elegantly. That property
should stay.

What seems to be missing is that the ops check answers "may I dereference the
pointer", not "may I call into the producer". The dma-fence rules say the
latter is not allowed once the fence is signalled, so I think both conditions
are needed:

	ops = rcu_dereference(fence->ops);
	if (ops && !dma_fence_test_signaled_flag(fence))
		return (const char __rcu *)ops->get_timeline_name(fence);
	else
		return (const char __rcu *)"signaled-timeline";

The ops load keeps the RCU/ordering guarantee from 035219a760ed, and the
signaled check restores the contract. That fixes every implementation which
keeps .release or .wait and assumes it is not called after signalling, rather
than just drm/sched, and it puts no lifetime burden on drivers.

Philipp, since 035219a760ed is yours - do you agree with adding the signaled
check back on top of the ops check? I would rather have your ack on that before
I respin.

One thing I noticed while checking the callers: the tracepoints in
include/trace/events/dma_fence.h and drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
call fence->ops->get_driver_name() / get_timeline_name() directly instead of
going through the helpers, so they are not covered by the above. That looks
like a pre-existing and much narrower exposure (tracing only), but let me know
if you want it addressed in the same series or separately.

So for v5 I plan:

  1. dma-buf/dma-fence: add the signaled check back to dma_fence_driver_name()
     and dma_fence_timeline_name(), Fixes: 035219a760ed, Cc: stable.
  2. Keep the KUnit regression test - it exercises exactly this path through
     dma_fence_timeline_name() and needs no change; it also picked up the
     teardown issue the review bot flagged, which I have fixed locally by
     using kunit_add_action_or_reset() + kunit_release_action().

and drop the drm/sched caching and TODO patches. I will wait for your and
Philipp's input before sending it.

Thanks,
Jonghyuk

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
@ 2026-09-04  8:39       ` Philipp Stanner
  2026-09-04  9:11         ` Jonghyuk Kim(MalHyuk)
  2026-09-04  9:07       ` Tvrtko Ursulin
  1 sibling, 1 reply; 16+ messages in thread
From: Philipp Stanner @ 2026-09-04  8:39 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk), christian.koenig, phasta, tursulin,
	matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti

On Fri, 2026-09-04 at 17:31 +0900, Jonghyuk Kim(MalHyuk) wrote:
> On 9/4/26 10:20, Christian König wrote:
> Understood - ops-detach is about producer/module decoupling, not about the
> scheduler's lifetime relative to the driver, so framing it as "the complete
> fix" for this bug was wrong. I will drop the TODO patch as well.

Please wait a bit until we discussed it for a bit.

> 
> > I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b
> > ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):
> > [...]
> > The problem is that we didn't considered that there a fence implementations
> > which still have a release or wait callbacks but rely on not needing to
> > return a string for a signaled fence.
> 
> That matches what I see in the code, thanks - this is the actual root cause and
> it is not drm/sched specific.
> 
> dma_fence_signal_timestamp_locked() only clears the ops pointer for fences that
> carry neither .release nor .wait:
> 
> 	ops = rcu_dereference_protected(fence->ops, true);
> 	if (!ops->release && !ops->wait)
> 		RCU_INIT_POINTER(fence->ops, NULL);
> 
> drm_sched_fence implements .release, so its ops survive signalling. Before
> 035219a760ed the helpers gated on the signaled bit, so such a fence returned
> the static string and the producer callback was never reached. Since that
> commit they gate on the ops pointer alone, so get_timeline_name() /
> get_driver_name() are called on a long-signalled fence - which is exactly the
> window my report hits, with ->sched already freed.
> 
> To be clear, I am not suggesting a revert: 035219a760ed fixes a real problem,
> namely that "set signaled bit, then NULL the ops" and "load ops, then check the
> signaled bit" can be reordered on weakly ordered platforms, and using the ops
> pointer as the synchronization point solves that elegantly. That property
> should stay.
> 
> What seems to be missing is that the ops check answers "may I dereference the
> pointer", not "may I call into the producer". The dma-fence rules say the
> latter is not allowed once the fence is signalled, so I think both conditions
> are needed:
> 
> 	ops = rcu_dereference(fence->ops);
> 	if (ops && !dma_fence_test_signaled_flag(fence))
> 		return (const char __rcu *)ops->get_timeline_name(fence);
> 	else
> 		return (const char __rcu *)"signaled-timeline";
> 
> The ops load keeps the RCU/ordering guarantee from 035219a760ed, and the
> signaled check restores the contract. That fixes every implementation which
> keeps .release or .wait and assumes it is not called after signalling, rather
> than just drm/sched, and it puts no lifetime burden on drivers.
> 
> Philipp, since 035219a760ed is yours - do you agree with adding the signaled
> check back on top of the ops check? I would rather have your ack on that before
> I respin.

That commit has to stay because it solves an ordering problem on some
architectures.

I do agree to having a conversation with Christian in my other mail,
where I wonder whether it would be better if a driver does the is-
signaled check, holding the lock.

These things are extremely complicated and cannot be rushed. We
discussed dma_fence synchronization for weeks this summer and concluded
that there is no easy solution at hands for some problems, notably
because of callbacks and legacy callbacks, and also because Christian
and I disagree on to what degree a fence should be locked.

> 
> One thing I noticed while checking the callers: the tracepoints in
> include/trace/events/dma_fence.h and drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> call fence->ops->get_driver_name() / get_timeline_name() directly instead of
> going through the helpers, so they are not covered by the above. That looks
> like a pre-existing and much narrower exposure (tracing only), but let me know
> if you want it addressed in the same series or separately.

Do you generate part of your mails with an LLM? It seems to be a huge
corpus for 11min of time between receiving and answer


P.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
  2026-09-04  8:39       ` Philipp Stanner
@ 2026-09-04  9:07       ` Tvrtko Ursulin
  1 sibling, 0 replies; 16+ messages in thread
From: Tvrtko Ursulin @ 2026-09-04  9:07 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk), christian.koenig, phasta, matthew.brost,
	dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti


To address only one narrow point quickly, I haven't had the time to 
catch up with the thread yet:

On 04/09/2026 09:31, Jonghyuk Kim(MalHyuk) wrote:

8><

> One thing I noticed while checking the callers: the tracepoints in
> include/trace/events/dma_fence.h and drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
> call fence->ops->get_driver_name() / get_timeline_name() directly instead of
> going through the helpers, so they are not covered by the above. That looks
> like a pre-existing and much narrower exposure (tracing only), but let me know
> if you want it addressed in the same series or separately.

For the dma_fence.h see the comment:

/*
  * Safe only for call sites which are guaranteed to not race with fence
  * signaling, holding the fence->lock and having checked for not 
signaled, or
  * the signaling path itself.
  *
  * TODO: Remove the need for this event class when drivers switch to 
independent
  *       fences.
  */


These tracepoints call the name vfuncs directly: dma_fence_init, 
dma_fence_enable_signal, dma_fence_signaled. So those are supposed to be 
safe as per comment and changing them to use the helper would create a 
different issue (can't remember from the top of my head which one - but 
the thread is moving fast so I wanted to reply quickly).

amdgpu_trace.h also look safe on a quick look because they are called 
before the job which can signal the fence is submitted.

Regards,

Tvrtko


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:39       ` Philipp Stanner
@ 2026-09-04  9:11         ` Jonghyuk Kim(MalHyuk)
  0 siblings, 0 replies; 16+ messages in thread
From: Jonghyuk Kim(MalHyuk) @ 2026-09-04  9:11 UTC (permalink / raw)
  To: phasta, christian.koenig, tursulin, matthew.brost, dakr
  Cc: Jonghyuk Kim(MalHyuk), dri-devel, linux-kernel, mdaenzer,
	alessio.belle, luigi.santivetti

On Fri, 2026-09-04 at 10:39 +0200, Philipp Stanner wrote:
> Please wait a bit until we discussed it for a bit.

Understood. No respin from me until you and Christian have settled the
direction.

> Do you generate part of your mails with an LLM? It seems to be a huge
> corpus for 11min of time between receiving and answer

Yes, I draft them with LLM assistance. I should have said so up front,
sorry. What I do not delegate is the checking: I verify the claims against
the tree before sending, and I am answerable for whatever goes out under my
name. The bug report itself came out of my own KASAN testing.

Your point about the volume is fair, and there is a concrete example of why
you are right to push back. In the v4 commit message I asserted that "every
in-tree driver passes a string literal". That is wrong: panthor kasprintf()s
the name (panthor_sched.c:3591) and kfree()s it in group_free_queue(). So
Christian's objection to caching holds for an in-tree driver today, not just
in theory, and I should have caught that before sending rather than after.
I will keep the mails shorter and slower.

Tvrtko, thanks for the quick answer on the tracepoints - the comment makes it
clear those call sites are fine, so I will drop that point.

Jonghyuk

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
  2026-09-04  8:18   ` sashiko-bot
  2026-09-04  8:20   ` Christian König
@ 2026-09-04  9:57   ` Danilo Krummrich
  2026-09-04 10:51     ` Philipp Stanner
  2 siblings, 1 reply; 16+ messages in thread
From: Danilo Krummrich @ 2026-09-04  9:57 UTC (permalink / raw)
  To: Jonghyuk Kim(MalHyuk)
  Cc: phasta, christian.koenig, tursulin, matthew.brost, dri-devel,
	linux-kernel, mdaenzer, alessio.belle, luigi.santivetti, stable

On Fri Sep 4, 2026 at 10:06 AM CEST, Jonghyuk Kim(MalHyuk) wrote:
> Fixes: 506aa8b02a8d ("dma-fence: Add safe access helpers and document the rules")
> Cc: stable@vger.kernel.org # we don't know since when

How is that? If there's a Fixes: tag it expresses that this commit introduced
the bug, so it should be known exactly since when it is present.

However, I don't think this is the correct Fixes: tag, the bug was present
before. I think it was present all the way back to commit f556cb0caeec
("drm/amd: add scheduler fence implementation (v2)").

It is also not the case that it wasn't a bug before we had per-context
schedulers, it's just that it became much more likely to trigger with
per-context schedulers; nothing prevents the kernel from having a reference
count of a DMA fence after the driver was unbound and hence the
per-device/per-ring scheduler was torn down.

More in general, putting "we don't know since when" for the stable folks is
rough. If neither the author of the patch nor the maintainers of the subsystem
can figure it out, how are the stable folks supposed to?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  9:57   ` Danilo Krummrich
@ 2026-09-04 10:51     ` Philipp Stanner
  0 siblings, 0 replies; 16+ messages in thread
From: Philipp Stanner @ 2026-09-04 10:51 UTC (permalink / raw)
  To: Danilo Krummrich, Jonghyuk Kim(MalHyuk)
  Cc: phasta, christian.koenig, tursulin, matthew.brost, dri-devel,
	linux-kernel, mdaenzer, alessio.belle, luigi.santivetti, stable

On Fri, 2026-09-04 at 11:57 +0200, Danilo Krummrich wrote:
> On Fri Sep 4, 2026 at 10:06 AM CEST, Jonghyuk Kim(MalHyuk) wrote:
> > Fixes: 506aa8b02a8d ("dma-fence: Add safe access helpers and document the rules")
> > Cc: stable@vger.kernel.org # we don't know since when
> 
> How is that?

That is referring to the fact that it only is a reachable bug if the
respective driver exposes a mechanism that runs into
get_timeline_name().

> 
> More in general, putting "we don't know since when" for the stable folks is
> rough. If neither the author of the patch nor the maintainers of the subsystem
> can figure it out, how are the stable folks supposed to?

That's on me, it was a joking phrase by me that Jonghyuk or his LLM
picked up 1:1.

Anyways. So would you suggest backporting a fix as far as possible to
the amd origin commit? That might not be backportable that far, so
stable would still have to figure it out.


P.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04  8:31     ` Philipp Stanner
@ 2026-09-04 12:49       ` Christian König
  2026-09-04 19:06         ` Philipp Stanner
  0 siblings, 1 reply; 16+ messages in thread
From: Christian König @ 2026-09-04 12:49 UTC (permalink / raw)
  To: phasta, Jonghyuk Kim(MalHyuk), tursulin, matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, stable

On 9/4/26 10:31, Philipp Stanner wrote:
> On Fri, 2026-09-04 at 10:20 +0200, Christian König wrote:
>>
> 
> […]
> 
>>>  
>>> +/*
>>> + * TODO: Both fences implement .release, so dma_fence keeps their ops attached
>>> + * after signalling. Dropping the callbacks would let dma_fence detach the ops,
>>> + * after which neither get_timeline_name() nor get_driver_name() can run against
>>> + * a freed scheduler or an unloaded module - the complete fix. It first requires
>>> + * auditing every to_drm_sched_fence() caller, since ops-detach makes the helper
>>> + * return NULL for a signalled fence. See Documentation/gpu/todo.rst.
>>> + */
>>
>> That sounds like a bad idea as well.
>>
>> Dropping the fence->ops is to detach the fence from the module which originally issued it and not solve lifetime problems between the scheduler and the driver.
> 
> It can be used to solve that problem though, can it not?

Yes, but I think forcing dma_fence implementations to drop their release callback to fix lifetime problems with the driver and timeline name functions is a bad idea.

We should keep this fix simple and focused so that we can easily backport it.

Fixing all dma_fence implementations to not need the release callback is something I really like to have as well, but not to fix this issue here. 

> 
> The underlying problem is that the driver has no chance to figure out
> when the scheduler is actually done with all the sched_fences.
> 
> Remember our lengthy discussions about drm_sched_fini(). Maybe we want
> to reconsider providing a function with which the driver can wait until
> the scheduler is done with all finished_fences?

The problem is that won't help unless we either add more checks or fix the checks in dma_fence_driver_name()/dma_fence_timeline_name().

The dma_fence object can trivially outlive both the driver and the scheduler instance it originally issued.

>>
>> I think we should rather re-consider patch 035219a760edb35ae9a9e96beba7f122e26a997b ("dma-buf: dma-fence: Fix potential NULL pointer dereference"):
>>
>> Here we changed the check in dma_fence_driver_name() and dma_fence_timeline_name():
>>
>> @@ -1167,7 +1167,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
>>  
>>         /* RCU protection is required for safe access to returned string */
>>         ops = rcu_dereference(fence->ops);
>> -       if (!dma_fence_test_signaled_flag(fence))
>> +       if (ops)
>>                 return (const char __rcu *)ops->get_driver_name(fence);
>>         else
>>                 return (const char __rcu *)"detached-driver";
>>
>> The problem is that we didn't considered that there a fence implementations which still have a release or wait callbacks but rely on not needing to return a string for a signaled fence.
>>
> 
> Could we move the signaled check to amdgpu and pvr?

Yes we could. I also considered that. But I would rather like to see it handled in the common dma_fence code.

If I remember correctly either Tvrko, you or somebody else was in favor of doing "if (!dma_fence_test_signaled_flag(fence) && ops)" here but I though that this was unnecessary and we would rather remove the release callbacks. Maybe I was wrong with that.
> IOW, we keep the solution presented here (removing ops->release for
> finished-fence) and the few drivers that check whether a fence is their
> own first do a locked dma_fence_is_signaled() check?

Works for me as well, but as I said I would rather like to keep it simple and stupid for backporting.

Regards,
Christian.

> 
> 
> P.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free
  2026-09-04 12:49       ` Christian König
@ 2026-09-04 19:06         ` Philipp Stanner
  0 siblings, 0 replies; 16+ messages in thread
From: Philipp Stanner @ 2026-09-04 19:06 UTC (permalink / raw)
  To: Christian König, phasta, Jonghyuk Kim(MalHyuk), tursulin,
	matthew.brost, dakr
  Cc: dri-devel, linux-kernel, mdaenzer, alessio.belle,
	luigi.santivetti, stable

On Fri, 2026-09-04 at 14:49 +0200, Christian König wrote:
> On 9/4/26 10:31, Philipp Stanner wrote:
> > On Fri, 2026-09-04 at 10:20 +0200, Christian König wrote:
> > > 

[…]

> > > 
> > > That sounds like a bad idea as well.
> > > 
> > > Dropping the fence->ops is to detach the fence from the module
> > > which originally issued it and not solve lifetime problems
> > > between the scheduler and the driver.
> > 
> > It can be used to solve that problem though, can it not?
> 
> Yes, but I think forcing dma_fence implementations to drop their
> release callback to fix lifetime problems with the driver and
> timeline name functions is a bad idea.

The issue we're facing is not so much related with life times, is it?

Even if the driver correctly tears down the scheduler (i.e., signal all
hardware fences), this problem still exists.

> 
> We should keep this fix simple and focused so that we can easily backport it.

My hope would be that the dma_fence backend solution can be used that
*already exists*. That could then both be simple and maintainable.


Correct me if I'm wrong, but it seems we have not found an alternative
solution that can work yet?

> 
> Fixing all dma_fence implementations to not need the release callback
> is something I really like to have as well, but not to fix this issue
> here. 
> 
> > 
> > The underlying problem is that the driver has no chance to figure out
> > when the scheduler is actually done with all the sched_fences.
> > 
> > Remember our lengthy discussions about drm_sched_fini(). Maybe we want
> > to reconsider providing a function with which the driver can wait until
> > the scheduler is done with all finished_fences?
> 
> The problem is that won't help unless we either add more checks or
> fix the checks in dma_fence_driver_name()/dma_fence_timeline_name().
> 
> The dma_fence object can trivially outlive both the driver and the
> scheduler instance it originally issued.

Right.

> 
> > > 
> > > I think we should rather re-consider patch
> > > 035219a760edb35ae9a9e96beba7f122e26a997b ("dma-buf: dma-fence:
> > > Fix potential NULL pointer dereference"):
> > > 
> > > Here we changed the check in dma_fence_driver_name() and
> > > dma_fence_timeline_name():
> > > 
> > > @@ -1167,7 +1167,7 @@ const char __rcu *dma_fence_driver_name(struct dma_fence *fence)
> > >  
> > >         /* RCU protection is required for safe access to returned string */
> > >         ops = rcu_dereference(fence->ops);
> > > -       if (!dma_fence_test_signaled_flag(fence))
> > > +       if (ops)
> > >                 return (const char __rcu *)ops->get_driver_name(fence);
> > >         else
> > >                 return (const char __rcu *)"detached-driver";
> > > 
> > > The problem is that we didn't considered that there a fence
> > > implementations which still have a release or wait callbacks but
> > > rely on not needing to return a string for a signaled fence.
> > > 
> > 
> > Could we move the signaled check to amdgpu and pvr?
> 
> Yes we could. I also considered that. But I would rather like to see
> it handled in the common dma_fence code.
> 
> If I remember correctly either Tvrko, you or somebody else was in
> favor of doing "if (!dma_fence_test_signaled_flag(fence) && ops)"
> here but I though that this was unnecessary and we would rather
> remove the release callbacks. Maybe I was wrong with that.

I hope that wasn't me because that again looks very racy.

I think that there is no way around using the spinlock. As I have
pointed out many times, the fact that the signaled-bit is set with lock
protection and read without it is fundamentally broken :(

> > IOW, we keep the solution presented here (removing ops->release for
> > finished-fence) and the few drivers that check whether a fence is
> > their
> > own first do a locked dma_fence_is_signaled() check?
> 
> Works for me as well, but as I said I would rather like to keep it
> simple and stupid for backporting.

If you can think of a stupid and simple solution, shoot. The only thing
I can think of is moving the string into the dma_fence, as a hard copy
:)


In the mean time, my proposal is to keep aiming for removing
sched_fence->ops->release and fixing pvr and amdgpu.

I think that will have to entail a signaled-check. Question would then
be whether we place it in the driver or the backend.


P.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-09-05 15:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04  8:06 [PATCH v4 0/3] drm/sched: fix use-after-free of the fence timeline name Jonghyuk Kim(MalHyuk)
2026-09-04  8:06 ` [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
2026-09-04  8:18   ` sashiko-bot
2026-09-04  8:20   ` Christian König
2026-09-04  8:31     ` Philipp Stanner
2026-09-04 12:49       ` Christian König
2026-09-04 19:06         ` Philipp Stanner
2026-09-04  8:31     ` Jonghyuk Kim(MalHyuk)
2026-09-04  8:39       ` Philipp Stanner
2026-09-04  9:11         ` Jonghyuk Kim(MalHyuk)
2026-09-04  9:07       ` Tvrtko Ursulin
2026-09-04  9:57   ` Danilo Krummrich
2026-09-04 10:51     ` Philipp Stanner
2026-09-04  8:06 ` [PATCH v4 2/3] drm/sched: add the fence ops-detach cleanup to the TODO list Jonghyuk Kim(MalHyuk)
2026-09-04  8:06 ` [PATCH v4 3/3] drm/sched/tests: add a UAF regression test for the timeline name Jonghyuk Kim(MalHyuk)
2026-09-04  8:15   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox