Re: [PATCH V3 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal

[Lizhi Hou <lizhi.hou@amd.com>]

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;
>