Re: [PATCH V2] accel/amdxdna: Fix potential NULL pointer dereference of abo->client

[Max Zhen]

On 7/7/2026 Tue 13:15, Lizhi Hou wrote:

Closing a BO handle clears abo->client, while the underlying GEM object
may remain alive due to internal kernel references. As a result, code
executed after the BO handle is closed may dereference a NULL abo->client
pointer.

Remove accesses to abo->client from code paths that may execute after the
BO handle has been closed.

Fixes: d76856beb4a4 ("accel/amdxdna: Refactor GEM BO handling and add helper APIs 
for address retrieval")
Signed-off-by: Lizhi Hou <lizhi....@amd.com>

Reviewed-by: Max Zhen <max.z...@amd.com>

---
  drivers/accel/amdxdna/aie2_message.c |  4 ++--
  drivers/accel/amdxdna/amdxdna_gem.c  | 11 +++++++++--
  drivers/accel/amdxdna/amdxdna_gem.h  | 11 +++++++++--
  3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_message.c 
b/drivers/accel/amdxdna/aie2_message.c
index c4b364801cc0..dfe0fbdf066d 100644
--- a/drivers/accel/amdxdna/aie2_message.c
+++ b/drivers/accel/amdxdna/aie2_message.c
@@ -840,7 +840,7 @@ static struct aie2_exec_msg_ops npu_exec_message_ops = {
  static int aie2_init_exec_req(void *req, struct amdxdna_gem_obj *cmd_abo,
                              size_t *size, u32 *msg_op)
  {
-       struct amdxdna_dev *xdna = cmd_abo->client->xdna;
+       struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
        int ret;
        u32 op;

@@ -874,7 +874,7 @@ static int

  aie2_cmdlist_fill_slot(void *slot, struct amdxdna_gem_obj *cmd_abo,
                       size_t *size, u32 *cmd_op)
  {
-       struct amdxdna_dev *xdna = cmd_abo->client->xdna;
+       struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(cmd_abo)->dev);
        int ret;
        u32 op;

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

index 1275f91ca705..4628a2787265 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -198,6 +198,7 @@ amdxdna_gem_destroy_obj(struct amdxdna_gem_obj *abo)
   */
  void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)
  {
+       struct amdxdna_dev *xdna = to_xdna_dev(to_gobj(abo)->dev);
        struct iosys_map map = IOSYS_MAP_INIT_VADDR(NULL);
        int ret;

@@ -210,7 +211,7 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo)

        if (!abo->mem.kva) {
                ret = drm_gem_vmap(to_gobj(abo), &map);
                if (ret)
-                       XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", 
ret);
+                       XDNA_ERR(xdna, "Vmap bo failed, ret %d", ret);
                else
                        abo->mem.kva = map.vaddr;
        }
@@ -354,7 +355,13 @@ static int amdxdna_hmm_register(struct amdxdna_gem_obj 
*abo,
        unsigned long nr_pages;
        int ret;

- if (!amdxdna_pasid_on(abo->client)) {

+       /*
+        * When PASID is off, amdxdna_gem_obj_open() called amdxdna_dma_map_bo()
+        * and mem.dma_addr is valid; use the DMA address directly and skip HMM.
+        * Avoid dereferencing abo->client which may be NULL (cleared in 
close())
+        * while internal kernel references are still held.
+        */
+       if (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) {
                /* Need to set uva for heap uva validation */
                abo->mem.uva = addr;
                return 0;
diff --git a/drivers/accel/amdxdna/amdxdna_gem.h 
b/drivers/accel/amdxdna/amdxdna_gem.h
index a35d2f15d32c..1e90e32bf3cd 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.h
+++ b/drivers/accel/amdxdna/amdxdna_gem.h
@@ -88,12 +88,19 @@ u64 amdxdna_gem_dev_addr(struct amdxdna_gem_obj *abo);

static inline u64 amdxdna_dev_bo_offset(struct amdxdna_gem_obj *abo)

  {
-       return amdxdna_gem_dev_addr(abo) - 
abo->client->xdna->dev_info->dev_mem_base;
+       return amdxdna_gem_dev_addr(abo) - 
to_xdna_dev(to_gobj(abo)->dev)->dev_info->dev_mem_base;
  }

static inline u64 amdxdna_obj_dma_addr(struct amdxdna_gem_obj *abo)

  {
-       return amdxdna_pasid_on(abo->client) ? amdxdna_gem_uva(abo) : 
abo->mem.dma_addr;
+       /*
+        * amdxdna_gem_obj_open() calls amdxdna_dma_map_bo() only when PASID is
+        * off, leaving mem.dma_addr at AMDXDNA_INVALID_ADDR when PASID is on.
+        * Avoid dereferencing abo->client, which is cleared to NULL by
+        * amdxdna_gem_obj_close() while internal kernel references remain.
+        */
+       return (abo->mem.dma_addr != AMDXDNA_INVALID_ADDR) ?
+               abo->mem.dma_addr : amdxdna_gem_uva(abo);
  }

void amdxdna_umap_put(struct amdxdna_umap *mapp);