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

[Lizhi Hou]

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@xxxxxxx>
> Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>
> > ---
> >   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;