mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal
@ 2026-06-11  5:51 Lizhi Hou
  2026-06-11  5:51 ` [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq " Lizhi Hou
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Lizhi Hou @ 2026-06-11  5:51 UTC (permalink / raw)
  To: ogabbay, quic_jhugo, dri-devel, mario.limonciello, karol.wachowski
  Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan

In amdxdna_remove(), all amdxdna_client structures are freed after
calling drm_dev_unplug(). However, drm_dev_unplug() does not force
existing file descriptors to be closed, so amdxdna_drm_close() may be
called after amdxdna_remove() has completed.

As a result, accessing client->pid for debug output in
amdxdna_drm_close() can lead to a use-after-free, since the access is
not protected by drm_dev_enter().

Fix this by decoupling hardware teardown from client cleanup.
amdxdna_remove() only performs hardware-related cleanup, while
per-client resources are released from amdxdna_drm_close() when the
corresponding file is closed.

Fixes: be462c97b7df ("accel/amdxdna: Add hardware context")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_pci_drv.c | 23 ++++++++++-------------
 drivers/accel/amdxdna/amdxdna_pci_drv.h |  1 +
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index 65489bb3f2b0..470bf4fc744b 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -138,9 +138,11 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
 		    xdna->dev_info->dev_heap_max_size);
 	mutex_init(&client->mm_lock);
 
+	mutex_lock(&xdna->client_lock);
 	mutex_lock(&xdna->dev_lock);
 	list_add_tail(&client->node, &xdna->client_list);
 	mutex_unlock(&xdna->dev_lock);
+	mutex_unlock(&xdna->client_lock);
 
 	filp->driver_priv = client;
 	client->filp = filp;
@@ -174,18 +176,14 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp)
 {
 	struct amdxdna_client *client = filp->driver_priv;
 	struct amdxdna_dev *xdna = to_xdna_dev(ddev);
-	int idx;
 
 	XDNA_DBG(xdna, "closing pid %d", client->pid);
 
-	if (!drm_dev_enter(&xdna->ddev, &idx))
-		return;
-
+	mutex_lock(&xdna->client_lock);
 	mutex_lock(&xdna->dev_lock);
 	amdxdna_client_cleanup(client);
 	mutex_unlock(&xdna->dev_lock);
-
-	drm_dev_exit(idx);
+	mutex_unlock(&xdna->client_lock);
 }
 
 static int amdxdna_drm_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
@@ -372,6 +370,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return -ENODEV;
 
 	drmm_mutex_init(ddev, &xdna->dev_lock);
+	drmm_mutex_init(ddev, &xdna->client_lock);
 	init_rwsem(&xdna->notifier_lock);
 	INIT_LIST_HEAD(&xdna->client_list);
 	pci_set_drvdata(pdev, xdna);
@@ -442,18 +441,16 @@ static void amdxdna_remove(struct pci_dev *pdev)
 	drm_dev_unplug(&xdna->ddev);
 	amdxdna_sysfs_fini(xdna);
 
+	mutex_lock(&xdna->client_lock);
 	mutex_lock(&xdna->dev_lock);
-	client = list_first_entry_or_null(&xdna->client_list,
-					  struct amdxdna_client, node);
-	while (client) {
-		amdxdna_client_cleanup(client);
-
-		client = list_first_entry_or_null(&xdna->client_list,
-						  struct amdxdna_client, node);
+	list_for_each_entry(client, &xdna->client_list, node) {
+		amdxdna_hwctx_remove_all(client);
+		amdxdna_sva_fini(client);
 	}
 
 	xdna->dev_info->ops->fini(xdna);
 	mutex_unlock(&xdna->dev_lock);
+	mutex_unlock(&xdna->client_lock);
 
 	amdxdna_iommu_fini(xdna);
 }
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h
index 34271c14d359..a997d27a504d 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
@@ -120,6 +120,7 @@ struct amdxdna_dev {
 
 	struct mutex			dev_lock; /* per device lock */
 	struct list_head		client_list;
+	struct mutex			client_lock; /* client_list */
 	struct amdxdna_fw_ver		fw_ver;
 	struct rw_semaphore		notifier_lock; /* for mmu notifier*/
 	struct workqueue_struct		*notifier_wq;
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq lifetime race during device removal
  2026-06-11  5:51 [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
@ 2026-06-11  5:51 ` Lizhi Hou
  2026-06-12 18:26   ` Mario Limonciello
  2026-06-11  5:51 ` [PATCH V3 3/3] accel/amdxdna: Fix iommu domain " Lizhi Hou
  2026-06-12 18:25 ` [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client " Mario Limonciello
  2 siblings, 1 reply; 7+ messages in thread
From: Lizhi Hou @ 2026-06-11  5:51 UTC (permalink / raw)
  To: ogabbay, quic_jhugo, dri-devel, mario.limonciello, karol.wachowski
  Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan

amdxdna_remove() destroys notifier_wq. If amdxdna_gem_obj_free() is
called after device removal, it may attempt to flush notifier_wq,
resulting in a use-after-free.

Fix the race by allocating notifier_wq with
drmm_alloc_ordered_workqueue(), so its lifetime is managed by DRM and
remains valid until all managed resources are released.

Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_pci_drv.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index 470bf4fc744b..b943c06a451e 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -389,9 +389,9 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (ret)
 		return ret;
 
-	xdna->notifier_wq = alloc_ordered_workqueue("notifier_wq", WQ_MEM_RECLAIM);
-	if (!xdna->notifier_wq) {
-		ret = -ENOMEM;
+	xdna->notifier_wq = drmm_alloc_ordered_workqueue(ddev, "notifier_wq", WQ_MEM_RECLAIM);
+	if (IS_ERR(xdna->notifier_wq)) {
+		ret = PTR_ERR(xdna->notifier_wq);
 		goto iommu_fini;
 	}
 
@@ -400,7 +400,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mutex_unlock(&xdna->dev_lock);
 	if (ret) {
 		XDNA_ERR(xdna, "Hardware init failed, ret %d", ret);
-		goto destroy_notifier_wq;
+		goto iommu_fini;
 	}
 
 	ret = amdxdna_sysfs_init(xdna);
@@ -424,8 +424,6 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mutex_lock(&xdna->dev_lock);
 	xdna->dev_info->ops->fini(xdna);
 	mutex_unlock(&xdna->dev_lock);
-destroy_notifier_wq:
-	destroy_workqueue(xdna->notifier_wq);
 iommu_fini:
 	amdxdna_iommu_fini(xdna);
 	return ret;
@@ -436,8 +434,6 @@ static void amdxdna_remove(struct pci_dev *pdev)
 	struct amdxdna_dev *xdna = pci_get_drvdata(pdev);
 	struct amdxdna_client *client;
 
-	destroy_workqueue(xdna->notifier_wq);
-
 	drm_dev_unplug(&xdna->ddev);
 	amdxdna_sysfs_fini(xdna);
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH V3 3/3] accel/amdxdna: Fix iommu domain lifetime race during device removal
  2026-06-11  5:51 [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
  2026-06-11  5:51 ` [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq " Lizhi Hou
@ 2026-06-11  5:51 ` Lizhi Hou
  2026-06-12 18:26   ` Mario Limonciello
  2026-06-12 18:25 ` [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client " Mario Limonciello
  2 siblings, 1 reply; 7+ messages in thread
From: Lizhi Hou @ 2026-06-11  5:51 UTC (permalink / raw)
  To: ogabbay, quic_jhugo, dri-devel, mario.limonciello, karol.wachowski
  Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan

When force_iova mode is enabled, amdxdna_remove() frees xdna->domain. If
amdxdna_gem_obj_free() is called after device removal, it may attempt to
access xdna->domain, resulting in a use-after-free.

Fix the race by adding freeing xdna->domain as a managed release action,
so its lifetime is managed by DRM and remains valid until all managed
resources are released.

Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
 drivers/accel/amdxdna/amdxdna_iommu.c | 43 ++++++++++++++++++---------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
index eff00131d0f8..4f245b969eef 100644
--- a/drivers/accel/amdxdna/amdxdna_iommu.c
+++ b/drivers/accel/amdxdna/amdxdna_iommu.c
@@ -4,6 +4,7 @@
  */
 
 #include <drm/amdxdna_accel.h>
+#include <drm/drm_managed.h>
 #include <linux/iommu.h>
 #include <linux/iova.h>
 
@@ -153,10 +154,30 @@ void amdxdna_iommu_free(struct amdxdna_dev *xdna, size_t size,
 	free_pages((unsigned long)cpu_addr, get_order(size));
 }
 
+static void amdxdna_cleanup_force_iova(struct drm_device *dev, void *res)
+{
+	struct amdxdna_dev *xdna = to_xdna_dev(dev);
+
+	if (xdna->domain) {
+		iommu_detach_group(xdna->domain, xdna->group);
+		put_iova_domain(&xdna->iovad);
+		iova_cache_put();
+		iommu_domain_free(xdna->domain);
+	}
+
+	iommu_group_put(xdna->group);
+}
+
+void amdxdna_iommu_fini(struct amdxdna_dev *xdna)
+{
+	if (xdna->group && !xdna->domain)
+		iommu_group_put(xdna->group);
+}
+
 int amdxdna_iommu_init(struct amdxdna_dev *xdna)
 {
 	unsigned long order;
-	int ret;
+	int ret = 0;
 
 	xdna->group = iommu_group_get(xdna->ddev.dev);
 	if (!xdna->group || !force_iova)
@@ -182,8 +203,14 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna)
 	if (ret)
 		goto put_iova;
 
+	ret = drmm_add_action(&xdna->ddev, amdxdna_cleanup_force_iova, NULL);
+	if (ret)
+		goto detach_group;
+
 	return 0;
 
+detach_group:
+	iommu_detach_group(xdna->domain, xdna->group);
 put_iova:
 	put_iova_domain(&xdna->iovad);
 	iova_cache_put();
@@ -191,20 +218,8 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna)
 	iommu_domain_free(xdna->domain);
 put_group:
 	iommu_group_put(xdna->group);
+	xdna->group = NULL;
 	xdna->domain = NULL;
 
 	return ret;
 }
-
-void amdxdna_iommu_fini(struct amdxdna_dev *xdna)
-{
-	if (xdna->domain) {
-		iommu_detach_group(xdna->domain, xdna->group);
-		put_iova_domain(&xdna->iovad);
-		iova_cache_put();
-		iommu_domain_free(xdna->domain);
-	}
-
-	if (xdna->group)
-		iommu_group_put(xdna->group);
-}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal
  2026-06-11  5:51 [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
  2026-06-11  5:51 ` [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq " Lizhi Hou
  2026-06-11  5:51 ` [PATCH V3 3/3] accel/amdxdna: Fix iommu domain " Lizhi Hou
@ 2026-06-12 18:25 ` Mario Limonciello
  2026-06-29 18:00   ` Lizhi Hou
  2 siblings, 1 reply; 7+ messages in thread
From: Mario Limonciello @ 2026-06-12 18:25 UTC (permalink / raw)
  To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, karol.wachowski
  Cc: linux-kernel, max.zhen, sonal.santan



On 6/11/26 00:51, Lizhi Hou wrote:
> In amdxdna_remove(), all amdxdna_client structures are freed after
> calling drm_dev_unplug(). However, drm_dev_unplug() does not force
> existing file descriptors to be closed, so amdxdna_drm_close() may be
> called after amdxdna_remove() has completed.
> 
> As a result, accessing client->pid for debug output in
> amdxdna_drm_close() can lead to a use-after-free, since the access is
> not protected by drm_dev_enter().
> 
> Fix this by decoupling hardware teardown from client cleanup.
> amdxdna_remove() only performs hardware-related cleanup, while
> per-client resources are released from amdxdna_drm_close() when the
> corresponding file is closed.
> 
> Fixes: be462c97b7df ("accel/amdxdna: Add hardware context")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/amdxdna_pci_drv.c | 23 ++++++++++-------------
>   drivers/accel/amdxdna/amdxdna_pci_drv.h |  1 +
>   2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index 65489bb3f2b0..470bf4fc744b 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -138,9 +138,11 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
>   		    xdna->dev_info->dev_heap_max_size);
>   	mutex_init(&client->mm_lock);
>   
> +	mutex_lock(&xdna->client_lock);
>   	mutex_lock(&xdna->dev_lock);
>   	list_add_tail(&client->node, &xdna->client_list);
>   	mutex_unlock(&xdna->dev_lock);
> +	mutex_unlock(&xdna->client_lock);
>   
>   	filp->driver_priv = client;
>   	client->filp = filp;
> @@ -174,18 +176,14 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp)
>   {
>   	struct amdxdna_client *client = filp->driver_priv;
>   	struct amdxdna_dev *xdna = to_xdna_dev(ddev);
> -	int idx;
>   
>   	XDNA_DBG(xdna, "closing pid %d", client->pid);
>   
> -	if (!drm_dev_enter(&xdna->ddev, &idx))
> -		return;
> -
> +	mutex_lock(&xdna->client_lock);
>   	mutex_lock(&xdna->dev_lock);
>   	amdxdna_client_cleanup(client);
>   	mutex_unlock(&xdna->dev_lock);
> -
> -	drm_dev_exit(idx);
> +	mutex_unlock(&xdna->client_lock);
>   }
>   
>   static int amdxdna_drm_get_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> @@ -372,6 +370,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		return -ENODEV;
>   
>   	drmm_mutex_init(ddev, &xdna->dev_lock);
> +	drmm_mutex_init(ddev, &xdna->client_lock);
>   	init_rwsem(&xdna->notifier_lock);
>   	INIT_LIST_HEAD(&xdna->client_list);
>   	pci_set_drvdata(pdev, xdna);
> @@ -442,18 +441,16 @@ static void amdxdna_remove(struct pci_dev *pdev)
>   	drm_dev_unplug(&xdna->ddev);
>   	amdxdna_sysfs_fini(xdna);
>   
> +	mutex_lock(&xdna->client_lock);
>   	mutex_lock(&xdna->dev_lock);
> -	client = list_first_entry_or_null(&xdna->client_list,
> -					  struct amdxdna_client, node);
> -	while (client) {
> -		amdxdna_client_cleanup(client);
> -
> -		client = list_first_entry_or_null(&xdna->client_list,
> -						  struct amdxdna_client, node);
> +	list_for_each_entry(client, &xdna->client_list, node) {
> +		amdxdna_hwctx_remove_all(client);
> +		amdxdna_sva_fini(client);
>   	}
>   
>   	xdna->dev_info->ops->fini(xdna);
>   	mutex_unlock(&xdna->dev_lock);
> +	mutex_unlock(&xdna->client_lock);
>   
>   	amdxdna_iommu_fini(xdna);
>   }
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> index 34271c14d359..a997d27a504d 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
> @@ -120,6 +120,7 @@ struct amdxdna_dev {
>   
>   	struct mutex			dev_lock; /* per device lock */
>   	struct list_head		client_list;
> +	struct mutex			client_lock; /* client_list */
>   	struct amdxdna_fw_ver		fw_ver;
>   	struct rw_semaphore		notifier_lock; /* for mmu notifier*/
>   	struct workqueue_struct		*notifier_wq;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq lifetime race during device removal
  2026-06-11  5:51 ` [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq " Lizhi Hou
@ 2026-06-12 18:26   ` Mario Limonciello
  0 siblings, 0 replies; 7+ messages in thread
From: Mario Limonciello @ 2026-06-12 18:26 UTC (permalink / raw)
  To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, karol.wachowski
  Cc: linux-kernel, max.zhen, sonal.santan



On 6/11/26 00:51, Lizhi Hou wrote:
> amdxdna_remove() destroys notifier_wq. If amdxdna_gem_obj_free() is
> called after device removal, it may attempt to flush notifier_wq,
> resulting in a use-after-free.
> 
> Fix the race by allocating notifier_wq with
> drmm_alloc_ordered_workqueue(), so its lifetime is managed by DRM and
> remains valid until all managed resources are released.
> 
> Fixes: e486147c912f ("accel/amdxdna: Add BO import and export")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/amdxdna_pci_drv.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index 470bf4fc744b..b943c06a451e 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -389,9 +389,9 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	if (ret)
>   		return ret;
>   
> -	xdna->notifier_wq = alloc_ordered_workqueue("notifier_wq", WQ_MEM_RECLAIM);
> -	if (!xdna->notifier_wq) {
> -		ret = -ENOMEM;
> +	xdna->notifier_wq = drmm_alloc_ordered_workqueue(ddev, "notifier_wq", WQ_MEM_RECLAIM);
> +	if (IS_ERR(xdna->notifier_wq)) {
> +		ret = PTR_ERR(xdna->notifier_wq);
>   		goto iommu_fini;
>   	}
>   
> @@ -400,7 +400,7 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	mutex_unlock(&xdna->dev_lock);
>   	if (ret) {
>   		XDNA_ERR(xdna, "Hardware init failed, ret %d", ret);
> -		goto destroy_notifier_wq;
> +		goto iommu_fini;
>   	}
>   
>   	ret = amdxdna_sysfs_init(xdna);
> @@ -424,8 +424,6 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	mutex_lock(&xdna->dev_lock);
>   	xdna->dev_info->ops->fini(xdna);
>   	mutex_unlock(&xdna->dev_lock);
> -destroy_notifier_wq:
> -	destroy_workqueue(xdna->notifier_wq);
>   iommu_fini:
>   	amdxdna_iommu_fini(xdna);
>   	return ret;
> @@ -436,8 +434,6 @@ static void amdxdna_remove(struct pci_dev *pdev)
>   	struct amdxdna_dev *xdna = pci_get_drvdata(pdev);
>   	struct amdxdna_client *client;
>   
> -	destroy_workqueue(xdna->notifier_wq);
> -
>   	drm_dev_unplug(&xdna->ddev);
>   	amdxdna_sysfs_fini(xdna);
>   


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V3 3/3] accel/amdxdna: Fix iommu domain lifetime race during device removal
  2026-06-11  5:51 ` [PATCH V3 3/3] accel/amdxdna: Fix iommu domain " Lizhi Hou
@ 2026-06-12 18:26   ` Mario Limonciello
  0 siblings, 0 replies; 7+ messages in thread
From: Mario Limonciello @ 2026-06-12 18:26 UTC (permalink / raw)
  To: Lizhi Hou, ogabbay, quic_jhugo, dri-devel, karol.wachowski
  Cc: linux-kernel, max.zhen, sonal.santan



On 6/11/26 00:51, Lizhi Hou wrote:
> When force_iova mode is enabled, amdxdna_remove() frees xdna->domain. If
> amdxdna_gem_obj_free() is called after device removal, it may attempt to
> access xdna->domain, resulting in a use-after-free.
> 
> Fix the race by adding freeing xdna->domain as a managed release action,
> so its lifetime is managed by DRM and remains valid until all managed
> resources are released.
> 
> Fixes: ece3e8980907 ("accel/amdxdna: Allow forcing IOVA-based DMA via module parameter")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>   drivers/accel/amdxdna/amdxdna_iommu.c | 43 ++++++++++++++++++---------
>   1 file changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/accel/amdxdna/amdxdna_iommu.c b/drivers/accel/amdxdna/amdxdna_iommu.c
> index eff00131d0f8..4f245b969eef 100644
> --- a/drivers/accel/amdxdna/amdxdna_iommu.c
> +++ b/drivers/accel/amdxdna/amdxdna_iommu.c
> @@ -4,6 +4,7 @@
>    */
>   
>   #include <drm/amdxdna_accel.h>
> +#include <drm/drm_managed.h>
>   #include <linux/iommu.h>
>   #include <linux/iova.h>
>   
> @@ -153,10 +154,30 @@ void amdxdna_iommu_free(struct amdxdna_dev *xdna, size_t size,
>   	free_pages((unsigned long)cpu_addr, get_order(size));
>   }
>   
> +static void amdxdna_cleanup_force_iova(struct drm_device *dev, void *res)
> +{
> +	struct amdxdna_dev *xdna = to_xdna_dev(dev);
> +
> +	if (xdna->domain) {
> +		iommu_detach_group(xdna->domain, xdna->group);
> +		put_iova_domain(&xdna->iovad);
> +		iova_cache_put();
> +		iommu_domain_free(xdna->domain);
> +	}
> +
> +	iommu_group_put(xdna->group);
> +}
> +
> +void amdxdna_iommu_fini(struct amdxdna_dev *xdna)
> +{
> +	if (xdna->group && !xdna->domain)
> +		iommu_group_put(xdna->group);
> +}
> +
>   int amdxdna_iommu_init(struct amdxdna_dev *xdna)
>   {
>   	unsigned long order;
> -	int ret;
> +	int ret = 0;
>   
>   	xdna->group = iommu_group_get(xdna->ddev.dev);
>   	if (!xdna->group || !force_iova)
> @@ -182,8 +203,14 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna)
>   	if (ret)
>   		goto put_iova;
>   
> +	ret = drmm_add_action(&xdna->ddev, amdxdna_cleanup_force_iova, NULL);
> +	if (ret)
> +		goto detach_group;
> +
>   	return 0;
>   
> +detach_group:
> +	iommu_detach_group(xdna->domain, xdna->group);
>   put_iova:
>   	put_iova_domain(&xdna->iovad);
>   	iova_cache_put();
> @@ -191,20 +218,8 @@ int amdxdna_iommu_init(struct amdxdna_dev *xdna)
>   	iommu_domain_free(xdna->domain);
>   put_group:
>   	iommu_group_put(xdna->group);
> +	xdna->group = NULL;
>   	xdna->domain = NULL;
>   
>   	return ret;
>   }
> -
> -void amdxdna_iommu_fini(struct amdxdna_dev *xdna)
> -{
> -	if (xdna->domain) {
> -		iommu_detach_group(xdna->domain, xdna->group);
> -		put_iova_domain(&xdna->iovad);
> -		iova_cache_put();
> -		iommu_domain_free(xdna->domain);
> -	}
> -
> -	if (xdna->group)
> -		iommu_group_put(xdna->group);
> -}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal
  2026-06-12 18:25 ` [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client " Mario Limonciello
@ 2026-06-29 18:00   ` Lizhi Hou
  0 siblings, 0 replies; 7+ messages in thread
From: Lizhi Hou @ 2026-06-29 18:00 UTC (permalink / raw)
  To: Mario Limonciello, ogabbay, quic_jhugo, dri-devel, karol.wachowski
  Cc: linux-kernel, max.zhen, sonal.santan

Applied the patches to drm-misc-fixes.

On 6/12/26 11:25, Mario Limonciello wrote:
>
>
> On 6/11/26 00:51, Lizhi Hou wrote:
>> In amdxdna_remove(), all amdxdna_client structures are freed after
>> calling drm_dev_unplug(). However, drm_dev_unplug() does not force
>> existing file descriptors to be closed, so amdxdna_drm_close() may be
>> called after amdxdna_remove() has completed.
>>
>> As a result, accessing client->pid for debug output in
>> amdxdna_drm_close() can lead to a use-after-free, since the access is
>> not protected by drm_dev_enter().
>>
>> Fix this by decoupling hardware teardown from client cleanup.
>> amdxdna_remove() only performs hardware-related cleanup, while
>> per-client resources are released from amdxdna_drm_close() when the
>> corresponding file is closed.
>>
>> Fixes: be462c97b7df ("accel/amdxdna: Add hardware context")
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   drivers/accel/amdxdna/amdxdna_pci_drv.c | 23 ++++++++++-------------
>>   drivers/accel/amdxdna/amdxdna_pci_drv.h |  1 +
>>   2 files changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c 
>> b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> index 65489bb3f2b0..470bf4fc744b 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
>> @@ -138,9 +138,11 @@ static int amdxdna_drm_open(struct drm_device 
>> *ddev, struct drm_file *filp)
>>               xdna->dev_info->dev_heap_max_size);
>>       mutex_init(&client->mm_lock);
>>   +    mutex_lock(&xdna->client_lock);
>>       mutex_lock(&xdna->dev_lock);
>>       list_add_tail(&client->node, &xdna->client_list);
>>       mutex_unlock(&xdna->dev_lock);
>> +    mutex_unlock(&xdna->client_lock);
>>         filp->driver_priv = client;
>>       client->filp = filp;
>> @@ -174,18 +176,14 @@ static void amdxdna_drm_close(struct drm_device 
>> *ddev, struct drm_file *filp)
>>   {
>>       struct amdxdna_client *client = filp->driver_priv;
>>       struct amdxdna_dev *xdna = to_xdna_dev(ddev);
>> -    int idx;
>>         XDNA_DBG(xdna, "closing pid %d", client->pid);
>>   -    if (!drm_dev_enter(&xdna->ddev, &idx))
>> -        return;
>> -
>> +    mutex_lock(&xdna->client_lock);
>>       mutex_lock(&xdna->dev_lock);
>>       amdxdna_client_cleanup(client);
>>       mutex_unlock(&xdna->dev_lock);
>> -
>> -    drm_dev_exit(idx);
>> +    mutex_unlock(&xdna->client_lock);
>>   }
>>     static int amdxdna_drm_get_info_ioctl(struct drm_device *dev, 
>> void *data, struct drm_file *filp)
>> @@ -372,6 +370,7 @@ static int amdxdna_probe(struct pci_dev *pdev, 
>> const struct pci_device_id *id)
>>           return -ENODEV;
>>         drmm_mutex_init(ddev, &xdna->dev_lock);
>> +    drmm_mutex_init(ddev, &xdna->client_lock);
>>       init_rwsem(&xdna->notifier_lock);
>>       INIT_LIST_HEAD(&xdna->client_list);
>>       pci_set_drvdata(pdev, xdna);
>> @@ -442,18 +441,16 @@ static void amdxdna_remove(struct pci_dev *pdev)
>>       drm_dev_unplug(&xdna->ddev);
>>       amdxdna_sysfs_fini(xdna);
>>   +    mutex_lock(&xdna->client_lock);
>>       mutex_lock(&xdna->dev_lock);
>> -    client = list_first_entry_or_null(&xdna->client_list,
>> -                      struct amdxdna_client, node);
>> -    while (client) {
>> -        amdxdna_client_cleanup(client);
>> -
>> -        client = list_first_entry_or_null(&xdna->client_list,
>> -                          struct amdxdna_client, node);
>> +    list_for_each_entry(client, &xdna->client_list, node) {
>> +        amdxdna_hwctx_remove_all(client);
>> +        amdxdna_sva_fini(client);
>>       }
>>         xdna->dev_info->ops->fini(xdna);
>>       mutex_unlock(&xdna->dev_lock);
>> +    mutex_unlock(&xdna->client_lock);
>>         amdxdna_iommu_fini(xdna);
>>   }
>> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.h 
>> b/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> index 34271c14d359..a997d27a504d 100644
>> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.h
>> @@ -120,6 +120,7 @@ struct amdxdna_dev {
>>         struct mutex            dev_lock; /* per device lock */
>>       struct list_head        client_list;
>> +    struct mutex            client_lock; /* client_list */
>>       struct amdxdna_fw_ver        fw_ver;
>>       struct rw_semaphore        notifier_lock; /* for mmu notifier*/
>>       struct workqueue_struct        *notifier_wq;
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-29 18:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-11  5:51 [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
2026-06-11  5:51 ` [PATCH V3 2/3] accel/amdxdna: Fix notifier_wq " Lizhi Hou
2026-06-12 18:26   ` Mario Limonciello
2026-06-11  5:51 ` [PATCH V3 3/3] accel/amdxdna: Fix iommu domain " Lizhi Hou
2026-06-12 18:26   ` Mario Limonciello
2026-06-12 18:25 ` [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client " Mario Limonciello
2026-06-29 18:00   ` Lizhi Hou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome