accel/amdxdna: RCU-free the scheduler-containing hwctx private
PATCH 1/1 accel/amdxdna: RCU-free the scheduler-containing hwctx private
struct amdxdna_hwctx_priv embeds a struct drm_gpu_scheduler (priv->sched). aie2_hwctx_fini() calls drm_sched_fini(&priv->sched) and then frees the whole object with plain kfree(hwctx->priv).
Every drm_sched_fence produced by that scheduler stores fence->sched = &priv->sched, and drm_sched_fence_get_timeline_name() returns fence->sched->name. The scheduler fence ops keep a .release callback, so the fence is not ops-detached on signalling: a finished fence that userspace still holds (exported via drm_syncobj / sync_file) keeps pointing at priv->sched after the hwctx is torn down. A later get_timeline_name() -- reachable unprivileged through SYNC_IOC_FILE_INFO on the exported sync_file -- then dereferences priv->sched->name in freed slab memory (KASAN slab-use-after-free read).
This is the amdxdna instance of the dma-fence lifetime contract: the exporter must keep the driver data backing a fence alive for an RCU grace period after the fence is signalled, so a concurrent rcu_read_lock'd dma_fence_timeline_name() cannot observe freed memory. aie2_hwctx_fini() already waits for all submitted jobs to complete/cancel, so the fences are signalled by teardown time; only the teardown race window remains, which an RCU-delayed free closes.
Free the scheduler-containing private with kfree_rcu() instead of kfree(). The init-failure unwind keeps plain kfree(): no job has been submitted there, so no drm_sched_fence has been exported.
--- drivers/accel/amdxdna/aie2_ctx.c | 2 +- drivers/accel/amdxdna/aie2_pci.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)
drivers/accel/amdxdna/aie2_ctx.c +1 -1
@@ -842,7 +842,7 @@ void aie2_hwctx_fini(struct amdxdna_hwctx *hwctx)mutex_destroy (&hwctx->priv->io_lock);kfree (hwctx->col_list); -kfree (hwctx->priv); +kfree_rcu (hwctx->priv, rcu);kfree (hwctx->cus); }
drivers/accel/amdxdna/aie2_pci.h +2 -0
@@ -107,6 +107,8 @@ struct amdxdna_hwctx_priv {struct amdxdna_gem_obj *heap;void *mbox_chann; +struct rcu_head rcu; +struct drm_gpu_scheduler sched;struct drm_sched_entity entity;
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] kfree_rcu() is insufficient to prevent Use-After-Free on fence->sched->name because exported fences can arbitrarily outlive the RCU grace period.
- [High] Synchronous freeing of hwctx and hwctx->name causes Use-After-Free when amdxdna_fence_get_timeline_name() accesses them.
- [High] Use-after-free of job->hwctx and job->hwctx->priv in aie2_job_release() due to a TOCTOU race with waitqueue evaluation. --
commit 136ff3c744e0784aadc2c898c0217b9fc55732e6
Author: Jonghyuk Kim(MalHyuk) <[email protected]>
Subject: accel/amdxdna: RCU-free the scheduler-containing hwctx private
This commit modifies aie2_hwctx_fini() to free the scheduler-containing private struct with kfree_rcu() instead of kfree(). It aims to address a dma-fence lifetime contract violation where exported fences can outlive the hardware context, causing a KASAN slab-use-after-free on priv->sched->name.
drivers/accel/amdxdna/aie2_ctx.c void aie2_hwctx_fini(struct amdxdna_hwctx *hwctx) · line 846
Quoted 10 lines
drivers/accel/amdxdna/aie2_ctx.c +1
-1@@ -843,7 +843,7 @@ void aie2_hwctx_fini(struct amdxdna_hwctx *hwctx)mutex_destroy (&hwctx->priv->io_lock);kfree (hwctx->col_list); -kfree (hwctx->priv); +kfree_rcu (hwctx->priv, rcu);
[Severity: High]
This isn't a bug introduced by this patch, but is kfree_rcu() sufficient to prevent a Use-After-Free on fence->sched->name?
Exported fences can arbitrarily outlive the RCU grace period. If userspace exports the syncobj to a sync_file fd via DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD, then destroys the hardware context, hwctx->priv is freed after an RCU grace period. However, if userspace later calls SYNC_IOC_FILE_INFO on the sync_file fd, it will invoke dma_fence_timeline_name() and dereference the dangling fence->sched pointer. The RCU grace period only protects against concurrent lookups, not against prolonged userspace holds on the fence.
drivers/accel/amdxdna/aie2_ctx.c void aie2_hwctx_fini(struct amdxdna_hwctx *hwctx) · line 847
Quoted 2 lines
drivers/accel/amdxdna/aie2_ctx.c +0
-0kfree (hwctx->cus); }
[Severity: High]
This is a pre-existing issue, but is there a TOCTOU race with waitqueue evaluation between aie2_job_release() and aie2_hwctx_fini() that leads to a Use-After-Free?
In aie2_job_release(), a thread could increment the counter and get preempted before calling wake_up():
drivers/accel/amdxdna/aie2_ctx.c:aie2_job_release() {
atomic64_inc(&job->hwctx->job_free_cnt);
// <--- Preempted here
wake_up(&job->hwctx->priv->job_free_wq);
}Concurrently, aie2_hwctx_fini() is waiting:
drivers/accel/amdxdna/aie2_ctx.c:aie2_hwctx_fini() {
wait_event(hwctx->priv->job_free_wq,
atomic64_read(&hwctx->job_submit_cnt) ==
atomic64_read(&hwctx->job_free_cnt));
...
}The wait_event condition locklessly evaluates to true and returns immediately. Context destruction continues, and hwctx is freed. When the preempted thread resumes, it calls wake_up(&job->hwctx->priv->job_free_wq), which writes to the freed waitqueue spinlock, potentially causing spinlock corruption and a panic.
[Severity: High]
This is a pre-existing issue, but does synchronous freeing of hwctx and hwctx->name in amdxdna_hwctx_destroy_rcu() also cause a Use-After-Free?
Similar to the above issue, if userspace holds a sync_file and destroys the context, amdxdna_hwctx_destroy_rcu() synchronously frees hwctx and hwctx->name:
drivers/accel/amdxdna/amdxdna_ctx.c:amdxdna_hwctx_destroy_rcu() {
...
kfree(hwctx->name);
kfree(hwctx);
}When the user later closes the sync_file fd, the reference count of the drm_sched_fence drops to zero, triggering dma_fence_release(). The tracepoint trace_dma_fence_destroy() then calls amdxdna_fence_get_timeline_name(), which dereferences the freed hwctx->name:
drivers/accel/amdxdna/amdxdna_ctx.c:amdxdna_fence_get_timeline_name() {
return xdna_fence->hwctx->name;
}Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1