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

[Max Zhen <max.zhen@amd.com>]

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.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@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);