[PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Guixin Liu]

We found an idxd_wq use-after-free issue with kasan
when remove the idxd PCI device:

BUG: KASAN: slab-use-after-free in idxd_device_drv_remove+0x1f8/0x240 [idxd]
Call Trace:
  <TASK>
  dump_stack_lvl+0x32/0x50
  print_address_description.constprop.0+0x2c/0x390
  ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
  print_report+0xba/0x280
  ? kasan_addr_to_slab+0x9/0xa0
  ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
  kasan_report+0xab/0xe0
  ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
  idxd_device_drv_remove+0x1f8/0x240 [idxd]
  device_release_driver_internal+0x391/0x560
  bus_remove_device+0x1f5/0x3f0
  device_del+0x392/0x990
  ? __pfx_device_del+0x10/0x10
  ? kobject_cleanup+0x117/0x360
  ? idxd_unregister_devices+0x229/0x320 [idxd]
  device_unregister+0x13/0xa0
  idxd_remove+0x4f/0x1b0 [idxd]
  pci_device_remove+0xa7/0x1d0
  device_release_driver_internal+0x391/0x560
  ? pci_pme_active+0x1e/0x450
  pci_stop_bus_device+0x10a/0x150
  pci_stop_and_remove_bus_device_locked+0x16/0x30
  remove_store+0xcf/0xe0

Freed by task 15535:
  kasan_save_stack+0x1c/0x40
  kasan_set_track+0x21/0x30
  kasan_save_free_info+0x27/0x40
  ____kasan_slab_free+0x171/0x240
  slab_free_freelist_hook+0xde/0x190
  __kmem_cache_free+0x19e/0x310
  device_release+0x98/0x210
  kobject_cleanup+0x102/0x360
  idxd_unregister_devices+0xb3/0x320 [idxd]
  dxd_remove+0x3f/0x1b0 [idxd]
  pci_device_remove+0xa7/0x1d0
  device_release_driver_internal+0x391/0x560
  pci_stop_bus_device+0x10a/0x150
  pci_stop_and_remove_bus_device_locked+0x16/0x30
  remove_store+0xcf/0xe0

In the idxd_remove() flow, when execution reaches
idxd_unregister_devices(), all idxd_wq instances have already been
freed. Subsequently, when device_unregister(idxd_confdev(idxd)) is
executed, it calls into idxd_device_drv_remove() which accesses the
already-freed idxd_wq. This fix resolves the issue by calling
device_release_driver() before idxd_unregister_devices().

Fixes: 98da0106aac0d ("dmanegine: idxd: fix resource free ordering on driver removal")
Co-developed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
v1 -> v2:
  1. Call device_release_driver() in advance instead of swapping the order of
     device_unregister() and idxd_unregister_devices().
  2. Add Co-developed-by: Shuai Xue <xueshuai@linux.alibaba.com>.
 drivers/dma/idxd/init.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f1cfc7790d95..3b0a0363ca65 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
 {
 	struct idxd_device *idxd = pci_get_drvdata(pdev);
 
+	/*
+	 * The idxd sub-driver's remove callback (idxd_device_drv_remove())
+	 * iterates idxd->wqs[] and accesses wq objects. We must unbind the
+	 * sub-driver before idxd_unregister_devices() frees these objects,
+	 * otherwise a use-after-free occurs.
+	 *
+	 * We cannot simply reorder device_unregister(idxd_confdev) before
+	 * idxd_unregister_devices() because device_del() -> kobject_del()
+	 * recursively removes the parent's sysfs directory, which destroys
+	 * children's sysfs entries. Subsequent device_unregister() on the
+	 * children then fails with "sysfs group 'power' not found".
+	 *
+	 * Use device_release_driver() to only unbind the driver (triggering
+	 * idxd_device_drv_remove()) without touching sysfs. Then safely
+	 * unregister children before the parent.
+	 */
+	device_release_driver(idxd_confdev(idxd));
 	idxd_unregister_devices(idxd);
+
 	/*
-	 * When ->release() is called for the idxd->conf_dev, it frees all the memory related
-	 * to the idxd context. The driver still needs those bits in order to do the rest of
-	 * the cleanup. However, we do need to unbound the idxd sub-driver. So take a ref
-	 * on the device here to hold off the freeing while allowing the idxd sub-driver
-	 * to unbind.
+	 * When ->release() is called for the idxd->conf_dev, it frees all the
+	 * memory related to the idxd context. The driver still needs those bits
+	 * in order to do the rest of the cleanup. So take a ref on the device
+	 * here to hold off the freeing.
 	 */
 	get_device(idxd_confdev(idxd));
 	device_unregister(idxd_confdev(idxd));
-- 
2.32.0.3.g01195cf9f

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Vinicius Costa Gomes]

