[PATCH] dmaengine: idxd: Avoid destroying the device workqueue twice

[PATCH] dmaengine: idxd: Avoid destroying the device workqueue twice

[Yao Kai]

If idxd_probe() fails after idxd_setup_internals() succeeds,
idxd_cleanup_internals() destroys idxd->wq. The PCI probe error path
then calls idxd_free(), whose put_device() invokes
idxd_conf_device_release() and destroys the same workqueue again.
The err_evl path in idxd_setup_internals() has the same problem.
This can corrupt the workqueue list or access freed memory.

Clear idxd->wq after both error-path destroy_workqueue() calls and
only destroy a non-NULL workqueue in the release callback. The NULL
check also handles failures before the workqueue is created, while
preserving its destruction on normal device removal.

Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
Cc: stable@vger.kernel.org
Signed-off-by: Yao Kai <yaokai34@huawei.com>
---
 drivers/dma/idxd/init.c  | 2 ++
 drivers/dma/idxd/sysfs.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 4b827a329756..ffbc5a74be6e 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -389,6 +389,7 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
 	idxd_clean_engines(idxd);
 	idxd_clean_wqs(idxd);
 	destroy_workqueue(idxd->wq);
+	idxd->wq = NULL;
 }
 
 static int idxd_init_evl(struct idxd_device *idxd)
@@ -460,6 +461,7 @@ static int idxd_setup_internals(struct idxd_device *idxd)
 
  err_evl:
 	destroy_workqueue(idxd->wq);
+	idxd->wq = NULL;
  err_wkq_create:
 	idxd_clean_groups(idxd);
  err_group:
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 6d251095c350..d5ffc641c856 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
 {
 	struct idxd_device *idxd = confdev_to_idxd(dev);
 
-	destroy_workqueue(idxd->wq);
+	if (idxd->wq)
+		destroy_workqueue(idxd->wq);
 	kfree(idxd->groups);
 	bitmap_free(idxd->wq_enable_map);
 	kfree(idxd->wqs);
-- 
2.43.0

Re: [PATCH] dmaengine: idxd: Avoid destroying the device workqueue twice

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] dmaengine: idxd: use-after-free and double free in
  idxd_alloc() [1]
--

commit 8d423d27ba8372d572adfd4d4096b791e0e07bdf
Author: Yao Kai <yaokai34@huawei.com>

dmaengine: idxd: Avoid destroying the device workqueue twice

This commit clears idxd->wq after destroying the workqueue on error paths
and guards destroy_workqueue() in idxd_conf_device_release() with a NULL
check so that the workqueue is not destroyed twice.

> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6d251095c350f..d5ffc641c8567 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
>  {
>  	struct idxd_device *idxd = confdev_to_idxd(dev);
>  
> -	destroy_workqueue(idxd->wq);
> +	if (idxd->wq)
> +		destroy_workqueue(idxd->wq);
>  	kfree(idxd->groups);
>  	bitmap_free(idxd->wq_enable_map);
>  	kfree(idxd->wqs);

[Severity: Critical]
This problem wasn't introduced by this patch, but can the err_name path in
idxd_alloc() cause a use-after-free and double free [1] of idxd,
idxd->opcap_bmap, and idxd->id?

drivers/dma/idxd/init.c:idxd_alloc() {
    ...
	device_initialize(conf_dev);
	conf_dev->parent = dev;
	conf_dev->bus = &dsa_bus_type;
	conf_dev->type = idxd->data->dev_type;
	rc = dev_set_name(conf_dev, "%s%d", idxd->data->name_prefix, idxd->id);
	if (rc < 0)
		goto err_name;
    ...
 err_name:
	put_device(conf_dev);
	bitmap_free(idxd->opcap_bmap);
 err_opcap:
	ida_free(&idxd_ida, idxd->id);
 err_ida:
	kfree(idxd);
    ...
}

When dev_set_name() fails, conf_dev->type is already set, so
put_device(conf_dev) invokes idxd_conf_device_release(), which frees
idxd->id, idxd->opcap_bmap, and idxd.  The fallthrough to err_opcap and
err_ida then reads from the freed idxd struct and frees all three a second
time.

[1] https://sashiko.dev/#/bug/linux-4811d59b-3028-4d67-8ce7-e0e9de64774f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111021026.3354620-1-yaokai34@huawei.com?part=1