Re: [PATCH v4 1/3] drm/sched: cache the timeline name to fix a use-after-free

[Tvrtko Ursulin]

On 07/09/2026 12:06, Tvrtko Ursulin wrote:
> On 07/09/2026 11:47, Philipp Stanner wrote:
> > On Mon, 2026-09-07 at 11:28 +0100, Tvrtko Ursulin wrote:
> > > >
> >
> > […]
> >
> > > > How did the others fix that?
> > >
> > > Combination of kfree_rcu, synchronize_rcu and storing the name in an
> > > object protected by those:
> >
> > […]
> >
> > > For the scheduler (nouveau_sched_destroy()) the same, kfree_rcu.
> > >
> > > That makes scheduler timeline name vfunc safe:
> > >
> > > 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;
> > >
> > > sched is then RCU protected. sched->name is already static so not a 
> > > concern.
> > >
> > > As you say nouveau_sched_fini() only tears down the scheduler after
> > > fences have been signaled it seems adding two new kfree_rcu make is 
> > > safe.
> >
> >
> > I don't see how any RCU mechanism would make anything here safe.
> >
> > As long as ops->release is implemented, ops will never be set to NULL,
> > (with or without RCU) – so everyone who holds a reference to finished-
> > fence can still run into drm_sched_fence_get_timeline_name() and cause
> > UAF on the sched pointer *and* the name pointer.
> >
> > Moreover, all of this falls apart should the driver unload.
> >
> > So I maintain the position that we only get it right by having the
> > signaled-bit be the decoupling point.
> >
> > Or am I missing something?
>
> The driver fixes you asked about and I listed work(ed) in the context of 
> 506aa8b02a8d ("dma-fence: Add safe access helpers and document the 
> rules"). In that "world" it would have been safe. Did 035219a760ed 
> ("dma-buf: dma-fence: Fix potential NULL pointer dereference") break 
> those fixes a bit?
>
> I can try my IGT and see..

With RCU frees in xe it is harder to hit it reliably for KASAN to 
notice. Guess I would need to have a way to flush the RCU callbacks from 
IGT but I did not bother with that, it seemed easier to check the 
returned name. Since it is not returning "detached-driver" / 
"detached-timeline" post signalling that proves things are indeed broken:

$ sudo tests/xe_sync_file
IGT-Version: 2.5-ge37a85b91 (x86_64) (Linux: 7.3.0-rc1+ x86_64)
Using IGT_SRANDOM=1788784104 for randomisation
Opened device: /dev/dri/card0
Starting subtest: sync_file_race
[2178.517756] (xe_sync_file:10970) CRITICAL: Test assertion failure 
function test_race, file ../tests/intel/xe_sync_file.c:130:
[2178.517790] (xe_sync_file:10970) CRITICAL: Failed assertion: 
!strcmp(driver_name, "detached-driver")
Stack trace:
  #0 ../lib/igt_core.c:2089 __igt_fail_assert()
  #1 ../tests/intel/xe_sync_file.c:155 __igt_unique____real_main136()
  #2 ../tests/intel/xe_sync_file.c:136 main()
  #3 ../sysdeps/nptl/libc_start_call_main.h:83 __libc_start_call_main()
  #4 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
  #5 [_start+0x25]
Subtest sync_file_race failed.
**** DEBUG ****
[2177.516393] (xe_sync_file:10970) DEBUG: 'drm_sched'/'rcs16' = 1
[2178.517745] (xe_sync_file:10970) DEBUG: 'drm_sched'/'rcs16' = 1
[2178.517756] (xe_sync_file:10970) CRITICAL: Test assertion failure 
function test_race, file ../tests/intel/xe_sync_file.c:130:
[2178.517790] (xe_sync_file:10970) CRITICAL: Failed assertion: 
!strcmp(driver_name, "detached-driver")
[2178.518762] (xe_sync_file:10970) igt_core-INFO: Stack trace:
[2178.523313] (xe_sync_file:10970) igt_core-INFO:   #0 
../lib/igt_core.c:2089 __igt_fail_assert()
[2178.523428] (xe_sync_file:10970) igt_core-INFO:   #1 
../tests/intel/xe_sync_file.c:155 __igt_unique____real_main136()
[2178.523436] (xe_sync_file:10970) igt_core-INFO:   #2 
../tests/intel/xe_sync_file.c:136 main()
[2178.564635] (xe_sync_file:10970) igt_core-INFO:   #3 
../sysdeps/nptl/libc_start_call_main.h:83 __libc_start_call_main()
[2178.564690] (xe_sync_file:10970) igt_core-INFO:   #4 
../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
[2178.564892] (xe_sync_file:10970) igt_core-INFO:   #5 [_start+0x25]
****  END  ****
Subtest sync_file_race: FAIL (1.054s)

I'll copy you on the updated IGT for reference.

Regards,

Tvrtko