On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu <kanie@linux.alibaba.com> wrote:
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index fb80803d5b57..c3cfd96074c9 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
> [ ... skip 14 lines ... ]
> +	 *
> +	 * Use device_release_driver() to only unbind the driver (triggering
> +	 * idxd_device_drv_remove()) without touching sysfs. Then safely
> +	 * unregister children before the parent.
> +	 */
> +	device_release_driver(idxd_confdev(idxd));

Below are sashiko comments verbatim, some notes:

 - I do believe the race condition that it points out is real (but
   very narrow), but not sure that we don't have a similar one in
   current code (i.e. it wasn't introduced by your patch);
 - The possible wq leak was definitely not introduced by this patch;
 - The question about what happens with the file references after a
   device is removed is a separate issue;

The patch looks good, not opposed to it at all, the only one I am a
bit concerned, about this patch, to be sure, is the race condition,
and the two "paths" to reach it. (the others we can fix separately)

sashiko.dev <sashiko@sashiko.dev>:

Does this introduce a race condition with sysfs?

Since the driver's sysfs nodes remain active after device_release_driver()
is called, a user with sysfs privileges could write to the bind attribute
to re-probe the driver before the device unregistration completes.

If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
idxd_unregister_devices(), does it cause a use-after-free when it
iterates over the idxd->wqs[] objects being freed, since
suppress_bind_attrs is not set?

Also, does this code leak the workqueue?

The workqueue idxd->wq is dynamically allocated using create_workqueue()
during device initialization, but destroy_workqueue(idxd->wq) does not
appear to be called in idxd_remove() or the sysfs device release callback.

>  	idxd_unregister_devices(idxd);

sashiko.dev <sashiko@sashiko.dev>:

If idxd_unregister_devices() drops the last reference to the child wq
objects and frees them, can idxd_shutdown() trigger a use-after-free
when called shortly after?

For example, if a hardware error interrupt fired during teardown,
idxd_shutdown()->flush_workqueue() could execute an error handler that
iterates over the freed idxd->wqs[] memory.

Additionally, if the hardware is wedged,
idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
iterates over idxd->wqs[] and attempts to acquire mutex_lock(&wq->wq_lock)
on the freed memory.

Is there also a missing reference count for the cdev open path?

Since idxd_unregister_devices() frees the wq structure, if user space
holds an open file descriptor, cdev_device_del() won't revoke it.
Can file operations like mmap or poll subsequently access the freed
ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
the parent wq device?

via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com

via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com

-- 
Vinicius

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Guixin Liu]

在 2026/4/23 05:34, Vinicius Costa Gomes 写道:
> On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu <kanie@linux.alibaba.com> wrote:
>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>> index fb80803d5b57..c3cfd96074c9 100644
>> --- a/drivers/dma/idxd/init.c
>> +++ b/drivers/dma/idxd/init.c
>> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
>> [ ... skip 14 lines ... ]
>> +	 *
>> +	 * Use device_release_driver() to only unbind the driver (triggering
>> +	 * idxd_device_drv_remove()) without touching sysfs. Then safely
>> +	 * unregister children before the parent.
>> +	 */
>> +	device_release_driver(idxd_confdev(idxd));
Thanks for the reply.
> Below are sashiko comments verbatim, some notes:
>
>   - I do believe the race condition that it points out is real (but
>     very narrow), but not sure that we don't have a similar one in
>     current code (i.e. it wasn't introduced by your patch);
>   - The possible wq leak was definitely not introduced by this patch;
>   - The question about what happens with the file references after a
>     device is removed is a separate issue;
>
> The patch looks good, not opposed to it at all, the only one I am a
> bit concerned, about this patch, to be sure, is the race condition,
> and the two "paths" to reach it. (the others we can fix separately)
>
> sashiko.dev <sashiko@sashiko.dev>:
>
> Does this introduce a race condition with sysfs?
>
> Since the driver's sysfs nodes remain active after device_release_driver()
> is called, a user with sysfs privileges could write to the bind attribute
> to re-probe the driver before the device unregistration completes.
>
> If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
> idxd_unregister_devices(), does it cause a use-after-free when it
> iterates over the idxd->wqs[] objects being freed, since
> suppress_bind_attrs is not set?
I think re-probe will call idxd_pci_probe() to allocate an new idxd device,
this avoids accessing stale data.
> Also, does this code leak the workqueue?
>
> The workqueue idxd->wq is dynamically allocated using create_workqueue()
> during device initialization, but destroy_workqueue(idxd->wq) does not
> appear to be called in idxd_remove() or the sysfs device release callback.
Looks like it's b7cb9a034305 ("dmaengine: idxd: Fix refcount underflow 
on module unload")
overlooked this, destory_workqueue(idxd->wq) is called in idxd_cleanup() 
-> idxd_cleanup_internals().

If so, I can send another patch to fix this.
>>   	idxd_unregister_devices(idxd);
> sashiko.dev <sashiko@sashiko.dev>:
>
> If idxd_unregister_devices() drops the last reference to the child wq
> objects and frees them, can idxd_shutdown() trigger a use-after-free
> when called shortly after?
>
> For example, if a hardware error interrupt fired during teardown,
> idxd_shutdown()->flush_workqueue() could execute an error handler that
> iterates over the freed idxd->wqs[] memory.
>
> Additionally, if the hardware is wedged,
> idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
> iterates over idxd->wqs[] and attempts to acquire mutex_lock(&wq->wq_lock)
> on the freed memory.
I took a look — idxd_shutdown() is only invoked by idxd_remove() and 
during system shutdown/reboot.
Is idxd_shutdown() ever reached from the idxd hardware error interrupt path?
>
> Is there also a missing reference count for the cdev open path?
>
> Since idxd_unregister_devices() frees the wq structure, if user space
> holds an open file descriptor, cdev_device_del() won't revoke it.
> Can file operations like mmap or poll subsequently access the freed
> ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
> the parent wq device?
To be honest, I'm not very familiar with the idxd driver. At this point,
it looks like the idxd driver needs a state machine to ensure mutual 
exclusion
across the various concurrent paths.

Best Regards,
Guixin Liu
>
> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>
> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Vinicius Costa Gomes]

