drm/nouveau: RCU-free the scheduler-containing nouveau_sched
Subsystems (4)
PATCH 1/1 drm/nouveau: RCU-free the scheduler-containing nouveau_sched
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().
--- drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +- drivers/gpu/drm/nouveau/nouveau_sched.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
drivers/gpu/drm/nouveau/nouveau_sched.c +1 -1
@@ -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 ; }
drivers/gpu/drm/nouveau/nouveau_sched.h +1 -0
@@ -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;
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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) <[email protected]>
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.
drivers/gpu/drm/nouveau/nouveau_sched.c nouveau_sched_destroy(struct nouveau_sched **psched) · line 520
Quoted 10 lines
drivers/gpu/drm/nouveau/nouveau_sched.c +1
-1@@ -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?
Quoted 3 lines
drivers/gpu/drm/nouveau/nouveau_sched.c +0
-0*psched =NULL ; }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Will push to drm-misc-fixes in just a moment
Quoted 60 lines · Jonghyuk Kim(MalHyuk)
struct
nouveau_schedembeds a structdrm_gpu_scheduler(base).nouveau_sched_destroy()callsnouveau_sched_fini()(which doesdrm_sched_fini(&sched->base)) and then frees the object with plain kfree(sched).
drm_sched_fence_get_timeline_name()returnsfence->sched->name, and the
scheduler fence keeps a .release callback so it is not ops-detached on
signalling. A finished fence exported to userspace viadrm_syncobj/sync_filetherefore keeps pointing at&sched->baseafternouveau_sched_destroy(),
and a laterget_timeline_name()-- reachable unprivileged throughSYNC_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 withkfree_rcu()instead ofkfree().Fixes 5f03a507b29e ("drm/nouveau: implement 1:1 scheduler - entityrelationship")
Signed-off-by Jonghyuk Kim(MalHyuk) <[email protected]>---
drivers/gpu/drm/nouveau/nouveau_sched.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_sched.h | 1 +2 files changed, 2 insertions(+), 1 deletion(-)drivers/gpu/drm/nouveau/nouveau_sched.c +0
-0@@ -516,7 +516,7 @@ nouveau_sched_destroy(struct nouveau_sched **psched) struct nouveau_sched *sched = *psched;
nouveau_sched_fini(sched);drivers/gpu/drm/nouveau/nouveau_sched.c +1
-1-kfree (sched); +kfree_rcu (sched, rcu);
*psched = NULL;
}drivers/gpu/drm/nouveau/nouveau_sched.h +0
-0@@ -98,6 +98,7 @@ void nouveau_job_free(struct nouveau_job *job);
structnouveau_sched{
structdrm_gpu_schedulerbase;drivers/gpu/drm/nouveau/nouveau_sched.h +1
-0+struct rcu_head rcu;struct
drm_sched_entityentity;
structworkqueue_struct*wq;
struct mutex mutex;