On 02/09/2026 10:46, Tvrtko Ursulin wrote: > It is not guaranteed in the documented contract that the name passed to > drm_sched_init has to outlive the scheduler.
Right. Caching the bare pointer only works because every in-tree driver passes a string literal today - xe's q->name (freed with the exec queue, hence 299bc6d50b1b) is the counter-example where v1 would still dangle. > Hm, that might be overkill.. how about we just keep a copy of the name > in the scheduler object? The catch is the scheduler object is itself freed on context teardown, so a copy that lives there dangles for the exported fence just the same. To actually stop dereferencing ->sched the copy has to live in the fence - kstrdup in drm_sched_fence_init(), freed from the fence release. That's an alloc per fence on the submit path though. If that overhead isn't wanted, the lighter option is to keep the pointer and document in gpu_scheduler.h that the drm_sched_init() name must follow the dma-fence safe access rules (outlive any exported fence). That matches what the already-fixed drivers do and leaves the submit path untouched. Either one fixes amdxdna/nouveau/msm in the core. I'd lean to the documented-pointer version unless you'd rather pay the kstrdup - let me know which and I'll respin as a core-only series (fix + the kunit test). Thanks for the 6bd90e700b42/299bc6d50b1b context, that clears up what the half-fix missed. Jonghyuk Kim(MalHyuk) On Wed, Sep 02, 2026 06:46 PM, Tvrtko Ursulin <[email protected]> wrote: > > + Christian's AMD email > > On 28/08/2026 15:57, 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. > > > > Scheduler names are persistent strings (string literals passed to > > drm_sched_init()), so cache the 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. > > It is not guaranteed in the documented contract that the name passed to > drm_sched_init has to outlive the scheduler. > > Case in point is asking "would have this patch fixed CVE-2025-38703" - > to which I think answer is no. Until recent 299bc6d50b1b ("drm/xe/guc: > Keep scheduler timeline name alive") my attempt in 6bd90e700b42 > ("drm/xe: Make dma-fences compliant with the safe access rules") only > half fixed it by adding a RCU grace to the sched object itself, while > missing the fact timeline name gets freed instantly. I haven't gotten > round trying to understand why KASAN did not catch this back when I was > upstreaming 6bd90e700b42. > > Anyway, I think the fix will have to be to either add the full RCU grace > around names in the scheduler object owning modules, or your patch plus > documenting that the name passed to drm_sched_init must follow the > dma-fence safe access rules (with kernel-doc cross-link and an > explanation of the connection between scheduler name and dma-fence > timeline name). > > Hm, that might be overkill.. how about we just keep a copy of the name > in the scheduler object? If we document that in gpu_scheduler.h it would > fix the drivers still vulnerable, while the ones which were fixed could > be simplified. > > Regards, > > Tvrtko > > > Signed-off-by: Jonghyuk Kim(MalHyuk) <[email protected]> > > --- > > drivers/gpu/drm/scheduler/sched_fence.c | 16 +++++++++++++++- > > include/drm/gpu_scheduler.h | 11 +++++++++++ > > 2 files changed, 26 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/scheduler/sched_fence.c > b/drivers/gpu/drm/scheduler/sched_fence.c > > index 85ee3d694dc9..280a2d036ffd 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) > > @@ -228,6 +234,14 @@ void drm_sched_fence_init(struct drm_sched_fence > *fence, > > unsigned seq; > > > > fence->sched = container_of(entity->rq, typeof(*fence->sched), rq); > > + /* > > + * 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. Scheduler names are persistent (string > > + * literals passed to drm_sched_init()). > > + */ > > + 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 d61c19e78182..4f780d70acb6 100644 > > --- a/include/drm/gpu_scheduler.h > > +++ b/include/drm/gpu_scheduler.h > > @@ -305,6 +305,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. > > */ > >
