[PATCH] drm/nouveau: RCU-free the scheduler-containing nouveau_sched

[PATCH] drm/nouveau: RCU-free the scheduler-containing nouveau_sched

[Jonghyuk Kim(MalHyuk)]

struct nouveau_sched embeds a struct drm_gpu_scheduler (base).
nouveau_sched_destroy() calls nouveau_sched_fini() (which does
drm_sched_fini(&sched->base)) and then frees the object with plain
kfree(sched).

drm_sched_fence_get_timeline_name() returns fence->sched->name, and the
scheduler fence keeps a .release callback so it is not ops-detached on
signalling.  A finished fence exported to userspace via drm_syncobj /
sync_file therefore keeps pointing at &sched->base after nouveau_sched_destroy(),
and a later get_timeline_name() -- reachable unprivileged through
SYNC_IOC_FILE_INFO -- dereferences freed memory (KASAN slab-use-after-free
read).

Per the dma-fence lifetime contract the exporter must keep the data backing a
signalled fence alive for an RCU grace period.  Free the scheduler-containing
object with kfree_rcu() instead of kfree().

Fixes: 5f03a507b29e ("drm/nouveau: implement 1:1 scheduler - entity relationship")
Cc: stable@vger.kernel.org
Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
 drivers/gpu/drm/nouveau/nouveau_sched.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c
index 2cbae003d6de..48dbb851df2f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
@@ -516,7 +516,7 @@ nouveau_sched_destroy(struct nouveau_sched **psched)
 	struct nouveau_sched *sched = *psched;
 
 	nouveau_sched_fini(sched);
-	kfree(sched);
+	kfree_rcu(sched, rcu);
 
 	*psched = NULL;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.h b/drivers/gpu/drm/nouveau/nouveau_sched.h
index 20cd1da8db73..51ce8dcf6285 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sched.h
+++ b/drivers/gpu/drm/nouveau/nouveau_sched.h
@@ -98,6 +98,7 @@ void nouveau_job_free(struct nouveau_job *job);
 
 struct nouveau_sched {
 	struct drm_gpu_scheduler base;
+	struct rcu_head rcu;
 	struct drm_sched_entity entity;
 	struct workqueue_struct *wq;
 	struct mutex mutex;

Re: [PATCH] drm/nouveau: RCU-free the scheduler-containing nouveau_sched

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free (Incomplete Fix)
--

commit a4cb3c10462917197ec6f8cdfe7a265ad10ef18a
Author: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>

drm/nouveau: RCU-free the scheduler-containing nouveau_sched

This commit attempts to fix a use-after-free issue by deferring the freeing
of the scheduler object to an RCU callback.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_sched.c b/drivers/gpu/drm/nouveau/nouveau_sched.c
> index 8b9f935afe094..b3f02c490ecb6 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_sched.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_sched.c
> @@ -517,7 +517,7 @@ nouveau_sched_destroy(struct nouveau_sched **psched)
>  	struct nouveau_sched *sched = *psched;
>  
>  	nouveau_sched_fini(sched);
> -	kfree(sched);
> +	kfree_rcu(sched, rcu);

[Severity: High]
This isn't a bug introduced by this patch, but does this actually fix the
underlying use-after-free issue?

Using kfree_rcu() only defers freeing the scheduler by a single RCU grace
period. However, dma_fence objects exported to userspace (e.g., via
sync_file) can outlive this grace period since userspace controls the file
descriptor lifetime.

When userspace eventually queries the fence info, sync_fill_fence_info()
acquires a new rcu_read_lock() and calls dma_fence_timeline_name(), which
invokes drm_sched_fence_get_timeline_name():

drivers/gpu/drm/scheduler/sched_fence.c:drm_sched_fence_get_timeline_name() {
    ...
    return (const char *)fence->sched->name;
}

Because the nouveau_sched memory is physically freed after the initial RCU
grace period elapses, this new RCU read-side critical section does not
protect it. Won't this unconditionally dereference fence->sched->name on
the already-freed memory, resulting in the exact same use-after-free read
this patch intends to fix?

>  
>  	*psched = NULL;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902012717.880724-1-malhyuk97@gmail.com?part=1