Re: [PATCH v3 0/2] drm/sched: fix use-after-free of the fence timeline name

[Philipp Stanner]

Well, that was a quick investigation ;)

On Wed, 2026-09-02 at 23:42 +0900, Jonghyuk Kim(MalHyuk) wrote:
> 

[…]

> Philipp suggested dropping the finished fence's ->release callback instead.
> That is what this series does. dma_fence detaches a fence's ops on signalling
> when it has neither .release nor .wait (dma_fence_signal_timestamp_locked()),
> and dma_fence_timeline_name() returns a static string once the ops are gone.
> So with the callback removed, get_timeline_name() is simply never reached on
> a signalled finished fence - no ->sched dereference at all, for static and

btw, you only ever mention get_timeline_name(), but get_driver_name()
is running into the same issue, isn't it?

> Link to v2 (name caching):
> https://lore.kernel.org/dri-devel/20260902105808.1541063-1-malhyuk97@xxxxxxxxx/

That link is dead (weirdly enough. Why isn't it in dri-devel?). Correct
one seems to be:

https://lore.kernel.org/lkml/20260902105808.1541063-1-malhyuk97@xxxxxxxxx/

Your help and industriousness is highly appreciated :)
Just be so kind and wait >24h with sending new revisions so that more
folks, especially from different time zones, can jump into the
discussion.

> 
> Note: detaching the finished fence's ops on signalling also makes
> to_drm_sched_fence() return NULL for a signalled finished fence. Callers
> already handle NULL (the normal foreign-fence result), a signalled fence is
> an already-satisfied dependency so the scheduler's dependency collapsing is
> unaffected, and it avoids the container_of() on a possibly-freed foreign
> scheduler that amdgpu_sync_same_dev() and pvr_queue_fence_is_native() would
> otherwise do. Flagging it explicitly since it touches an exported helper.

That unfortunately does look a bit dangerous.

Isn't pvr here already a race condition?

		if (pvr_queue_fence_is_native(uf)) {
			struct drm_sched_fence *s_fence = to_drm_sched_fence(uf);

> I did not add Fixes:/Cc: stable tags: the ->sched->name deref dates back to
> 1b1f42d8fde4 ("drm: move amd_gpu_scheduler into common location") but only
> became reachable once drivers began allocating per-context schedulers, so
> the right attribution is unclear to me. This is stable material as the
> driver instances are live - happy to add whatever tags you prefer.

I think for such cases merely adding Cc: stable and let the stable
folks figure out how far they want to backport is fine. You can hint at
us not knowing since when userspace can access this in a commit

Cc: stable … # we don't know since when

What I'm a bit more nervous about is that we probably really want to
backport this, but it's also a bit regression-endangered. So I suppose
we want to give it careful testing. I hope the others can help with
that, too.

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

Did you test with kmemleak? That's always a tool of choice when it
comes to refcounting.

> 
> Jonghyuk Kim(MalHyuk) (2):
>   drm/sched: fix use-after-free of the fence timeline name
>   drm/sched/tests: add a UAF regression test for the timeline name

I answer on those soonish.



Thanks
Philipp