accel/amdxdna: Fix iommu_map_sgtable() return value handling

[PATCH V1] accel/amdxdna: Fix iommu_map_sgtable() return value handling

Posted by Lizhi Hou 2 months ago

iommu_map_sgtable() returns negative error codes on failure, but the
result is stored in an unsigned variable. This prevents proper error
detection.

Change the variable type to ssize_t so negative error values can be
handled correctly.

Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/all/adk7kOUBwIyYnX1M@stanley.mountain/
Signed-off-by: Wendy Liang <wendy.liang@amd.com>
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_iommu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
index 4626434d4180..2676cfcfabee 100644
--- a/drivers/accel/amdxdna/amdxdna_iommu.c
+++ b/drivers/accel/amdxdna/amdxdna_iommu.c
@@ -40,7 +40,7 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, struct amdxdna_gem_obj *abo)
 	struct sg_table *sgt;
 	dma_addr_t dma_addr;
 	struct iova *iova;
-	size_t size;
+	ssize_t size;
 
 	if (abo->type != AMDXDNA_BO_DEV_HEAP && abo->type != AMDXDNA_BO_SHMEM)
 		return 0;
@@ -65,7 +65,14 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, struct amdxdna_gem_obj *abo)
 
 	size = iommu_map_sgtable(xdna->domain, dma_addr, sgt,
 				 IOMMU_READ | IOMMU_WRITE);
+	if (size < 0) {
+		XDNA_ERR(xdna, "iommu_map_sgtable failed: %zd", size);
+		__free_iova(&xdna->iovad, iova);
+		return size;
+	}
+
 	if (size < abo->mem.size) {
+		iommu_unmap(xdna->domain, dma_addr, size);
 		__free_iova(&xdna->iovad, iova);
 		return -ENXIO;
 	}
-- 
2.34.1

Re: [PATCH V1] accel/amdxdna: Fix iommu_map_sgtable() return value handling

Posted by Mario Limonciello 2 months ago


On 4/13/26 13:02, Lizhi Hou wrote:
> iommu_map_sgtable() returns negative error codes on failure, but the
> result is stored in an unsigned variable. This prevents proper error
> detection.
> 
> Change the variable type to ssize_t so negative error values can be
> handled correctly.
> 
> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/all/adk7kOUBwIyYnX1M@stanley.mountain/
> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/amdxdna_iommu.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
> index 4626434d4180..2676cfcfabee 100644
> --- a/drivers/accel/amdxdna/amdxdna_iommu.c
> +++ b/drivers/accel/amdxdna/amdxdna_iommu.c
> @@ -40,7 +40,7 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, struct amdxdna_gem_obj *abo)
>   	struct sg_table *sgt;
>   	dma_addr_t dma_addr;
>   	struct iova *iova;
> -	size_t size;
> +	ssize_t size;
>   
>   	if (abo->type != AMDXDNA_BO_DEV_HEAP && abo->type != AMDXDNA_BO_SHMEM)
>   		return 0;
> @@ -65,7 +65,14 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, struct amdxdna_gem_obj *abo)
>   
>   	size = iommu_map_sgtable(xdna->domain, dma_addr, sgt,
>   				 IOMMU_READ | IOMMU_WRITE);
> +	if (size < 0) {
> +		XDNA_ERR(xdna, "iommu_map_sgtable failed: %zd", size);
> +		__free_iova(&xdna->iovad, iova);
> +		return size;
> +	}
> +
>   	if (size < abo->mem.size) {
> +		iommu_unmap(xdna->domain, dma_addr, size);
>   		__free_iova(&xdna->iovad, iova);
>   		return -ENXIO;
>   	}

Re: [PATCH V1] accel/amdxdna: Fix iommu_map_sgtable() return value handling

Posted by Lizhi Hou 2 months ago

Applied to drm-misc-next

On 4/13/26 11:27, Mario Limonciello wrote:
>
>
> On 4/13/26 13:02, Lizhi Hou wrote:
>> iommu_map_sgtable() returns negative error codes on failure, but the
>> result is stored in an unsigned variable. This prevents proper error
>> detection.
>>
>> Change the variable type to ssize_t so negative error values can be
>> handled correctly.
>>
>> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via 
>> module parameter")
>> Reported-by: Dan Carpenter <error27@gmail.com>
>> Closes: https://lore.kernel.org/all/adk7kOUBwIyYnX1M@stanley.mountain/
>> Signed-off-by: Wendy Liang <wendy.liang@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   drivers/accel/amdxdna/amdxdna_iommu.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c 
>> b/drivers/accel/amdxdna/amdxdna_iommu.c
>> index 4626434d4180..2676cfcfabee 100644
>> --- a/drivers/accel/amdxdna/amdxdna_iommu.c
>> +++ b/drivers/accel/amdxdna/amdxdna_iommu.c
>> @@ -40,7 +40,7 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, 
>> struct amdxdna_gem_obj *abo)
>>       struct sg_table *sgt;
>>       dma_addr_t dma_addr;
>>       struct iova *iova;
>> -    size_t size;
>> +    ssize_t size;
>>         if (abo->type != AMDXDNA_BO_DEV_HEAP && abo->type != 
>> AMDXDNA_BO_SHMEM)
>>           return 0;
>> @@ -65,7 +65,14 @@ int amdxdna_iommu_map_bo(struct amdxdna_dev *xdna, 
>> struct amdxdna_gem_obj *abo)
>>         size = iommu_map_sgtable(xdna->domain, dma_addr, sgt,
>>                    IOMMU_READ | IOMMU_WRITE);
>> +    if (size < 0) {
>> +        XDNA_ERR(xdna, "iommu_map_sgtable failed: %zd", size);
>> +        __free_iova(&xdna->iovad, iova);
>> +        return size;
>> +    }
>> +
>>       if (size < abo->mem.size) {
>> +        iommu_unmap(xdna->domain, dma_addr, size);
>>           __free_iova(&xdna->iovad, iova);
>>           return -ENXIO;
>>       }
>