accel/amdxdna: Fix amdxdna_client lifetime race during device removal

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")
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
Link: https://patch.msgid.link/20260611055150.3070216-1-lizhi.hou@amd.com

diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index 65489bb3f2b074acc6f92550fd93b4a684e8ef18..5937666829407aca3858a520494ca59c2a29e2de 100644 (file)
--- 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)
@@ -371,6 +369,10 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
        if (!xdna->dev_info)
                return -ENODEV;
 
+       ret = drmm_mutex_init(ddev, &xdna->client_lock);
+       if (ret)
+               return ret;
+
        drmm_mutex_init(ddev, &xdna->dev_lock);
        init_rwsem(&xdna->notifier_lock);
        INIT_LIST_HEAD(&xdna->client_list);
@@ -442,18 +444,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 34271c14d359f8a5aeb1cbc4e44a8ed910f15b59..a997d27a504dcf5c4687ff16b33b9b968b2ab5ec 100644 (file)
--- 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;