Guixin Liu <kanie@linux.alibaba.com> writes:

> 在 2026/4/23 05:34, Vinicius Costa Gomes 写道:
>> On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu <kanie@linux.alibaba.com> wrote:
>>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>>> index fb80803d5b57..c3cfd96074c9 100644
>>> --- a/drivers/dma/idxd/init.c
>>> +++ b/drivers/dma/idxd/init.c
>>> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
>>> [ ... skip 14 lines ... ]
>>> +	 *
>>> +	 * Use device_release_driver() to only unbind the driver (triggering
>>> +	 * idxd_device_drv_remove()) without touching sysfs. Then safely
>>> +	 * unregister children before the parent.
>>> +	 */
>>> +	device_release_driver(idxd_confdev(idxd));
> Thanks for the reply.
>> Below are sashiko comments verbatim, some notes:
>>
>>   - I do believe the race condition that it points out is real (but
>>     very narrow), but not sure that we don't have a similar one in
>>     current code (i.e. it wasn't introduced by your patch);
>>   - The possible wq leak was definitely not introduced by this patch;
>>   - The question about what happens with the file references after a
>>     device is removed is a separate issue;
>>
>> The patch looks good, not opposed to it at all, the only one I am a
>> bit concerned, about this patch, to be sure, is the race condition,
>> and the two "paths" to reach it. (the others we can fix separately)
>>
>> sashiko.dev <sashiko@sashiko.dev>:
>>
>> Does this introduce a race condition with sysfs?
>>
>> Since the driver's sysfs nodes remain active after device_release_driver()
>> is called, a user with sysfs privileges could write to the bind attribute
>> to re-probe the driver before the device unregistration completes.
>>
>> If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
>> idxd_unregister_devices(), does it cause a use-after-free when it
>> iterates over the idxd->wqs[] objects being freed, since
>> suppress_bind_attrs is not set?
> I think re-probe will call idxd_pci_probe() to allocate an new idxd device,
> this avoids accessing stale data.

Yeah, I missed the (now obvious) that a new 'probe' also means a new
device. So this "path" is not valid. Thank you.

>> Also, does this code leak the workqueue?
>>
>> The workqueue idxd->wq is dynamically allocated using create_workqueue()
>> during device initialization, but destroy_workqueue(idxd->wq) does not
>> appear to be called in idxd_remove() or the sysfs device release callback.
> Looks like it's b7cb9a034305 ("dmaengine: idxd: Fix refcount underflow 
> on module unload")
> overlooked this, destory_workqueue(idxd->wq) is called in idxd_cleanup() 
> -> idxd_cleanup_internals().
>
> If so, I can send another patch to fix this.

Yes, please do.

>>>   	idxd_unregister_devices(idxd);
>> sashiko.dev <sashiko@sashiko.dev>:
>>
>> If idxd_unregister_devices() drops the last reference to the child wq
>> objects and frees them, can idxd_shutdown() trigger a use-after-free
>> when called shortly after?
>>
>> For example, if a hardware error interrupt fired during teardown,
>> idxd_shutdown()->flush_workqueue() could execute an error handler that
>> iterates over the freed idxd->wqs[] memory.
>>
>> Additionally, if the hardware is wedged,
>> idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
>> iterates over idxd->wqs[] and attempts to acquire mutex_lock(&wq->wq_lock)
>> on the freed memory.
> I took a look — idxd_shutdown() is only invoked by idxd_remove() and 
> during system shutdown/reboot.
> Is idxd_shutdown() ever reached from the idxd hardware error interrupt
> path?

The bot's suggestion that this code path could be hit by a hardware
error is false, but the shutdown path (like the user pressing a button)
could be valid.

A crash during that could cause the machine to not poweroff, which is
not good. Worth thinking about this, if there's something we could do
while we are here.

>>
>> Is there also a missing reference count for the cdev open path?
>>
>> Since idxd_unregister_devices() frees the wq structure, if user space
>> holds an open file descriptor, cdev_device_del() won't revoke it.
>> Can file operations like mmap or poll subsequently access the freed
>> ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
>> the parent wq device?
> To be honest, I'm not very familiar with the idxd driver. At this point,
> it looks like the idxd driver needs a state machine to ensure mutual 
> exclusion
> across the various concurrent paths.

Don't worry about this one, it's a separate issue.

>
> Best Regards,
> Guixin Liu
>>
>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>
>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>
>


Cheers,
-- 
Vinicius

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Guixin Liu]

