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: [email protected] # we don't know since when
> Signed-off-by: Jonghyuk Kim(MalHyuk) <[email protected]>
> ---
> 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.
