[PATCH 1/2] accel/amdxdna: fix memory leak in amdxdna_iommu_alloc() when iommu_map() fails

[PATCH 1/2] accel/amdxdna: fix memory leak in amdxdna_iommu_alloc() when iommu_map() fails

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

When iommu_map() fails in amdxdna_iommu_alloc(), the code jumps to
the 'free_iova' label which only frees the IOVA allocation. The
physical pages allocated by __get_free_pages() are not freed, causing
a memory leak.

Add a 'free_pages' intermediate label to properly release the pages
before freeing the IOVA, matching the cleanup order in
amdxdna_iommu_free().

Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 drivers/accel/amdxdna/amdxdna_iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
index 4626434d4180..af67e059d599 100644
--- a/drivers/accel/amdxdna/amdxdna_iommu.c
+++ b/drivers/accel/amdxdna/amdxdna_iommu.c
@@ -110,10 +110,12 @@ void *amdxdna_iommu_alloc(struct amdxdna_dev *xdna, size_t size, dma_addr_t *dma
 			iova_align(&xdna->iovad, size),
 			IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
 	if (ret)
-		goto free_iova;
+		goto free_pages;
 
 	return cpu_addr;
 
+free_pages:
+	free_pages((unsigned long)cpu_addr, get_order(size));
 free_iova:
 	__free_iova(&xdna->iovad, iova);
 	return ERR_PTR(ret);
-- 
2.54.0

[PATCH 2/2] accel/amdxdna: fix error code being silently swallowed in hwctx status queries

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

Both aie2_get_hwctx_status() and aie2_query_ctx_status_array() iterate
over hardware contexts calling amdxdna_hwctx_walk() with a callback
that can fail with -ENOMEM (allocation failure) or -EFAULT
(copy_to_user failure). When the walk fails, the loop correctly breaks
but the functions always return 0 to userspace, swallowing the error.

This means userspace receives no indication that the data may be
incomplete or that the operation failed. Save the error code and
return it to properly propagate failures to userspace.

Fixes: 81233d5419cf ("accel/amdxdna: Fix uninitialized return value")
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 drivers/accel/amdxdna/aie2_pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 4500b9ccb02e..0ff87fafd049 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -908,7 +908,7 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
 	struct amdxdna_drm_get_array array_args;
 	struct amdxdna_dev *xdna = client->xdna;
 	struct amdxdna_client *tmp_client;
-	int ret;
+	int ret = 0;
 
 	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
 
@@ -923,7 +923,7 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
 	}
 
 	args->buffer_size -= (u32)(array_args.buffer - args->buffer);
-	return 0;
+	return ret;
 }
 
 static int aie2_query_resource_info(struct amdxdna_client *client,
@@ -1105,7 +1105,7 @@ static int aie2_query_ctx_status_array(struct amdxdna_client *client,
 	struct amdxdna_drm_get_array array_args;
 	struct amdxdna_dev *xdna = client->xdna;
 	struct amdxdna_client *tmp_client;
-	int ret;
+	int ret = 0;
 
 	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
 
@@ -1131,7 +1131,7 @@ static int aie2_query_ctx_status_array(struct amdxdna_client *client,
 	args->num_element = (u32)((array_args.buffer - args->buffer) /
 				  args->element_size);
 
-	return 0;
+	return ret;
 }
 
 static int aie2_get_array(struct amdxdna_client *client,
-- 
2.54.0

Re: [PATCH 1/2] accel/amdxdna: fix memory leak in amdxdna_iommu_alloc() when iommu_map() fails

[Lizhi Hou]

Hi Jackie,

Thanks for providing the patch. This is a known bug and the fix is on 
the way.

https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/432fafdc9a3122a7bee5b2bfd23dcf2dc262a3d7

Lizhi

On 6/2/26 18:35, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> When iommu_map() fails in amdxdna_iommu_alloc(), the code jumps to
> the 'free_iova' label which only frees the IOVA allocation. The
> physical pages allocated by __get_free_pages() are not freed, causing
> a memory leak.
>
> Add a 'free_pages' intermediate label to properly release the pages
> before freeing the IOVA, matching the cleanup order in
> amdxdna_iommu_free().
>
> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>   drivers/accel/amdxdna/amdxdna_iommu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
> index 4626434d4180..af67e059d599 100644
> --- a/drivers/accel/amdxdna/amdxdna_iommu.c
> +++ b/drivers/accel/amdxdna/amdxdna_iommu.c
> @@ -110,10 +110,12 @@ void *amdxdna_iommu_alloc(struct amdxdna_dev *xdna, size_t size, dma_addr_t *dma
>   			iova_align(&xdna->iovad, size),
>   			IOMMU_READ | IOMMU_WRITE, GFP_KERNEL);
>   	if (ret)
> -		goto free_iova;
> +		goto free_pages;
>   
>   	return cpu_addr;
>   
> +free_pages:
> +	free_pages((unsigned long)cpu_addr, get_order(size));
>   free_iova:
>   	__free_iova(&xdna->iovad, iova);
>   	return ERR_PTR(ret);

Re: [PATCH 2/2] accel/amdxdna: fix error code being silently swallowed in hwctx status queries

[Lizhi Hou]

On 6/2/26 18:35, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
>
> Both aie2_get_hwctx_status() and aie2_query_ctx_status_array() iterate
> over hardware contexts calling amdxdna_hwctx_walk() with a callback
> that can fail with -ENOMEM (allocation failure) or -EFAULT
> (copy_to_user failure). When the walk fails, the loop correctly breaks
> but the functions always return 0 to userspace, swallowing the error.
>
> This means userspace receives no indication that the data may be
> incomplete or that the operation failed. Save the error code and
> return it to properly propagate failures to userspace.
>
> Fixes: 81233d5419cf ("accel/amdxdna: Fix uninitialized return value")

The retrieving information can be partially done. The amount of valid 
results is indicated by the returned buffer_size, element_size, and 
num_element fields. Thus, 0 is returned. So user space will not consider 
this is a broken error and still get the partial results.

Lizhi

> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>   drivers/accel/amdxdna/aie2_pci.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
> index 4500b9ccb02e..0ff87fafd049 100644
> --- a/drivers/accel/amdxdna/aie2_pci.c
> +++ b/drivers/accel/amdxdna/aie2_pci.c
> @@ -908,7 +908,7 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
>   	struct amdxdna_drm_get_array array_args;
>   	struct amdxdna_dev *xdna = client->xdna;
>   	struct amdxdna_client *tmp_client;
> -	int ret;
> +	int ret = 0;
>   
>   	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
>   
> @@ -923,7 +923,7 @@ static int aie2_get_hwctx_status(struct amdxdna_client *client,
>   	}
>   
>   	args->buffer_size -= (u32)(array_args.buffer - args->buffer);
> -	return 0;
> +	return ret;
>   }
>   
>   static int aie2_query_resource_info(struct amdxdna_client *client,
> @@ -1105,7 +1105,7 @@ static int aie2_query_ctx_status_array(struct amdxdna_client *client,
>   	struct amdxdna_drm_get_array array_args;
>   	struct amdxdna_dev *xdna = client->xdna;
>   	struct amdxdna_client *tmp_client;
> -	int ret;
> +	int ret = 0;
>   
>   	drm_WARN_ON(&xdna->ddev, !mutex_is_locked(&xdna->dev_lock));
>   
> @@ -1131,7 +1131,7 @@ static int aie2_query_ctx_status_array(struct amdxdna_client *client,
>   	args->num_element = (u32)((array_args.buffer - args->buffer) /
>   				  args->element_size);
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static int aie2_get_array(struct amdxdna_client *client,