[PATCH V1 1/2] accel/amdxdna: Disable BO import via flink

[PATCH V1 1/2] accel/amdxdna: Disable BO import via flink

[Lizhi Hou]

Accessing abo->client in amdxdna_gem_del_bo_usage() may result in a
use-after-free when the BO is imported via flink.

Disable flink import by verifing that filp->driver_priv matches the client
stored in abo->client before accessing the client.

Fixes: 3cc5d7a59519 ("accel/amdxdna: Add carveout memory support for non-IOMMU systems")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 476649685e5a..1353393194e2 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -697,6 +697,8 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
 	int ret;
 
 	guard(mutex)(&abo->lock);
+	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
+		return -EPERM;
 	abo->open_ref++;
 	if (abo->open_ref > 1)
 		return 0;
-- 
2.34.1

[PATCH V1 2/2] accel/amdxdna: Fix potential deadlock in BO open and close callbacks

[Lizhi Hou]

In amdxdna_gem_obj_open(), abo->lock is held when calling
amdxdna_gem_add_bo_usage(), which then acquires client->mm_lock.

However, the heap update path may acquire these locks in the reverse
order, creating a potential deadlock.

Fix this by saving the client pointer locally before acquiring abo->lock,
and releasing abo->lock before calling amdxdna_gem_add_bo_usage().

Apply the same change to amdxdna_gem_obj_close().

Fixes: 1f513a3ec3a9 ("accel/amdxdna: Add per-process BO memory usage query support")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_gem.c | 34 +++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
index 1353393194e2..0d165b66c1fc 100644
--- a/drivers/accel/amdxdna/amdxdna_gem.c
+++ b/drivers/accel/amdxdna/amdxdna_gem.c
@@ -641,10 +641,8 @@ amdxdna_gem_skip_bo_usage(struct amdxdna_gem_obj *abo)
 }
 
 static void
-amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
+amdxdna_gem_add_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
 {
-	struct amdxdna_client *client = abo->client;
-
 	if (amdxdna_gem_skip_bo_usage(abo))
 		return;
 
@@ -656,10 +654,8 @@ amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
 }
 
 static void
-amdxdna_gem_del_bo_usage(struct amdxdna_gem_obj *abo)
+amdxdna_gem_del_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
 {
-	struct amdxdna_client *client = abo->client;
-
 	if (amdxdna_gem_skip_bo_usage(abo))
 		return;
 
@@ -694,14 +690,20 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
 {
 	struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
 	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
+	struct amdxdna_client *client;
 	int ret;
 
-	guard(mutex)(&abo->lock);
-	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
+	mutex_lock(&abo->lock);
+	if (abo->open_ref > 0 && filp->driver_priv != abo->client) {
+		mutex_unlock(&abo->lock);
 		return -EPERM;
+	}
+
 	abo->open_ref++;
-	if (abo->open_ref > 1)
+	if (abo->open_ref > 1) {
+		mutex_unlock(&abo->lock);
 		return 0;
+	}
 
 	/* Attached to the client when first opened by it. */
 	abo->client = filp->driver_priv;
@@ -712,26 +714,34 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
 		if (ret) {
 			abo->open_ref--;
 			abo->client = NULL;
+			mutex_unlock(&abo->lock);
 			return ret;
 		}
 	}
+	client = abo->client;
+	mutex_unlock(&abo->lock);
 
-	amdxdna_gem_add_bo_usage(abo);
+	amdxdna_gem_add_bo_usage(client, abo);
 	return 0;
 }
 
 static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp)
 {
 	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
+	struct amdxdna_client *client = NULL;
 
-	guard(mutex)(&abo->lock);
+	mutex_lock(&abo->lock);
 	abo->open_ref--;
 
 	if (abo->open_ref == 0) {
-		amdxdna_gem_del_bo_usage(abo);
 		/* Detach from the client when last closed by it. */
+		client = abo->client;
 		abo->client = NULL;
 	}
+	mutex_unlock(&abo->lock);
+
+	if (client)
+		amdxdna_gem_del_bo_usage(client, abo);
 }
 
 static int amdxdna_gem_obj_vmap(struct drm_gem_object *obj, struct iosys_map *map)
-- 
2.34.1

Re: [PATCH V1 2/2] accel/amdxdna: Fix potential deadlock in BO open and close callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Repeated open and close of an exported BO leaks IOVA and IOMMU mappings.
--

commit 51f6a251a76d33602344897b8464cf1529e24b2e
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Fix potential deadlock in BO open and close callbacks