在 2026/4/24 02:54, Vinicius Costa Gomes 写道:
> Guixin Liu <kanie@linux.alibaba.com> writes:
>
>> 在 2026/4/23 05:34, Vinicius Costa Gomes 写道:
>>> On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu <kanie@linux.alibaba.com> wrote:
>>>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>>>> index fb80803d5b57..c3cfd96074c9 100644
>>>> --- a/drivers/dma/idxd/init.c
>>>> +++ b/drivers/dma/idxd/init.c
>>>> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
>>>> [ ... skip 14 lines ... ]
>>>> +	 *
>>>> +	 * Use device_release_driver() to only unbind the driver (triggering
>>>> +	 * idxd_device_drv_remove()) without touching sysfs. Then safely
>>>> +	 * unregister children before the parent.
>>>> +	 */
>>>> +	device_release_driver(idxd_confdev(idxd));
>> Thanks for the reply.
>>> Below are sashiko comments verbatim, some notes:
>>>
>>>    - I do believe the race condition that it points out is real (but
>>>      very narrow), but not sure that we don't have a similar one in
>>>      current code (i.e. it wasn't introduced by your patch);
>>>    - The possible wq leak was definitely not introduced by this patch;
>>>    - The question about what happens with the file references after a
>>>      device is removed is a separate issue;
>>>
>>> The patch looks good, not opposed to it at all, the only one I am a
>>> bit concerned, about this patch, to be sure, is the race condition,
>>> and the two "paths" to reach it. (the others we can fix separately)
>>>
>>> sashiko.dev <sashiko@sashiko.dev>:
>>>
>>> Does this introduce a race condition with sysfs?
>>>
>>> Since the driver's sysfs nodes remain active after device_release_driver()
>>> is called, a user with sysfs privileges could write to the bind attribute
>>> to re-probe the driver before the device unregistration completes.
>>>
>>> If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
>>> idxd_unregister_devices(), does it cause a use-after-free when it
>>> iterates over the idxd->wqs[] objects being freed, since
>>> suppress_bind_attrs is not set?
>> I think re-probe will call idxd_pci_probe() to allocate an new idxd device,
>> this avoids accessing stale data.
> Yeah, I missed the (now obvious) that a new 'probe' also means a new
> device. So this "path" is not valid. Thank you.
>
>>> Also, does this code leak the workqueue?
>>>
>>> The workqueue idxd->wq is dynamically allocated using create_workqueue()
>>> during device initialization, but destroy_workqueue(idxd->wq) does not
>>> appear to be called in idxd_remove() or the sysfs device release callback.
>> Looks like it's b7cb9a034305 ("dmaengine: idxd: Fix refcount underflow
>> on module unload")
>> overlooked this, destory_workqueue(idxd->wq) is called in idxd_cleanup()
>> -> idxd_cleanup_internals().
>>
>> If so, I can send another patch to fix this.
> Yes, please do.
Will be added in v3.
>>>>    	idxd_unregister_devices(idxd);
>>> sashiko.dev <sashiko@sashiko.dev>:
>>>
>>> If idxd_unregister_devices() drops the last reference to the child wq
>>> objects and frees them, can idxd_shutdown() trigger a use-after-free
>>> when called shortly after?
>>>
>>> For example, if a hardware error interrupt fired during teardown,
>>> idxd_shutdown()->flush_workqueue() could execute an error handler that
>>> iterates over the freed idxd->wqs[] memory.
>>>
>>> Additionally, if the hardware is wedged,
>>> idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
>>> iterates over idxd->wqs[] and attempts to acquire mutex_lock(&wq->wq_lock)
>>> on the freed memory.
>> I took a look — idxd_shutdown() is only invoked by idxd_remove() and
>> during system shutdown/reboot.
>> Is idxd_shutdown() ever reached from the idxd hardware error interrupt
>> path?
> The bot's suggestion that this code path could be hit by a hardware
> error is false, but the shutdown path (like the user pressing a button)
> could be valid.
>
> A crash during that could cause the machine to not poweroff, which is
> not good. Worth thinking about this, if there's something we could do
> while we are here.

Both idxd_remove() and idxd_shutdown() are called under device_lock(dev) —
the lock is acquired by the driver core before invoking any bus callback:

 1.

    Remove path: device_release_driver_internal() →
    __device_driver_lock(dev, parent) → device_lock(dev) → ... →
    pci_device_remove() → idxd_remove()

 2.

    Shutdown path: device_shutdown() → device_lock(dev) →
    dev->bus->shutdown(dev) → pci_device_shutdown() → idxd_shutdown()

Since both paths acquire the same dev->mutex on the same PCI device,

idxd_remove() and idxd_shutdown() are mutually exclusive.

Best Regards,

Guixin Liu

>>> Is there also a missing reference count for the cdev open path?
>>>
>>> Since idxd_unregister_devices() frees the wq structure, if user space
>>> holds an open file descriptor, cdev_device_del() won't revoke it.
>>> Can file operations like mmap or poll subsequently access the freed
>>> ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
>>> the parent wq device?
>> To be honest, I'm not very familiar with the idxd driver. At this point,
>> it looks like the idxd driver needs a state machine to ensure mutual
>> exclusion
>> across the various concurrent paths.
> Don't worry about this one, it's a separate issue.
>
>> Best Regards,
>> Guixin Liu
>>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>
>>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>> via: https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>
>
> Cheers,

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Guixin Liu]

