Re: [PATCH V1] accel/amdxdna: Fix use-after-free in debug BO command handling

[Lizhi Hou]

Applied to drm-misc-fixes

On 7/1/26 14:28, Mario Limonciello wrote:

On 7/1/26 10:55, Lizhi Hou wrote:

When a debug BO command completes, job->drv_cmd may already have been
freed. Accessing it from aie2_sched_drvcmd_resp_handler() can result in
a use-after-free and memory corruption.

Fix this by introducing reference counting for drv_cmd objects and
transferring ownership to the job while it is in flight. This ensures
that the command remains valid until the completion handler finishes
processing it.

Fixes: 7ea046838021 ("accel/amdxdna: Support firmware debug buffer")
Signed-off-by: Lizhi Hou <lizhi....@amd.com>

Reviwed-by: Mario Limonciello (AMD) <supe...@kernel.org>

---
  drivers/accel/amdxdna/aie2_ctx.c    | 68 +++++++++++++++++++++--------
  drivers/accel/amdxdna/amdxdna_ctx.h |  1 +
  2 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_ctx.c b/drivers/accel/amdxdna/aie2_ctx.c

index 55eb29dece5b..8ec8a4d69b14 100644
--- a/drivers/accel/amdxdna/aie2_ctx.c
+++ b/drivers/accel/amdxdna/aie2_ctx.c
@@ -59,6 +59,18 @@ static bool aie2_tdr_detect(struct amdxdna_dev *xdna)
      return false;
  }
  +static void aie2_cmd_release(struct kref *ref)
+{

+    struct amdxdna_drv_cmd *drv_cmd = container_of(ref, struct amdxdna_drv_cmd, refcnt);

+
+    kfree(drv_cmd);
+}
+
+static void aie2_cmd_put(struct amdxdna_drv_cmd *drv_cmd)
+{
+    kref_put(&drv_cmd->refcnt, aie2_cmd_release);
+}
+
  static void aie2_job_release(struct kref *ref)
  {
      struct amdxdna_sched_job *job;
@@ -70,6 +82,8 @@ static void aie2_job_release(struct kref *ref)
      wake_up(&job->hwctx->priv->job_free_wq);
      if (job->out_fence)
          dma_fence_put(job->out_fence);
+    if (job->drv_cmd)
+        aie2_cmd_put(job->drv_cmd);
      kfree(job->aie2_job_health);
      kfree(job);
  }

@@ -901,7 +915,7 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,

  {
      struct amdxdna_client *client = hwctx->client;
      struct amdxdna_dev *xdna = client->xdna;
-    struct amdxdna_drv_cmd cmd = { 0 };
+    struct amdxdna_drv_cmd *cmd;
      struct amdxdna_gem_obj *abo;
      u64 seq;
      int ret;

@@ -912,32 +926,39 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,

          return -EINVAL;
      }
  +    cmd = kzalloc_obj(*cmd);
+    if (!cmd) {
+        ret = -ENOMEM;
+        goto put_obj;
+    }
+    kref_init(&cmd->refcnt);
+
      if (attach) {
          if (abo->assigned_hwctx != AMDXDNA_INVALID_CTX_HANDLE) {
              ret = -EBUSY;
-            goto put_obj;
+            goto put_cmd;
          }
-        cmd.opcode = ATTACH_DEBUG_BO;
+        cmd->opcode = ATTACH_DEBUG_BO;
      } else {
          if (abo->assigned_hwctx != hwctx->id) {
              ret = -EINVAL;
-            goto put_obj;
+            goto put_cmd;
          }
-        cmd.opcode = DETACH_DEBUG_BO;
+        cmd->opcode = DETACH_DEBUG_BO;
      }
  -    ret = amdxdna_cmd_submit(client, &cmd, AMDXDNA_INVALID_BO_HANDLE,
+    ret = amdxdna_cmd_submit(client, cmd, AMDXDNA_INVALID_BO_HANDLE,
                   &bo_hdl, 1, hwctx->id, &seq);
      if (ret) {
          XDNA_ERR(xdna, "Submit command failed");
-        goto put_obj;
+        goto put_cmd;
      }
        aie2_cmd_wait(hwctx, seq);
-    if (cmd.result) {
-        XDNA_ERR(xdna, "Response failure 0x%x", cmd.result);
+    if (cmd->result) {
+        XDNA_ERR(xdna, "Response failure 0x%x", cmd->result);
          ret = -EINVAL;
-        goto put_obj;
+        goto put_cmd;
      }
        if (attach)

@@ -947,6 +968,8 @@ static int aie2_hwctx_cfg_debug_bo(struct amdxdna_hwctx *hwctx, u32 bo_hdl,

        XDNA_DBG(xdna, "Config debug BO %d to %s", bo_hdl, hwctx->name);
  +put_cmd:
+    aie2_cmd_put(cmd);
  put_obj:
      amdxdna_gem_put_obj(abo);
      return ret;

@@ -974,25 +997,32 @@ int aie2_hwctx_sync_debug_bo(struct amdxdna_hwctx *hwctx, u32 debug_bo_hdl)

  {
      struct amdxdna_client *client = hwctx->client;
      struct amdxdna_dev *xdna = client->xdna;
-    struct amdxdna_drv_cmd cmd = { 0 };
+    struct amdxdna_drv_cmd *cmd;
      u64 seq;
      int ret;
  -    cmd.opcode = SYNC_DEBUG_BO;
-    ret = amdxdna_cmd_submit(client, &cmd, AMDXDNA_INVALID_BO_HANDLE,
+    cmd = kzalloc_obj(*cmd);
+    if (!cmd)
+        return -ENOMEM;
+    kref_init(&cmd->refcnt);
+
+    cmd->opcode = SYNC_DEBUG_BO;
+    ret = amdxdna_cmd_submit(client, cmd, AMDXDNA_INVALID_BO_HANDLE,
                   &debug_bo_hdl, 1, hwctx->id, &seq);
      if (ret) {
          XDNA_ERR(xdna, "Submit command failed");
-        return ret;
+        goto put_cmd;
      }
        aie2_cmd_wait(hwctx, seq);
-    if (cmd.result) {
-        XDNA_ERR(xdna, "Response failure 0x%x", cmd.result);
-        return -EINVAL;
+    if (cmd->result) {
+        XDNA_ERR(xdna, "Response failure 0x%x", cmd->result);
+        ret = -EINVAL;
      }
  -    return 0;
+put_cmd:
+    aie2_cmd_put(cmd);
+    return ret;
  }
    static int aie2_populate_range(struct amdxdna_gem_obj *abo)

@@ -1139,6 +1169,8 @@ int aie2_cmd_submit(struct amdxdna_hwctx *hwctx, struct amdxdna_sched_job *job,           dma_resv_add_fence(job->bos[i]->resv, job->out_fence, DMA_RESV_USAGE_WRITE);

      job->seq = hwctx->priv->seq++;
      kref_get(&job->refcnt);
+    if (job->drv_cmd)
+        kref_get(&job->drv_cmd->refcnt);
      drm_sched_entity_push_job(&job->base);
        *seq = job->seq;

diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h

index aaae16430466..b6bef3af7dab 100644
--- a/drivers/accel/amdxdna/amdxdna_ctx.h
+++ b/drivers/accel/amdxdna/amdxdna_ctx.h
@@ -132,6 +132,7 @@ enum amdxdna_job_opcode {
  struct amdxdna_drv_cmd {
      enum amdxdna_job_opcode    opcode;
      u32            result;
+    struct kref        refcnt;
  };
    struct app_health_report;