mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tursulin@ursulin.net>
To: "Jonghyuk Kim(MalHyuk)" <malhyuk97@gmail.com>,
	Matthew Brost <matthew.brost@intel.com>,
	Danilo Krummrich <dakr@kernel.org>,
	Philipp Stanner <phasta@kernel.org>
Cc: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH v1 1/2] drm/sched: cache the timeline name to fix a use-after-free
Date: Wed, 2 Sep 2026 10:46:25 +0100	[thread overview]
Message-ID: <8c2a4df5-37a4-414e-81e2-cd97218e5355@ursulin.net> (raw)
In-Reply-To: <b191f55e53492f95d965407f891fd93b0e74958d.1787928528.git.malhyuk97@gmail.com>


+ 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) <malhyuk97@gmail.com>
> ---
>   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.
>            */


  reply	other threads:[~2026-09-02  9:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28 14:57 [PATCH v1 0/2] drm/sched: fix a use-after-free in get_timeline_name() Jonghyuk Kim(MalHyuk)
2026-08-28 14:57 ` [PATCH v1 1/2] drm/sched: cache the timeline name to fix a use-after-free Jonghyuk Kim(MalHyuk)
2026-09-02  9:46   ` Tvrtko Ursulin [this message]
     [not found]     ` <CADQRuqtjBu7wR4kKVDcS42KLKQTr+7SBtRn24mgZ+ApettMr1A@mail.gmail.com>
2026-09-02 10:07       ` Philipp Stanner
2026-09-02 10:20         ` Tvrtko Ursulin
2026-09-02 11:39           ` Philipp Stanner
2026-09-02 13:38             ` Christian König
2026-08-28 14:57 ` [PATCH v1 2/2] drm/sched/tests: add a UAF regression test for get_timeline_name() Jonghyuk Kim(MalHyuk)
2026-09-02 10:04   ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8c2a4df5-37a4-414e-81e2-cd97218e5355@ursulin.net \
    --to=tursulin@ursulin.net \
    --cc=christian.koenig@amd.com \
    --cc=ckoenig.leichtzumerken@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=malhyuk97@gmail.com \
    --cc=matthew.brost@intel.com \
    --cc=phasta@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®