* [PATCH V2 2/3] accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap()
2026-06-08 16:14 [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
@ 2026-06-08 16:14 ` Lizhi Hou
2026-06-08 16:32 ` sashiko-bot
2026-06-08 16:14 ` [PATCH V2 3/3] accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo() Lizhi Hou
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Lizhi Hou @ 2026-06-08 16:14 UTC (permalink / raw)
To: ogabbay, quic_jhugo, dri-devel, mario.limonciello,
karol.wachowski
Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan
amdxdna_drm_gem_mmap() can be called after drm_dev_unplug() during device
removal. In that case, the mmap path may access amdxdna_client after it
has been freed, leading to a use-after-free.
Guard the mmap path with drm_dev_enter()/drm_dev_exit() and return an
error if the device is already unplugged.
Fixes: 91f4da826c08 ("accel/amdxdna: Add command doorbell and wait support")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
drivers/accel/amdxdna/amdxdna_pci_drv.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index a22c986f1b33..c18ddad9f645 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -253,15 +253,24 @@ static int amdxdna_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
{
struct drm_file *drm_filp = filp->private_data;
struct amdxdna_client *client = drm_filp->driver_priv;
- struct amdxdna_dev *xdna = client->xdna;
+ struct amdxdna_dev *xdna;
+ int ret, idx;
if (likely(vma->vm_pgoff >= DRM_FILE_PAGE_OFFSET_START))
return drm_gem_mmap(filp, vma);
- if (!xdna->dev_info->ops->mmap)
- return -EOPNOTSUPP;
+ if (!drm_dev_enter(drm_filp->minor->dev, &idx))
+ return -ENODEV;
- return xdna->dev_info->ops->mmap(client, vma);
+ xdna = client->xdna;
+ if (xdna->dev_info->ops->mmap)
+ ret = xdna->dev_info->ops->mmap(client, vma);
+ else
+ ret = -EOPNOTSUPP;
+
+ drm_dev_exit(idx);
+
+ return ret;
}
static const struct drm_ioctl_desc amdxdna_drm_ioctls[] = {
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/3] accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap()
2026-06-08 16:14 ` [PATCH V2 2/3] accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap() Lizhi Hou
@ 2026-06-08 16:32 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-08 16:32 UTC (permalink / raw)
To: Lizhi Hou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Caching the `vm_area_struct` pointer in the MMU interval notifier leads to Use-After-Free on `munmap()`.
- [High] The patch incompletely fixes a Use-After-Free of `amdxdna_client` during device removal. The UAF remains in the standard GEM mmap path, IOCTLs, and GEM destruction callbacks.
- [High] Double put of DRM GEM object reference on the error path in `amdxdna_insert_pages`.
- [High] Premature destruction of `notifier_wq` leads to Use-After-Free when unmapping VMAs after device removal.
--
commit 4a66abc4067bb1f72f27f8b25d0d1de298787835
Author: Lizhi Hou <lizhi.hou@amd.com>
accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap()
amdxdna_drm_gem_mmap() can be called after drm_dev_unplug() during device
removal. In that case, the mmap path may access amdxdna_client after it
has been freed, leading to a use-after-free.
Guard the mmap path with drm_dev_enter()/drm_dev_exit() and return an
error if the device is already unplugged.
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index a22c986f1b331..c18ddad9f6459 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -253,15 +253,24 @@ static int amdxdna_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> struct drm_file *drm_filp = filp->private_data;
> struct amdxdna_client *client = drm_filp->driver_priv;
> - struct amdxdna_dev *xdna = client->xdna;
> + struct amdxdna_dev *xdna;
> + int ret, idx;
>
> if (likely(vma->vm_pgoff >= DRM_FILE_PAGE_OFFSET_START))
> return drm_gem_mmap(filp, vma);
>
[Severity: High]
This is a pre-existing issue, but does this bypass of drm_dev_enter() leave
the use-after-free unresolved for standard GEM mmaps?
The patch adds drm_dev_enter() below this check, but if the condition
(vma->vm_pgoff >= DRM_FILE_PAGE_OFFSET_START) is met, it calls drm_gem_mmap()
which eventually calls amdxdna_gem_obj_mmap() without the device removal
protection. Since amdxdna_remove() frees all client structures regardless of
open file descriptors, abo->client becomes a dangling pointer here.
Additionally, do the IOCTL handlers (like amdxdna_drm_config_hwctx_ioctl in
drivers/accel/amdxdna/amdxdna_ctx.c) and GEM destruction callbacks
(like amdxdna_gem_del_bo_usage in drivers/accel/amdxdna/amdxdna_gem.c)
also need drm_dev_enter() protection? They appear to extract and dereference
filp->driver_priv without synchronization.
> - if (!xdna->dev_info->ops->mmap)
> - return -EOPNOTSUPP;
> + if (!drm_dev_enter(drm_filp->minor->dev, &idx))
> + return -ENODEV;
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but could the error path in
amdxdna_insert_pages() lead to a double put of the GEM object reference?
In drivers/accel/amdxdna/amdxdna_gem.c:amdxdna_insert_pages(), if page
insertion fails:
if (ret) {
XDNA_ERR(xdna, "Failed insert pages %d", ret);
vma->vm_ops->close(vma);
return ret;
}
For shmem GEM objects, calling close() drops the object reference. When this
error returns to drm_gem_mmap_obj(), it executes its error cleanup path which
calls drm_gem_object_put() again, causing the same reference to be dropped
twice.
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to destroy the
notifier_wq during device removal?
In drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_remove():
destroy_workqueue(xdna->notifier_wq);
If userspace retains mapped VMAs after the device is removed, a subsequent
munmap() will trigger amdxdna_hmm_invalidate() in
drivers/accel/amdxdna/amdxdna_gem.c, which queues work to the destroyed
workqueue:
queue_work(xdna->notifier_wq, &mapp->hmm_unreg_work);
Can this lead to a use-after-free if the workqueue was already freed?
[Severity: Critical]
This isn't a bug introduced by this patch, but does caching the vm_area_struct
pointer lead to a use-after-free during munmap()?
In drivers/accel/amdxdna/amdxdna_gem.c:amdxdna_hmm_register(), the vma
pointer is cached:
mapp->vma = vma;
The core MM subsystem can split or free VMA objects independently. When
munmap() is called, the notifier queues amdxdna_hmm_unreg_work() to run
asynchronously. By the time amdxdna_umap_release() runs, the core MM has
already freed the VMA, but the driver dereferences it:
if (is_import_bo(abo) && vma->vm_file && vma->vm_file->f_mapping)
Does this need to be tracked differently to avoid accessing a freed VMA?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608161418.2748988-1-lizhi.hou@amd.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 3/3] accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo()
2026-06-08 16:14 [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
2026-06-08 16:14 ` [PATCH V2 2/3] accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap() Lizhi Hou
@ 2026-06-08 16:14 ` Lizhi Hou
2026-06-08 16:28 ` sashiko-bot
2026-06-08 16:30 ` [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal sashiko-bot
2026-06-09 18:17 ` Lizhi Hou
3 siblings, 1 reply; 7+ messages in thread
From: Lizhi Hou @ 2026-06-08 16:14 UTC (permalink / raw)
To: ogabbay, quic_jhugo, dri-devel, mario.limonciello,
karol.wachowski
Cc: Lizhi Hou, linux-kernel, max.zhen, sonal.santan
amdxdna_show_fdinfo() can be called after drm_dev_unplug() during device
removal. In that case, the show_fdinfo path may access amdxdna_client
after it has been freed, leading to a use-after-free.
Guard the show_fdinfo path with drm_dev_enter()/drm_dev_exit() and return
if the device is already unplugged.
Fixes: e0169d0c690f ("accel/amdxdna: Expose per-client BO memory usage via fdinfo")
Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
---
drivers/accel/amdxdna/amdxdna_pci_drv.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index c18ddad9f645..e6b541db3898 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -296,6 +296,10 @@ static void amdxdna_show_fdinfo(struct drm_printer *p, struct drm_file *filp)
struct amdxdna_client *client = filp->driver_priv;
size_t heap_usage, external_usage, internal_usage;
char *drv_name = filp->minor->dev->driver->name;
+ int idx;
+
+ if (!drm_dev_enter(filp->minor->dev, &idx))
+ return;
mutex_lock(&client->mm_lock);
@@ -320,6 +324,8 @@ static void amdxdna_show_fdinfo(struct drm_printer *p, struct drm_file *filp)
* drm-shared-memory counts BO shared with other processes/devices.
*/
drm_show_memory_stats(p, filp);
+
+ drm_dev_exit(idx);
}
static const struct file_operations amdxdna_fops = {
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH V2 3/3] accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo()
2026-06-08 16:14 ` [PATCH V2 3/3] accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo() Lizhi Hou
@ 2026-06-08 16:28 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-08 16:28 UTC (permalink / raw)
To: Lizhi Hou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The patch addresses a Use-After-Free in `amdxdna_show_fdinfo()`, but identical UAF vulnerabilities exist across most ioctls and GEM destruction paths due to broken client lifecycle management.
- [Critical] Premature destruction of `notifier_wq` in `amdxdna_remove()` causes a Use-After-Free when GEM objects are subsequently freed.
- [High] `amdxdna_sva_init()` leaves `client->sva` as a dangling pointer if `iommu_sva_get_pasid()` fails, leading to a double `iommu_sva_unbind_device()` on client cleanup.
--
commit 225c80eb0fc36a5511f44ce636794ce0b887dbc7
Author: Lizhi Hou <lizhi.hou@amd.com>
accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo()
This patch guards the show_fdinfo path with drm_dev_enter() and
drm_dev_exit() to prevent use-after-free of the amdxdna_client structure
after the device has been unplugged.
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index c18ddad9f6459..e6b541db38981 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -296,6 +296,10 @@ static void amdxdna_show_fdinfo(struct drm_printer *p, struct drm_file *filp)
> struct amdxdna_client *client = filp->driver_priv;
> size_t heap_usage, external_usage, internal_usage;
> char *drv_name = filp->minor->dev->driver->name;
> + int idx;
> +
> + if (!drm_dev_enter(filp->minor->dev, &idx))
> + return;
[Severity: Critical]
This is a pre-existing issue, but while this patch addresses the UAF in
amdxdna_show_fdinfo(), do the same vulnerabilities exist across other ioctls
and GEM destruction paths due to broken client lifecycle management?
For example, amdxdna_remove() forcefully frees each amdxdna_client struct
by calling amdxdna_client_cleanup() while file descriptors might still be
open:
drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_remove() {
...
client = list_first_entry_or_null(&xdna->client_list, ...);
while (client) {
amdxdna_client_cleanup(client);
...
}
}
Meanwhile, IOCTL handlers like amdxdna_drm_submit_cmd_ioctl(),
amdxdna_drm_config_hwctx_ioctl(), and amdxdna_drm_wait_cmd_ioctl() omit
drm_dev_enter(), extracting the client pointer directly from
filp->driver_priv:
drivers/accel/amdxdna/amdxdna_ctx.c:amdxdna_drm_submit_cmd_ioctl() {
struct amdxdna_client *client = filp->driver_priv;
...
return amdxdna_drm_submit_execbuf(client, args);
}
If amdxdna_remove() runs concurrently, will these IOCTLs access the freed
client structure, causing a Use-After-Free? Similarly, when the file
descriptor is eventually closed post-unplug, will GEM destruction paths
like amdxdna_gem_obj_close() and amdxdna_gem_dev_obj_free() dereference the
already-freed abo->client pointer?
>
> mutex_lock(&client->mm_lock);
>
[ ... ]
[Severity: High]
This is a pre-existing issue, but does amdxdna_sva_init() leave client->sva
as a dangling pointer if iommu_sva_get_pasid() fails?
In amdxdna_sva_init(), if iommu_sva_bind_device() succeeds but getting the
pasid fails, the device is unbound but client->sva is not set to NULL:
drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_sva_init() {
...
client->pasid = iommu_sva_get_pasid(client->sva);
if (client->pasid == IOMMU_PASID_INVALID) {
iommu_sva_unbind_device(client->sva);
XDNA_ERR(xdna, "SVA get pasid failed");
return -ENODEV;
}
}
Then in amdxdna_drm_open(), if amdxdna_sva_init() fails but
amdxdna_use_carveout(xdna) is true, the driver issues a warning and allows
the client initialization to succeed.
When the file descriptor is closed and amdxdna_client_cleanup() calls
amdxdna_sva_fini(), will this dangling pointer bypass the IS_ERR_OR_NULL()
check and cause a double iommu_sva_unbind_device()?
drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_sva_fini() {
if (IS_ERR_OR_NULL(client->sva))
return;
iommu_sva_unbind_device(client->sva);
}
[Severity: Critical]
This is a pre-existing issue, but does the premature destruction of
notifier_wq in amdxdna_remove() cause a Use-After-Free when GEM objects
are subsequently freed?
amdxdna_remove() destroys the workqueue before GEM objects are completely
released:
drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_remove() {
...
destroy_workqueue(xdna->notifier_wq);
...
drm_dev_unplug(&xdna->ddev);
}
When a GEM object's refcount drops to zero later, amdxdna_gem_obj_free()
is invoked and unconditionally calls flush_workqueue() on the destroyed
workqueue:
drivers/accel/amdxdna/amdxdna_gem.c:amdxdna_gem_obj_free() {
...
amdxdna_hmm_unregister(abo, NULL);
flush_workqueue(xdna->notifier_wq);
}
Could the destruction of the workqueue be deferred to the DRM device release
callback instead of the PCI remove callback?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608161418.2748988-1-lizhi.hou@amd.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal
2026-06-08 16:14 [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
2026-06-08 16:14 ` [PATCH V2 2/3] accel/amdxdna: Fix amdxdna_client accessing race in amdxdna_drm_gem_mmap() Lizhi Hou
2026-06-08 16:14 ` [PATCH V2 3/3] accel/amdxdna: Fix amdxdna_client race in amdxdna_show_fdinfo() Lizhi Hou
@ 2026-06-08 16:30 ` sashiko-bot
2026-06-09 18:17 ` Lizhi Hou
3 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-08 16:30 UTC (permalink / raw)
To: Lizhi Hou; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free in IOCTLs due to lack of device unplug synchronization
- [High] Deterministic Use-After-Free in GEM callbacks when userspace closes the file descriptor after device removal
- [High] Double free/unbind of the IOMMU SVA device handle on PASID failure
--
commit 8636266787707619953362d31190f3d7097a3d27
Author: Lizhi Hou <lizhi.hou@amd.com>
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 moving the debug output after drm_dev_enter(). Similarly,
add the drm_dev_enter() protection for amdxdna_drm_open().
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index 1f066ed8d2365..a22c986f1b331 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -108,10 +108,16 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
> {
> struct amdxdna_dev *xdna = to_xdna_dev(ddev);
> struct amdxdna_client *client;
> + int ret = 0, idx;
> +
> + if (!drm_dev_enter(ddev, &idx))
> + return -ENODEV;
>
> client = kzalloc_obj(*client);
> - if (!client)
> - return -ENOMEM;
> + if (!client) {
> + ret = -ENOMEM;
> + goto dev_exit;
> + }
>
> client->pid = pid_nr(rcu_access_pointer(filp->pid));
> client->xdna = xdna;
> @@ -125,7 +131,8 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
> if (!amdxdna_use_carveout(xdna)) {
> XDNA_ERR(xdna, "PASID unavailable and carveout not configured");
> kfree(client);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto dev_exit;
> }
[Severity: High]
This isn't a bug introduced by this patch, but does this error handling path
lead to a double-free of the SVA device handle?
In amdxdna_sva_init(), if iommu_sva_get_pasid() fails, the code calls
iommu_sva_unbind_device(client->sva) but leaves client->sva pointing to the
unbound handle:
drivers/accel/amdxdna/amdxdna_pci_drv.c:amdxdna_sva_init() {
client->pasid = iommu_sva_get_pasid(client->sva);
if (client->pasid == IOMMU_PASID_INVALID) {
iommu_sva_unbind_device(client->sva);
XDNA_ERR(xdna, "SVA get pasid failed");
return -ENODEV;
}
}
If amdxdna_use_carveout() is true, the error is ignored here, and the open
succeeds.
Later, when amdxdna_client_cleanup() calls amdxdna_sva_fini(), the
IS_ERR_OR_NULL(client->sva) check passes since client->sva is a dangling
pointer, leading to a second call to iommu_sva_unbind_device(client->sva).
[ ... ]
> @@ -175,11 +186,10 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp)
> 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;
>
> + XDNA_DBG(xdna, "closing pid %d", client->pid);
> mutex_lock(&xdna->dev_lock);
> amdxdna_client_cleanup(client);
> mutex_unlock(&xdna->dev_lock);
[Severity: High]
This is a pre-existing issue, but while drm_dev_enter() now protects open
and close, are the ioctl handlers still vulnerable to a use-after-free during
device removal?
The DRM core's drm_ioctl() does not hold drm_dev_enter() and only performs a
lockless check of drm_dev_is_unplugged(). If an ioctl is running concurrently
with amdxdna_remove(), drm_dev_unplug() will not wait for the ioctl to finish.
The client structure can be freed while concurrently running ioctls try to
access it.
For example, in amdxdna_drm_create_bo_ioctl():
drivers/accel/amdxdna/amdxdna_gem.c:amdxdna_drm_create_bo_ioctl() {
struct amdxdna_client *client = filp->driver_priv;
...
}
Should drm_dev_enter() be added to the ioctl handlers as well to prevent
accessing a freed client structure?
[Severity: High]
This is a pre-existing issue, but does freeing the client structure during
device removal cause a deterministic use-after-free when GEM objects are
eventually closed?
When amdxdna_remove() forcefully frees the client structure via
amdxdna_client_cleanup(), active GEM objects created by this client are not
destroyed because the DRM core holds references to them as long as the user's
file descriptor remains open.
When the user eventually closes the file descriptor or unmaps the GEM objects,
amdxdna_gem_obj_close() is invoked, which accesses the freed client structure:
drivers/accel/amdxdna/amdxdna_gem.c:amdxdna_gem_del_bo_usage() {
guard(mutex)(&client->mm_lock);
...
}
Could the client structure be kept alive until all GEM objects drop their
references?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260608161418.2748988-1-lizhi.hou@amd.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal
2026-06-08 16:14 [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal Lizhi Hou
` (2 preceding siblings ...)
2026-06-08 16:30 ` [PATCH V2 1/3] accel/amdxdna: Fix amdxdna_client lifetime race during device removal sashiko-bot
@ 2026-06-09 18:17 ` Lizhi Hou
3 siblings, 0 replies; 7+ messages in thread
From: Lizhi Hou @ 2026-06-09 18:17 UTC (permalink / raw)
To: ogabbay, quic_jhugo, dri-devel, mario.limonciello,
karol.wachowski
Cc: linux-kernel, max.zhen, sonal.santan
Please ignore this patch set. I would figure out a complete fix and run
sashiko scan locally.
Sorry for the noise.
Lizhi
On 6/8/26 09:14, 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 moving the debug output after drm_dev_enter(). Similarly,
> add the drm_dev_enter() protection for amdxdna_drm_open().
>
> Fixes: be462c97b7df ("accel/amdxdna: Add hardware context")
> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
> ---
> drivers/accel/amdxdna/amdxdna_pci_drv.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index 1f066ed8d236..a22c986f1b33 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -108,10 +108,16 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
> {
> struct amdxdna_dev *xdna = to_xdna_dev(ddev);
> struct amdxdna_client *client;
> + int ret = 0, idx;
> +
> + if (!drm_dev_enter(ddev, &idx))
> + return -ENODEV;
>
> client = kzalloc_obj(*client);
> - if (!client)
> - return -ENOMEM;
> + if (!client) {
> + ret = -ENOMEM;
> + goto dev_exit;
> + }
>
> client->pid = pid_nr(rcu_access_pointer(filp->pid));
> client->xdna = xdna;
> @@ -125,7 +131,8 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
> if (!amdxdna_use_carveout(xdna)) {
> XDNA_ERR(xdna, "PASID unavailable and carveout not configured");
> kfree(client);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto dev_exit;
> }
> }
> }
> @@ -145,7 +152,11 @@ static int amdxdna_drm_open(struct drm_device *ddev, struct drm_file *filp)
> client->filp = filp;
>
> XDNA_DBG(xdna, "pid %d opened", client->pid);
> - return 0;
> +
> +dev_exit:
> + drm_dev_exit(idx);
> +
> + return ret;
> }
>
> static void amdxdna_client_cleanup(struct amdxdna_client *client)
> @@ -175,11 +186,10 @@ static void amdxdna_drm_close(struct drm_device *ddev, struct drm_file *filp)
> 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;
>
> + XDNA_DBG(xdna, "closing pid %d", client->pid);
> mutex_lock(&xdna->dev_lock);
> amdxdna_client_cleanup(client);
> mutex_unlock(&xdna->dev_lock);
^ permalink raw reply [flat|nested] 7+ messages in thread