[PATCH] accel/amdxdna: RCU-free the scheduler-containing hwctx private

[PATCH] accel/amdxdna: RCU-free the scheduler-containing hwctx private

[Jonghyuk Kim(MalHyuk)]

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.

Fixes: be462c97b7df ("accel/amdxdna: Add hardware context")
Cc: stable@vger.kernel.org
Signed-off-by: Jonghyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 drivers/accel/amdxdna/aie2_ctx.c | 2 +-
 drivers/accel/amdxdna/aie2_pci.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
index 4b3a62aa8798..26bd6e001792 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -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);
 }
 
diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
index ea1dac106400..e08b8f64328c 100644
--- a/drivers/accel/amdxdna/aie2_pci.h
+++ b/drivers/accel/amdxdna/aie2_pci.h
@@ -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;
 

Re: [PATCH] accel/amdxdna: RCU-free the scheduler-containing hwctx private

[sashiko-bot]

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

> diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c
> index baf9a8b90a4d6..cef750007b47d 100644
> --- a/drivers/accel/amdxdna/aie2_ctx.c
> +++ b/drivers/accel/amdxdna/aie2_ctx.c
> @@ -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.

>  	kfree(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;
}

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