在 2026/4/24 13:43, Guixin Liu 写道:
>
>
> 在 2026/4/24 02:54, Vinicius Costa Gomes 写道:
>> Guixin Liu <kanie@linux.alibaba.com> writes:
>>
>>> 在 2026/4/23 05:34, Vinicius Costa Gomes 写道:
>>>> On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu 
>>>> <kanie@linux.alibaba.com> wrote:
>>>>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>>>>> index fb80803d5b57..c3cfd96074c9 100644
>>>>> --- a/drivers/dma/idxd/init.c
>>>>> +++ b/drivers/dma/idxd/init.c
>>>>> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev *pdev)
>>>>> [ ... skip 14 lines ... ]
>>>>> +     *
>>>>> +     * Use device_release_driver() to only unbind the driver 
>>>>> (triggering
>>>>> +     * idxd_device_drv_remove()) without touching sysfs. Then safely
>>>>> +     * unregister children before the parent.
>>>>> +     */
>>>>> +    device_release_driver(idxd_confdev(idxd));
>>> Thanks for the reply.
>>>> Below are sashiko comments verbatim, some notes:
>>>>
>>>>    - I do believe the race condition that it points out is real (but
>>>>      very narrow), but not sure that we don't have a similar one in
>>>>      current code (i.e. it wasn't introduced by your patch);
>>>>    - The possible wq leak was definitely not introduced by this patch;
>>>>    - The question about what happens with the file references after a
>>>>      device is removed is a separate issue;
>>>>
>>>> The patch looks good, not opposed to it at all, the only one I am a
>>>> bit concerned, about this patch, to be sure, is the race condition,
>>>> and the two "paths" to reach it. (the others we can fix separately)
>>>>
>>>> sashiko.dev <sashiko@sashiko.dev>:
>>>>
>>>> Does this introduce a race condition with sysfs?
>>>>
>>>> Since the driver's sysfs nodes remain active after 
>>>> device_release_driver()
>>>> is called, a user with sysfs privileges could write to the bind 
>>>> attribute
>>>> to re-probe the driver before the device unregistration completes.
>>>>
>>>> If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
>>>> idxd_unregister_devices(), does it cause a use-after-free when it
>>>> iterates over the idxd->wqs[] objects being freed, since
>>>> suppress_bind_attrs is not set?
>>> I think re-probe will call idxd_pci_probe() to allocate an new idxd 
>>> device,
>>> this avoids accessing stale data.
>> Yeah, I missed the (now obvious) that a new 'probe' also means a new
>> device. So this "path" is not valid. Thank you.
>>
>>>> Also, does this code leak the workqueue?
>>>>
>>>> The workqueue idxd->wq is dynamically allocated using 
>>>> create_workqueue()
>>>> during device initialization, but destroy_workqueue(idxd->wq) does not
>>>> appear to be called in idxd_remove() or the sysfs device release 
>>>> callback.
>>> Looks like it's b7cb9a034305 ("dmaengine: idxd: Fix refcount underflow
>>> on module unload")
>>> overlooked this, destory_workqueue(idxd->wq) is called in 
>>> idxd_cleanup()
>>> -> idxd_cleanup_internals().
>>>
>>> If so, I can send another patch to fix this.
>> Yes, please do.
> Will be added in v3.
Well, the idxd->wq is freed in idxd_conf_device_release(), so there is no
problem, no need to fix.
>>>>> idxd_unregister_devices(idxd);
>>>> sashiko.dev <sashiko@sashiko.dev>:
>>>>
>>>> If idxd_unregister_devices() drops the last reference to the child wq
>>>> objects and frees them, can idxd_shutdown() trigger a use-after-free
>>>> when called shortly after?
>>>>
>>>> For example, if a hardware error interrupt fired during teardown,
>>>> idxd_shutdown()->flush_workqueue() could execute an error handler that
>>>> iterates over the freed idxd->wqs[] memory.
>>>>
>>>> Additionally, if the hardware is wedged,
>>>> idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
>>>> iterates over idxd->wqs[] and attempts to acquire 
>>>> mutex_lock(&wq->wq_lock)
>>>> on the freed memory.
>>> I took a look — idxd_shutdown() is only invoked by idxd_remove() and
>>> during system shutdown/reboot.
>>> Is idxd_shutdown() ever reached from the idxd hardware error interrupt
>>> path?
>> The bot's suggestion that this code path could be hit by a hardware
>> error is false, but the shutdown path (like the user pressing a button)
>> could be valid.
>>
>> A crash during that could cause the machine to not poweroff, which is
>> not good. Worth thinking about this, if there's something we could do
>> while we are here.
>
> Both idxd_remove() and idxd_shutdown() are called under 
> device_lock(dev) —
> the lock is acquired by the driver core before invoking any bus callback:
>
> 1.
>
>    Remove path: device_release_driver_internal() →
>    __device_driver_lock(dev, parent) → device_lock(dev) → ... →
>    pci_device_remove() → idxd_remove()
>
> 2.
>
>    Shutdown path: device_shutdown() → device_lock(dev) →
>    dev->bus->shutdown(dev) → pci_device_shutdown() → idxd_shutdown()
>
> Since both paths acquire the same dev->mutex on the same PCI device,
>
> idxd_remove() and idxd_shutdown() are mutually exclusive.
>
> Best Regards,
>
> Guixin Liu
>
>>>> Is there also a missing reference count for the cdev open path?
>>>>
>>>> Since idxd_unregister_devices() frees the wq structure, if user space
>>>> holds an open file descriptor, cdev_device_del() won't revoke it.
>>>> Can file operations like mmap or poll subsequently access the freed
>>>> ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
>>>> the parent wq device?
>>> To be honest, I'm not very familiar with the idxd driver. At this 
>>> point,
>>> it looks like the idxd driver needs a state machine to ensure mutual
>>> exclusion
>>> across the various concurrent paths.
>> Don't worry about this one, it's a separate issue.
>>
>>> Best Regards,
>>> Guixin Liu
>>>> via: 
>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>> via: 
>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>
>>>> via: 
>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>> via: 
>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>
>>
>> Cheers,
>

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Guixin Liu]