This commit fixes a potential deadlock during BO open and close operations by
acquiring client and BO locks in a consistent order. It locally saves the
client pointer before acquiring the BO lock to prevent reverse order locking.

> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index a12269f1543ee..05332894849cd 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
[ ... ]
> @@ -663,14 +659,20 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>  {
>  	struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
>  	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +	struct amdxdna_client *client;
>  	int ret;
>  
> -	guard(mutex)(&abo->lock);
> -	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
> +	mutex_lock(&abo->lock);
> +	if (abo->open_ref > 0 && filp->driver_priv != abo->client) {
> +		mutex_unlock(&abo->lock);
>  		return -EPERM;
> +	}
> +
>  	abo->open_ref++;
> -	if (abo->open_ref > 1)
> +	if (abo->open_ref > 1) {
> +		mutex_unlock(&abo->lock);
>  		return 0;
> +	}
>  
>  	/* Attached to the client when first opened by it. */
>  	abo->client = filp->driver_priv;
> @@ -681,26 +683,34 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>  		if (ret) {

[Severity: High]
This is a pre-existing issue, but does this repeated open and close of an
exported BO leak IOVA and IOMMU mappings?

A userspace process can create a BO, export it to a DMA-BUF file descriptor,
and then repeatedly close the GEM handle and re-import the same FD to a new
handle.

When the handle is closed, amdxdna_gem_obj_close() drops abo->open_ref to 0
but does not unmap the IOVA. Re-importing the FD calls amdxdna_gem_obj_open(),
which sees open_ref transition from 0 to 1 and unconditionally calls
amdxdna_dma_map_bo().

This allocates a new IOVA in amdxdna_iommu.c:amdxdna_dma_map_bo() without
checking if the BO already holds a valid dma_addr:

	iova = amdxdna_iommu_alloc_iova(xdna, abo->mem.size, &dma_addr,
					(abo->type == AMDXDNA_BO_DEV_HEAP));

Can this overwrite abo->mem.dma_addr without freeing the old one, leading to
a denial of service when IOVA space is exhausted?

>  			abo->open_ref--;
>  			abo->client = NULL;
> +			mutex_unlock(&abo->lock);
>  			return ret;
>  		}
>  	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902180955.3999242-1-lizhi.hou@amd.com?part=2

Re: [PATCH V1 2/2] accel/amdxdna: Fix potential deadlock in BO open and close callbacks

[Max Zhen]

On 9/2/2026 Wed 11:09, Lizhi Hou wrote:
> In amdxdna_gem_obj_open(), abo->lock is held when calling
> amdxdna_gem_add_bo_usage(), which then acquires client->mm_lock.
> 
> However, the heap update path may acquire these locks in the reverse
> order, creating a potential deadlock.
> 
> Fix this by saving the client pointer locally before acquiring abo->lock,
> and releasing abo->lock before calling amdxdna_gem_add_bo_usage().
> 
> Apply the same change to amdxdna_gem_obj_close().
> 
> Fixes: 1f513a3ec3a9 ("accel/amdxdna: Add per-process BO memory usage query support")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 34 +++++++++++++++++++----------
>   1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 1353393194e2..0d165b66c1fc 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -641,10 +641,8 @@ amdxdna_gem_skip_bo_usage(struct amdxdna_gem_obj *abo)
>   }
>   
>   static void
> -amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
> +amdxdna_gem_add_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
>   {
> -	struct amdxdna_client *client = abo->client;
> -
>   	if (amdxdna_gem_skip_bo_usage(abo))
>   		return;
>   
> @@ -656,10 +654,8 @@ amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
>   }
>   
>   static void
> -amdxdna_gem_del_bo_usage(struct amdxdna_gem_obj *abo)
> +amdxdna_gem_del_bo_usage(struct amdxdna_client *client, struct amdxdna_gem_obj *abo)
>   {
> -	struct amdxdna_client *client = abo->client;
> -
>   	if (amdxdna_gem_skip_bo_usage(abo))
>   		return;
>   
> @@ -694,14 +690,20 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>   {
>   	struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +	struct amdxdna_client *client;
>   	int ret;
>   
> -	guard(mutex)(&abo->lock);
> -	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
> +	mutex_lock(&abo->lock);
> +	if (abo->open_ref > 0 && filp->driver_priv != abo->client) {
> +		mutex_unlock(&abo->lock);
>   		return -EPERM;
> +	}
> +
>   	abo->open_ref++;
> -	if (abo->open_ref > 1)
> +	if (abo->open_ref > 1) {
> +		mutex_unlock(&abo->lock);
>   		return 0;
> +	}
>   
>   	/* Attached to the client when first opened by it. */
>   	abo->client = filp->driver_priv;
> @@ -712,26 +714,34 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>   		if (ret) {
>   			abo->open_ref--;
>   			abo->client = NULL;
> +			mutex_unlock(&abo->lock);
>   			return ret;
>   		}
>   	}
> +	client = abo->client;
> +	mutex_unlock(&abo->lock);
>   
> -	amdxdna_gem_add_bo_usage(abo);
> +	amdxdna_gem_add_bo_usage(client, abo);
>   	return 0;
>   }
>   
>   static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, struct drm_file *filp)
>   {
>   	struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
> +	struct amdxdna_client *client = NULL;
>   
> -	guard(mutex)(&abo->lock);
> +	mutex_lock(&abo->lock);
>   	abo->open_ref--;
>   
>   	if (abo->open_ref == 0) {
> -		amdxdna_gem_del_bo_usage(abo);
>   		/* Detach from the client when last closed by it. */
> +		client = abo->client;
>   		abo->client = NULL;
>   	}
> +	mutex_unlock(&abo->lock);
> +
> +	if (client)
> +		amdxdna_gem_del_bo_usage(client, abo);
>   }
>   
>   static int amdxdna_gem_obj_vmap(struct drm_gem_object *obj, struct iosys_map *map)

Re: [PATCH V1 2/2] accel/amdxdna: Fix potential deadlock in BO open and close callbacks

[Lizhi Hou]

Applied to drm-misc-next

On 9/2/26 11:51, Max Zhen wrote:
>
>
> On 9/2/2026 Wed 11:09, Lizhi Hou wrote:
>> In amdxdna_gem_obj_open(), abo->lock is held when calling
>> amdxdna_gem_add_bo_usage(), which then acquires client->mm_lock.
>>
>> However, the heap update path may acquire these locks in the reverse
>> order, creating a potential deadlock.
>>
>> Fix this by saving the client pointer locally before acquiring 
>> abo->lock,
>> and releasing abo->lock before calling amdxdna_gem_add_bo_usage().
>>
>> Apply the same change to amdxdna_gem_obj_close().
>>
>> Fixes: 1f513a3ec3a9 ("accel/amdxdna: Add per-process BO memory usage 
>> query support")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c | 34 +++++++++++++++++++----------
>>   1 file changed, 22 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 1353393194e2..0d165b66c1fc 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -641,10 +641,8 @@ amdxdna_gem_skip_bo_usage(struct amdxdna_gem_obj 
>> *abo)
>>   }
>>     static void
>> -amdxdna_gem_add_bo_usage(struct amdxdna_gem_obj *abo)
>> +amdxdna_gem_add_bo_usage(struct amdxdna_client *client, struct 
>> amdxdna_gem_obj *abo)
>>   {
>> -    struct amdxdna_client *client = abo->client;
>> -
>>       if (amdxdna_gem_skip_bo_usage(abo))
>>           return;
>>   @@ -656,10 +654,8 @@ amdxdna_gem_add_bo_usage(struct 
>> amdxdna_gem_obj *abo)
>>   }
>>     static void
>> -amdxdna_gem_del_bo_usage(struct amdxdna_gem_obj *abo)
>> +amdxdna_gem_del_bo_usage(struct amdxdna_client *client, struct 
>> amdxdna_gem_obj *abo)
>>   {
>> -    struct amdxdna_client *client = abo->client;
>> -
>>       if (amdxdna_gem_skip_bo_usage(abo))
>>           return;
>>   @@ -694,14 +690,20 @@ static int amdxdna_gem_obj_open(struct 
>> drm_gem_object *gobj, struct drm_file *fi
>>   {
>>       struct amdxdna_dev *xdna = to_xdna_dev(gobj->dev);
>>       struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
>> +    struct amdxdna_client *client;
>>       int ret;
>>   -    guard(mutex)(&abo->lock);
>> -    if (abo->open_ref > 0 && filp->driver_priv != abo->client)
>> +    mutex_lock(&abo->lock);
>> +    if (abo->open_ref > 0 && filp->driver_priv != abo->client) {
>> +        mutex_unlock(&abo->lock);
>>           return -EPERM;
>> +    }
>> +
>>       abo->open_ref++;
>> -    if (abo->open_ref > 1)
>> +    if (abo->open_ref > 1) {
>> +        mutex_unlock(&abo->lock);
>>           return 0;
>> +    }
>>         /* Attached to the client when first opened by it. */
>>       abo->client = filp->driver_priv;
>> @@ -712,26 +714,34 @@ static int amdxdna_gem_obj_open(struct 
>> drm_gem_object *gobj, struct drm_file *fi
>>           if (ret) {
>>               abo->open_ref--;
>>               abo->client = NULL;
>> +            mutex_unlock(&abo->lock);
>>               return ret;
>>           }
>>       }
>> +    client = abo->client;
>> +    mutex_unlock(&abo->lock);
>>   -    amdxdna_gem_add_bo_usage(abo);
>> +    amdxdna_gem_add_bo_usage(client, abo);
>>       return 0;
>>   }
>>     static void amdxdna_gem_obj_close(struct drm_gem_object *gobj, 
>> struct drm_file *filp)
>>   {
>>       struct amdxdna_gem_obj *abo = to_xdna_obj(gobj);
>> +    struct amdxdna_client *client = NULL;
>>   -    guard(mutex)(&abo->lock);
>> +    mutex_lock(&abo->lock);
>>       abo->open_ref--;
>>         if (abo->open_ref == 0) {
>> -        amdxdna_gem_del_bo_usage(abo);
>>           /* Detach from the client when last closed by it. */
>> +        client = abo->client;
>>           abo->client = NULL;
>>       }
>> +    mutex_unlock(&abo->lock);
>> +
>> +    if (client)
>> +        amdxdna_gem_del_bo_usage(client, abo);
>>   }
>>     static int amdxdna_gem_obj_vmap(struct drm_gem_object *obj, 
>> struct iosys_map *map)
>

Re: [PATCH V1 1/2] accel/amdxdna: Disable BO import via flink

[Max Zhen]

On 9/2/2026 Wed 11:09, Lizhi Hou wrote:
> Accessing abo->client in amdxdna_gem_del_bo_usage() may result in a
> use-after-free when the BO is imported via flink.
> 
> Disable flink import by verifing that filp->driver_priv matches the client
> stored in abo->client before accessing the client.
> 
> Fixes: 3cc5d7a59519 ("accel/amdxdna: Add carveout memory support for non-IOMMU systems")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Max Zhen <max.zhen@amd.com>
> ---
>   drivers/accel/amdxdna/amdxdna_gem.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c
> index 476649685e5a..1353393194e2 100644
> --- a/drivers/accel/amdxdna/amdxdna_gem.c
> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
> @@ -697,6 +697,8 @@ static int amdxdna_gem_obj_open(struct drm_gem_object *gobj, struct drm_file *fi
>   	int ret;
>   
>   	guard(mutex)(&abo->lock);
> +	if (abo->open_ref > 0 && filp->driver_priv != abo->client)
> +		return -EPERM;
>   	abo->open_ref++;
>   	if (abo->open_ref > 1)
>   		return 0;

Re: [PATCH V1 1/2] accel/amdxdna: Disable BO import via flink

[Lizhi Hou]

Applied to drm-misc-next

On 9/2/26 11:48, Max Zhen wrote:
>
>
> On 9/2/2026 Wed 11:09, Lizhi Hou wrote:
>> Accessing abo->client in amdxdna_gem_del_bo_usage() may result in a
>> use-after-free when the BO is imported via flink.
>>
>> Disable flink import by verifing that filp->driver_priv matches the 
>> client
>> stored in abo->client before accessing the client.
>>
>> Fixes: 3cc5d7a59519 ("accel/amdxdna: Add carveout memory support for 
>> non-IOMMU systems")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Max Zhen <max.zhen@amd.com>
>> ---
>>   drivers/accel/amdxdna/amdxdna_gem.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_gem.c 
>> b/drivers/accel/amdxdna/amdxdna_gem.c
>> index 476649685e5a..1353393194e2 100644
>> --- a/drivers/accel/amdxdna/amdxdna_gem.c
>> +++ b/drivers/accel/amdxdna/amdxdna_gem.c
>> @@ -697,6 +697,8 @@ static int amdxdna_gem_obj_open(struct 
>> drm_gem_object *gobj, struct drm_file *fi
>>       int ret;
>>         guard(mutex)(&abo->lock);
>> +    if (abo->open_ref > 0 && filp->driver_priv != abo->client)
>> +        return -EPERM;
>>       abo->open_ref++;
>>       if (abo->open_ref > 1)
>>           return 0;
>