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

[Philipp Stanner]

On Mon, 2026-09-07 at 10:15 +0100, Tvrtko Ursulin wrote:
> 
> 
> On 04/09/2026 20:06, Philipp Stanner wrote:
> 
> 8><
> 
> > 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.
> 
> Fixing the drivers sounds like an obvious thing to try indeed. Along the 
> same lines as it was done for xe and panthor. It is an already 
> established and well understood approach so shouldn't be controversial. 
> After that we can discuss in leisurely pace if something better is 
> possible in the scheduler core.
> 
> I understand its amdxdna, nouveau, and msm. Was it attempted so far? Is 
> it significantly more complicated than it was for panthor and xe?

How did the others fix that?

If we look at nouveau:

static void
nouveau_sched_fini(struct nouveau_sched *sched)
{
 struct drm_gpu_scheduler *drm_sched = &sched->base;
 struct drm_sched_entity *entity = &sched->entity;

 wait_event(sched->job.wq, nouveau_sched_job_list_empty(sched));

 drm_sched_entity_fini(entity);
 drm_sched_fini(drm_sched);

 /* Destroy workqueue after scheduler tear down, otherwise it might still
 * be in use.
 */
 if (sched->wq)
 destroy_workqueue(sched->wq);
}


We see that it
   1. stops accepting jobs from userspace (not visible here)
   2. waits until all hardware fences in this ring are signaled
   3. only then tears down drm_sched

Then nouveau might unload or free up resources.

The nasty thing is that I don't see how nouveau misbehaves here and how
the stuff might be fixed.

The problem is that the sched_fence implements ops->release, so the
check doesn't take effect.

Moreover, even if we did remove ops->release in drm_sched, it would
still be a race: a driver's contract is the hardware_fence, the rule
being that you have to signal those. So after signaling the last
hardware_fence, you could actually start releasing resources, but it
might be that finished_fences are still in-flight and are unsignaled.

So we have some sort of fence -> fence race here, too.

> 
> As for regarding the 035219a760ed ("dma-buf: dma-fence: Fix potential
> NULL pointer dereference") sub-thread - I did not manage to penetrate
> the consensus there - whether it was established that it needs adding
> the is signaled check back (with additional memory barriers, like v1 of 
> that patch) or not? Regardless of fixing the drivers or what?


As far as my understanding goes this is the only way to get this right
for everyone, i.e. also users who implement ops->release(). Then at
least the driver could unload after signalling all its fences (with the
exception of those who have a shared spinlock maybe).


However, I suppose then we would then have two mechanisms, one dancing
with RCU around the ops pointer, the other checking whether the fence
is signaled, presumably with manual ordering through barriers.

My first guess would be that maybe we should only rely on the signaled-
state and leave the ops-pointer untouched? This should also work for
pvr, notably.


P.