Hi Vinicius,

It looks like all the issues from the AI Review have been clarified,
Can this patch be accepted?

Best Regards,
Guixin Liu

在 2026/4/24 13:56, Guixin Liu 写道:
>
>
> 在 2026/4/24 13:43, Guixin Liu 写道:
>>
>>
>> 在 2026/4/24 02:54, Vinicius Costa Gomes 写道:
>>> Guixin Liu <kanie@linux.alibaba.com> writes:
>>>
>>>> 在 2026/4/23 05:34, Vinicius Costa Gomes 写道:
>>>>> On Wed, 15 Apr 2026 17:50:30 +0800, Guixin Liu 
>>>>> <kanie@linux.alibaba.com> wrote:
>>>>>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>>>>>> index fb80803d5b57..c3cfd96074c9 100644
>>>>>> --- a/drivers/dma/idxd/init.c
>>>>>> +++ b/drivers/dma/idxd/init.c
>>>>>> @@ -1293,13 +1293,30 @@ static void idxd_remove(struct pci_dev 
>>>>>> *pdev)
>>>>>> [ ... skip 14 lines ... ]
>>>>>> +     *
>>>>>> +     * Use device_release_driver() to only unbind the driver 
>>>>>> (triggering
>>>>>> +     * idxd_device_drv_remove()) without touching sysfs. Then 
>>>>>> safely
>>>>>> +     * unregister children before the parent.
>>>>>> +     */
>>>>>> +    device_release_driver(idxd_confdev(idxd));
>>>> Thanks for the reply.
>>>>> Below are sashiko comments verbatim, some notes:
>>>>>
>>>>>    - I do believe the race condition that it points out is real (but
>>>>>      very narrow), but not sure that we don't have a similar one in
>>>>>      current code (i.e. it wasn't introduced by your patch);
>>>>>    - The possible wq leak was definitely not introduced by this 
>>>>> patch;
>>>>>    - The question about what happens with the file references after a
>>>>>      device is removed is a separate issue;
>>>>>
>>>>> The patch looks good, not opposed to it at all, the only one I am a
>>>>> bit concerned, about this patch, to be sure, is the race condition,
>>>>> and the two "paths" to reach it. (the others we can fix separately)
>>>>>
>>>>> sashiko.dev <sashiko@sashiko.dev>:
>>>>>
>>>>> Does this introduce a race condition with sysfs?
>>>>>
>>>>> Since the driver's sysfs nodes remain active after 
>>>>> device_release_driver()
>>>>> is called, a user with sysfs privileges could write to the bind 
>>>>> attribute
>>>>> to re-probe the driver before the device unregistration completes.
>>>>>
>>>>> If idxd_device_drv_probe()->idxd_wqs_setup() runs concurrently with
>>>>> idxd_unregister_devices(), does it cause a use-after-free when it
>>>>> iterates over the idxd->wqs[] objects being freed, since
>>>>> suppress_bind_attrs is not set?
>>>> I think re-probe will call idxd_pci_probe() to allocate an new idxd 
>>>> device,
>>>> this avoids accessing stale data.
>>> Yeah, I missed the (now obvious) that a new 'probe' also means a new
>>> device. So this "path" is not valid. Thank you.
>>>
>>>>> Also, does this code leak the workqueue?
>>>>>
>>>>> The workqueue idxd->wq is dynamically allocated using 
>>>>> create_workqueue()
>>>>> during device initialization, but destroy_workqueue(idxd->wq) does 
>>>>> not
>>>>> appear to be called in idxd_remove() or the sysfs device release 
>>>>> callback.
>>>> Looks like it's b7cb9a034305 ("dmaengine: idxd: Fix refcount underflow
>>>> on module unload")
>>>> overlooked this, destory_workqueue(idxd->wq) is called in 
>>>> idxd_cleanup()
>>>> -> idxd_cleanup_internals().
>>>>
>>>> If so, I can send another patch to fix this.
>>> Yes, please do.
>> Will be added in v3.
> Well, the idxd->wq is freed in idxd_conf_device_release(), so there is no
> problem, no need to fix.
>>>>>> idxd_unregister_devices(idxd);
>>>>> sashiko.dev <sashiko@sashiko.dev>:
>>>>>
>>>>> If idxd_unregister_devices() drops the last reference to the child wq
>>>>> objects and frees them, can idxd_shutdown() trigger a use-after-free
>>>>> when called shortly after?
>>>>>
>>>>> For example, if a hardware error interrupt fired during teardown,
>>>>> idxd_shutdown()->flush_workqueue() could execute an error handler 
>>>>> that
>>>>> iterates over the freed idxd->wqs[] memory.
>>>>>
>>>>> Additionally, if the hardware is wedged,
>>>>> idxd_shutdown()->idxd_device_disable()->idxd_device_clear_state()
>>>>> iterates over idxd->wqs[] and attempts to acquire 
>>>>> mutex_lock(&wq->wq_lock)
>>>>> on the freed memory.
>>>> I took a look — idxd_shutdown() is only invoked by idxd_remove() and
>>>> during system shutdown/reboot.
>>>> Is idxd_shutdown() ever reached from the idxd hardware error interrupt
>>>> path?
>>> The bot's suggestion that this code path could be hit by a hardware
>>> error is false, but the shutdown path (like the user pressing a button)
>>> could be valid.
>>>
>>> A crash during that could cause the machine to not poweroff, which is
>>> not good. Worth thinking about this, if there's something we could do
>>> while we are here.
>>
>> Both idxd_remove() and idxd_shutdown() are called under 
>> device_lock(dev) —
>> the lock is acquired by the driver core before invoking any bus 
>> callback:
>>
>> 1.
>>
>>    Remove path: device_release_driver_internal() →
>>    __device_driver_lock(dev, parent) → device_lock(dev) → ... →
>>    pci_device_remove() → idxd_remove()
>>
>> 2.
>>
>>    Shutdown path: device_shutdown() → device_lock(dev) →
>>    dev->bus->shutdown(dev) → pci_device_shutdown() → idxd_shutdown()
>>
>> Since both paths acquire the same dev->mutex on the same PCI device,
>>
>> idxd_remove() and idxd_shutdown() are mutually exclusive.
>>
>> Best Regards,
>>
>> Guixin Liu
>>
>>>>> Is there also a missing reference count for the cdev open path?
>>>>>
>>>>> Since idxd_unregister_devices() frees the wq structure, if user space
>>>>> holds an open file descriptor, cdev_device_del() won't revoke it.
>>>>> Can file operations like mmap or poll subsequently access the freed
>>>>> ctx->wq pointer? Should idxd_cdev_open() take a kobject reference on
>>>>> the parent wq device?
>>>> To be honest, I'm not very familiar with the idxd driver. At this 
>>>> point,
>>>> it looks like the idxd driver needs a state machine to ensure mutual
>>>> exclusion
>>>> across the various concurrent paths.
>>> Don't worry about this one, it's a separate issue.
>>>
>>>> Best Regards,
>>>> Guixin Liu
>>>>> via: 
>>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>> via: 
>>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>>
>>>>> via: 
>>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>> via: 
>>>>> https://sashiko.dev/#/message/20260415095030.42183-1-kanie@linux.alibaba.com
>>>>>
>>>
>>> Cheers,
>>

Re: [PATCH v2] dmaengine: idxd: Fix use-after-free of idxd_wq

[Vinicius Costa Gomes]

Guixin Liu <kanie@linux.alibaba.com> writes:

> We found an idxd_wq use-after-free issue with kasan
> when remove the idxd PCI device:
>
> BUG: KASAN: slab-use-after-free in idxd_device_drv_remove+0x1f8/0x240 [idxd]
> Call Trace:
>   <TASK>
>   dump_stack_lvl+0x32/0x50
>   print_address_description.constprop.0+0x2c/0x390
>   ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
>   print_report+0xba/0x280
>   ? kasan_addr_to_slab+0x9/0xa0
>   ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
>   kasan_report+0xab/0xe0
>   ? idxd_device_drv_remove+0x1f8/0x240 [idxd]
>   idxd_device_drv_remove+0x1f8/0x240 [idxd]
>   device_release_driver_internal+0x391/0x560
>   bus_remove_device+0x1f5/0x3f0
>   device_del+0x392/0x990
>   ? __pfx_device_del+0x10/0x10
>   ? kobject_cleanup+0x117/0x360
>   ? idxd_unregister_devices+0x229/0x320 [idxd]
>   device_unregister+0x13/0xa0
>   idxd_remove+0x4f/0x1b0 [idxd]
>   pci_device_remove+0xa7/0x1d0
>   device_release_driver_internal+0x391/0x560
>   ? pci_pme_active+0x1e/0x450
>   pci_stop_bus_device+0x10a/0x150
>   pci_stop_and_remove_bus_device_locked+0x16/0x30
>   remove_store+0xcf/0xe0
>
> Freed by task 15535:
>   kasan_save_stack+0x1c/0x40
>   kasan_set_track+0x21/0x30
>   kasan_save_free_info+0x27/0x40
>   ____kasan_slab_free+0x171/0x240
>   slab_free_freelist_hook+0xde/0x190
>   __kmem_cache_free+0x19e/0x310
>   device_release+0x98/0x210
>   kobject_cleanup+0x102/0x360
>   idxd_unregister_devices+0xb3/0x320 [idxd]
>   dxd_remove+0x3f/0x1b0 [idxd]
>   pci_device_remove+0xa7/0x1d0
>   device_release_driver_internal+0x391/0x560
>   pci_stop_bus_device+0x10a/0x150
>   pci_stop_and_remove_bus_device_locked+0x16/0x30
>   remove_store+0xcf/0xe0
>
> In the idxd_remove() flow, when execution reaches
> idxd_unregister_devices(), all idxd_wq instances have already been
> freed. Subsequently, when device_unregister(idxd_confdev(idxd)) is
> executed, it calls into idxd_device_drv_remove() which accesses the
> already-freed idxd_wq. This fix resolves the issue by calling
> device_release_driver() before idxd_unregister_devices().
>
> Fixes: 98da0106aac0d ("dmanegine: idxd: fix resource free ordering on driver removal")
> Co-developed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---

All questions that I had after the AI review are handled:

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius