[PATCH 0/6] dmaengine: idxd: More error handling fixes

[PATCH 0/6] dmaengine: idxd: More error handling fixes

[Vinicius Costa Gomes]

Mostly this series proposes fixes for issues in the error handling
paths of the idxd driver. Patches 1/6, 2/6 and 3/6 are for issues that
have been shown to happen in the real world.

Patch 6/6 is an exception, in the sense that it's not error path
related, it's an issue that I introduced (my scripts were removing
the idxd module after each dmatest run, so I didn't caught this).

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
Steve Wahl (2):
      dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq
      dmaengine: idxd: fix duplicate memory frees on initialization error path

Vinicius Costa Gomes (4):
      dmaengine: idxd: Fix potential double free on probe error path
      dmaengine: idxd: Fix unwind bounds in device registration error paths
      dmaengine: idxd: Fix error code handling in idxd_cdev_open()
      dmaengine: idxd: Do not release interrupt handles on terminate_all

 drivers/dma/idxd/cdev.c   |  8 ++++++--
 drivers/dma/idxd/device.c | 11 ++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 drivers/dma/idxd/init.c   | 21 ++++++---------------
 drivers/dma/idxd/irq.c    |  1 +
 drivers/dma/idxd/sysfs.c  |  9 +++++----
 6 files changed, 29 insertions(+), 22 deletions(-)
---
base-commit: 42c31fec1ca76b8b5f7759def122d418dff2cd4b
change-id: 20260803-wt-idxd-fixes-1a06bb69b1e0

Best regards,
--  
Vinicius

[PATCH 1/6] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[Vinicius Costa Gomes]

From: Steve Wahl <steve.wahl@hpe.com>

Error paths within idxd_pci_probe_alloc and related functions end up
calling destroy_workqueue with a null pointer, from
idxd_conf_device_release via put_device, because that allocation has
not yet occurred when the error is hit.

This was encountered running in a kexec'd kdump kernel with reduced
resources, causing the "Device is HALTED!" branch in
idxd_device_init_reset to be taken.

In idxd_conf_device_release, check that the workqueue has been
allocated before trying to destroy it.

Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/sysfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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.56.0

Re: [PATCH 1/6] dmaengine: idxd: Do not call destroy_workqueue with null idxd->wq

[Frank Li]

On Mon, Oct 12, 2026 at 02:38:04PM -0700, Vinicius Costa Gomes wrote:
> From: Steve Wahl <steve.wahl@hpe.com>
>
> Error paths within idxd_pci_probe_alloc and related functions end up
> calling destroy_workqueue with a null pointer, from

all funciton need (), include subject.

destroy_workqueue()

Frank

> idxd_conf_device_release via put_device, because that allocation has
> not yet occurred when the error is hit.
>
> This was encountered running in a kexec'd kdump kernel with reduced
> resources, causing the "Device is HALTED!" branch in
> idxd_device_init_reset to be taken.
>
> In idxd_conf_device_release, check that the workqueue has been
> allocated before trying to destroy it.
>
> Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> ---
>  drivers/dma/idxd/sysfs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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.56.0
>

[PATCH 2/6] dmaengine: idxd: fix duplicate memory frees on initialization error path

[Vinicius Costa Gomes]

From: Steve Wahl <steve.wahl@hpe.com>

Error paths within idxd_pci_probe_alloc and related functions end up
attempting to free memory already freed from idxd_conf_device_release
via put_device.

This was encountered running in a kexec'd kdump kernel with reduced
resources, causing the "Device is HALTED!" branch in
idxd_device_init_reset to be taken.

[   18.628791] idxd 0000:00:01.0: Device is HALTED!
[   18.631447] idxd 0000:00:01.0: Intel(R) IDXD DMA Engine init failed
[   18.631450] ------------[ cut here ]------------
[   18.631451] ida_free called for id=0 which is not allocated.
[   18.631462] WARNING: CPU: 0 PID: 11 at lib/idr.c:525 ida_free+0xd3/0x130
[   18.631474] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Not tainted 6.12.0-1-rt-amd64 #1
[   18.631477] Hardware name: Dell Inc. PowerEdge XR8720t/0J91KV, BIOS 1.1.3 02/03/2026
[   18.631478] Workqueue: events work_for_cpu_fn
[   18.631480] RIP: 0010:ida_free+0xd3/0x130
[   18.631492] Call Trace:
[   18.631494]  <TASK>
[   18.631495]  idxd_pci_probe+0x1b0/0x1860 [idxd]
[   18.631506]  local_pci_probe+0x43/0xa0
[   18.631508]  work_for_cpu_fn+0x13/0x20
[   18.631510]  process_one_work+0x179/0x390
[   18.631512]  worker_thread+0x237/0x340
[   18.631517]  kthread+0xc6/0x100
[   18.631520]  ret_from_fork+0x2d/0x50
[   18.631524]  ret_from_fork_asm+0x1a/0x30
[   18.631526]  </TASK>

In idxd_free and idxd_alloc, do not attempt to free allocations that
will already have been freed.

Fixes: 90022b3a6981 ("dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe")
Fixes: 46a5cca76c76 ("dmaengine: idxd: fix memory leak in error handling path of idxd_alloc")
Reported-by: Bogdan Codres <bogdan.codres@windriver.com>
Link: https://lore.kernel.org/all/akLBL7b2h73pK0AC@ilogsc-qemu.local/
Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
---
 drivers/dma/idxd/init.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 4b827a329756..4adb3f3bcd01 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -581,9 +581,6 @@ static void idxd_free(struct idxd_device *idxd)
 		return;
 
 	put_device(idxd_confdev(idxd));
-	bitmap_free(idxd->opcap_bmap);
-	ida_free(&idxd_ida, idxd->id);
-	kfree(idxd);
 }
 
 static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
@@ -623,8 +620,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
 	return idxd;
 
 err_name:
+	/*
+	 * once device_initialize(conf_dev) is called,
+	 * put_device(conf_dev) will end up calling
+	 * idxd_conf_device_release() which will free the rest.
+	 */
 	put_device(conf_dev);
-	bitmap_free(idxd->opcap_bmap);
+	return NULL;
 err_opcap:
 	ida_free(&idxd_ida, idxd->id);
 err_ida:

-- 
2.56.0

Re: [PATCH 2/6] dmaengine: idxd: fix duplicate memory frees on initialization error path

[Frank Li]

On Mon, Oct 12, 2026 at 02:38:05PM -0700, Vinicius Costa Gomes wrote:
> From: Steve Wahl <steve.wahl@hpe.com>
>
> Error paths within idxd_pci_probe_alloc and related functions end

idxd_pci_probe_alloc(), check other commit message.

Frank

> attempting to free memory already freed from idxd_conf_device_release
> via put_device.
>
> This was encountered running in a kexec'd kdump kernel with reduced
> resources, causing the "Device is HALTED!" branch in
> idxd_device_init_reset to be taken.
>
> [   18.628791] idxd 0000:00:01.0: Device is HALTED!
> [   18.631447] idxd 0000:00:01.0: Intel(R) IDXD DMA Engine init failed
> [   18.631450] ------------[ cut here ]------------
> [   18.631451] ida_free called for id=0 which is not allocated.
> [   18.631462] WARNING: CPU: 0 PID: 11 at lib/idr.c:525 ida_free+0xd3/0x130
> [   18.631474] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Not tainted 6.12.0-1-rt-amd64 #1
> [   18.631477] Hardware name: Dell Inc. PowerEdge XR8720t/0J91KV, BIOS 1.1.3 02/03/2026
> [   18.631478] Workqueue: events work_for_cpu_fn
> [   18.631480] RIP: 0010:ida_free+0xd3/0x130
> [   18.631492] Call Trace:
> [   18.631494]  <TASK>
> [   18.631495]  idxd_pci_probe+0x1b0/0x1860 [idxd]
> [   18.631506]  local_pci_probe+0x43/0xa0
> [   18.631508]  work_for_cpu_fn+0x13/0x20
> [   18.631510]  process_one_work+0x179/0x390
> [   18.631512]  worker_thread+0x237/0x340
> [   18.631517]  kthread+0xc6/0x100
> [   18.631520]  ret_from_fork+0x2d/0x50
> [   18.631524]  ret_from_fork_asm+0x1a/0x30
> [   18.631526]  </TASK>
>
> In idxd_free and idxd_alloc, do not attempt to free allocations that
> will already have been freed.
>
> Fixes: 90022b3a6981 ("dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe")
> Fixes: 46a5cca76c76 ("dmaengine: idxd: fix memory leak in error handling path of idxd_alloc")
> Reported-by: Bogdan Codres <bogdan.codres@windriver.com>
> Link: https://lore.kernel.org/all/akLBL7b2h73pK0AC@ilogsc-qemu.local/
> Signed-off-by: Steve Wahl <steve.wahl@hpe.com>
> ---
>  drivers/dma/idxd/init.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 4b827a329756..4adb3f3bcd01 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -581,9 +581,6 @@ static void idxd_free(struct idxd_device *idxd)
>  		return;
>
>  	put_device(idxd_confdev(idxd));
> -	bitmap_free(idxd->opcap_bmap);
> -	ida_free(&idxd_ida, idxd->id);
> -	kfree(idxd);
>  }
>
>  static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
> @@ -623,8 +620,13 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
>  	return idxd;
>
>  err_name:
> +	/*
> +	 * once device_initialize(conf_dev) is called,
> +	 * put_device(conf_dev) will end up calling
> +	 * idxd_conf_device_release() which will free the rest.
> +	 */
>  	put_device(conf_dev);
> -	bitmap_free(idxd->opcap_bmap);
> +	return NULL;
>  err_opcap:
>  	ida_free(&idxd_ida, idxd->id);
>  err_ida:
>
> --
> 2.56.0
>

[PATCH 3/6] dmaengine: idxd: Fix potential double free on probe error path

[Vinicius Costa Gomes]

On the probe error path the device is released via put_device() ->
idxd_conf_device_release(), which frees idxd->wq and the idxd->wqs,
idxd->engines, idxd->groups and idxd->wq_enable_map arrays. The
idxd_clean_*() helpers and the idxd_setup_*() error unwind paths free
the same objects, so they get freed twice.

With commit ec2d428b2e32 ("dmaengine: idxd: fix double free of wq,
engine, and group structs") we made use of the kobject lifetime to
keep track of device memory allocations. These deallocations were left
behind. Remove them as idxd_conf_device_release() is already taking
care of that.

Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
Link: https://lore.kernel.org/all/20260615103932.61828-1-bogdan.codres@windriver.com/
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/init.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 4adb3f3bcd01..8cf478c5b26d 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -166,8 +166,6 @@ static void idxd_clean_wqs(struct idxd_device *idxd)
 		conf_dev = wq_confdev(idxd->wqs[i]);
 		put_device(conf_dev);
 	}
-	bitmap_free(idxd->wq_enable_map);
-	kfree(idxd->wqs);
 }
 
 static int idxd_setup_wqs(struct idxd_device *idxd)
@@ -245,11 +243,8 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		conf_dev = wq_confdev(idxd->wqs[i]);
 		put_device(conf_dev);
 	}
-	bitmap_free(idxd->wq_enable_map);
 
 err_free_wqs:
-	kfree(idxd->wqs);
-
 	return rc;
 }
 
@@ -262,7 +257,6 @@ static void idxd_clean_engines(struct idxd_device *idxd)
 		conf_dev = engine_confdev(idxd->engines[i]);
 		put_device(conf_dev);
 	}
-	kfree(idxd->engines);
 }
 
 static int idxd_setup_engines(struct idxd_device *idxd)
@@ -308,7 +302,6 @@ static int idxd_setup_engines(struct idxd_device *idxd)
 		conf_dev = engine_confdev(idxd->engines[i]);
 		put_device(conf_dev);
 	}
-	kfree(idxd->engines);
 
 	return rc;
 }
@@ -320,7 +313,6 @@ static void idxd_clean_groups(struct idxd_device *idxd)
 	for (i = 0; i < idxd->max_groups; i++) {
 		put_device(group_confdev(idxd->groups[i]));
 	}
-	kfree(idxd->groups);
 }
 
 static int idxd_setup_groups(struct idxd_device *idxd)
@@ -378,7 +370,6 @@ static int idxd_setup_groups(struct idxd_device *idxd)
 		group = idxd->groups[i];
 		put_device(group_confdev(group));
 	}
-	kfree(idxd->groups);
 
 	return rc;
 }
@@ -388,7 +379,6 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
 	idxd_clean_groups(idxd);
 	idxd_clean_engines(idxd);
 	idxd_clean_wqs(idxd);
-	destroy_workqueue(idxd->wq);
 }
 
 static int idxd_init_evl(struct idxd_device *idxd)
@@ -459,7 +449,6 @@ static int idxd_setup_internals(struct idxd_device *idxd)
 	return 0;
 
  err_evl:
-	destroy_workqueue(idxd->wq);
  err_wkq_create:
 	idxd_clean_groups(idxd);
  err_group:

-- 
2.56.0

[PATCH 4/6] dmaengine: idxd: Fix unwind bounds in device registration error paths

[Vinicius Costa Gomes]

When registering the per-idxd wq, engine, or group devices, a failure
partway through must roll back exactly the devices already added, i.e.
the range [0, i) when device_add() fails at index i.

idxd_register_{wq,engine,group}_devices() get this range wrong: each
seeds the "while (j--)" rollback loop with j = i - 1 instead of j = i.
This skips one device (leaking it), and when the very first device
fails (i == 0) the index underflows to -1 and the loop walks off the
front of the array.

Start the rollback at i.

Fixes: 7c5dd23e57c1 ("dmaengine: idxd: fix wq conf_dev 'struct device' lifetime")
Fixes: 75b911309060 ("dmaengine: idxd: fix engine conf_dev lifetime")
Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/sysfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index d5ffc641c856..e828292e5fb8 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1876,7 +1876,7 @@ static int idxd_register_engine_devices(struct idxd_device *idxd)
 	return 0;
 
 cleanup:
-	j = i - 1;
+	j = i;
 	for (; i < idxd->max_engines; i++) {
 		engine = idxd->engines[i];
 		put_device(engine_confdev(engine));
@@ -1904,7 +1904,7 @@ static int idxd_register_group_devices(struct idxd_device *idxd)
 	return 0;
 
 cleanup:
-	j = i - 1;
+	j = i;
 	for (; i < idxd->max_groups; i++) {
 		group = idxd->groups[i];
 		put_device(group_confdev(group));
@@ -1932,7 +1932,7 @@ static int idxd_register_wq_devices(struct idxd_device *idxd)
 	return 0;
 
 cleanup:
-	j = i - 1;
+	j = i;
 	for (; i < idxd->max_wqs; i++) {
 		wq = idxd->wqs[i];
 		put_device(wq_confdev(wq));

-- 
2.56.0

Re: [PATCH 4/6] dmaengine: idxd: Fix unwind bounds in device registration error paths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: idxd: UAF/double-free on device registration failure
--

commit 571be93356be3fdc63d0506588c4e560e5d9eccc
Author: Vinicius Costa Gomes <vinicius.gomes@intel.com>

dmaengine: idxd: Fix unwind bounds in device registration error paths

When registering the per-idxd wq, engine, or group devices, a failure partway
through must roll back exactly the devices already added. This commit fixes
an underflow loop by starting the rollback at i instead of i - 1.

> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index d5ffc641c8567..e828292e5fb88 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1876,7 +1876,7 @@ static int idxd_register_engine_devices(struct idxd_device *idxd)
>  	return 0;
>  
>  cleanup:
> -	j = i - 1;
> +	j = i;
>  	for (; i < idxd->max_engines; i++) {
>  		engine = idxd->engines[i];
>  		put_device(engine_confdev(engine));

[Severity: High]
Does this error handling sequence lead to a double-free of the child devices?

If device registration fails partway, this local cleanup path drops the refcounts
on the child device kobjects via put_device() and device_unregister(), which
frees them.

The failure then cascades up to idxd_register_devices(), which rolls back
previously registered child devices (like WQs), also freeing them. Crucially,
the pointers in idxd->engines and idxd->wqs are left dangling.

Then, the error is returned to idxd_pci_probe_alloc(), which unwinds by jumping
to err_dev_register and explicitly calling idxd_cleanup():

drivers/dma/idxd/init.c:idxd_pci_probe_alloc() {
    ...
	rc = idxd_register_devices(idxd);
	if (rc) {
		dev_err(dev, "IDXD sysfs setup failed\n");
		goto err_dev_register;
	}
    ...
 err_dev_register:
	idxd_cleanup(idxd);
    ...
}

idxd_cleanup() invokes idxd_cleanup_internals(), which iterates over the arrays
via idxd_clean_engines() and calls put_device() again on the dangling pointers:

drivers/dma/idxd/init.c:idxd_clean_engines() {
	for (i = 0; i < idxd->max_engines; i++) {
		conf_dev = engine_confdev(idxd->engines[i]);
		put_device(conf_dev);
	}
}

Could this secondary cleanup access the freed device pointer and execute a
double-put, causing a use-after-free?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-wt-idxd-fixes-v1-0-30a35a445077@intel.com?part=4

[PATCH 5/6] dmaengine: idxd: Fix error code handling in idxd_cdev_open()

[Vinicius Costa Gomes]

idxd_cdev_open() mishandles rc on two error paths:

  - On ida_alloc() failure it jumps to failed_ida without setting rc,
    returning the leftover 0 from the earlier successful xa_insert(), so
    open() falsely reports success.

  - When a PASID entry already exists, xa_insert() returns -EBUSY.  This
    is non-fatal and only warned about, but rc is left negative and can
    leak out for a shared wq (which skips the idxd_wq_set_pasid() branch
    that would reset it).

Set rc to the ida_alloc() error before unwinding, and reset rc to 0
after the "PASID already exists" (Fixed the "exist -> exists" typo
while at it) warning.

Fixes: e6fd6d7e5f0f ("dmaengine: idxd: add a device to represent the file opened")
Fixes: b022f59725f0 ("dmaengine: idxd: add idxd_copy_cr() to copy user completion record during page fault handling")
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/cdev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
index 82b07cf942ef..abbab9bdd06c 100644
--- a/drivers/dma/idxd/cdev.c
+++ b/drivers/dma/idxd/cdev.c
@@ -264,8 +264,11 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp)
 		mutex_lock(&wq->uc_lock);
 		rc = xa_insert(&wq->upasid_xa, pasid, ctx, GFP_KERNEL);
 		mutex_unlock(&wq->uc_lock);
-		if (rc < 0)
-			dev_warn(dev, "PASID entry already exist in xarray.\n");
+		if (rc < 0) {
+			/* An existing entry is harmless, warn and continue */
+			dev_warn(dev, "PASID entry already exists in xarray.\n");
+			rc = 0;
+		}
 
 		if (wq_dedicated(wq)) {
 			rc = idxd_wq_set_pasid(wq, pasid);
@@ -279,6 +282,7 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp)
 	idxd_cdev = wq->idxd_cdev;
 	ctx->id = ida_alloc(&file_ida, GFP_KERNEL);
 	if (ctx->id < 0) {
+		rc = ctx->id;
 		dev_warn(dev, "ida alloc failure\n");
 		goto failed_ida;
 	}

-- 
2.56.0

Re: [PATCH 5/6] dmaengine: idxd: Fix error code handling in idxd_cdev_open()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dmaengine: idxd: idxd_cdev_open() masks -ENOMEM from xa_insert()
- [High] dmaengine: stranded PASID xarray entry on shared FD close breaks fault handling
- [Medium] dmaengine: idxd: missing idxd_wq_disable_pasid() rollback on ida_alloc() failure
--

commit 63ea0ebbf579eb846dc4486575508a710df270fc
Author: Vinicius Costa Gomes <vinicius.gomes@intel.com>

dmaengine: idxd: Fix error code handling in idxd_cdev_open()

idxd_cdev_open() mishandles rc on two error paths: on ida_alloc() failure,
it jumps to failed_ida without setting rc, returning 0. And when xa_insert()
returns -EBUSY, rc is left negative and can leak out. This commit fixes
both paths.

> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index 82b07cf942ef8..abbab9bdd06c3 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -264,8 +264,11 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp)
>  		mutex_lock(&wq->uc_lock);
>  		rc = xa_insert(&wq->upasid_xa, pasid, ctx, GFP_KERNEL);
>  		mutex_unlock(&wq->uc_lock);
> -		if (rc < 0)
> -			dev_warn(dev, "PASID entry already exist in xarray.\n");
> +		if (rc < 0) {
> +			/* An existing entry is harmless, warn and continue */
> +			dev_warn(dev, "PASID entry already exists in xarray.\n");
> +			rc = 0;
> +		}

[Severity: High]
Does this code unintentionally mask all negative return codes, including
-ENOMEM?

xa_insert() can return -ENOMEM if memory allocation fails. By unconditionally
clearing all negative return codes (rc = 0), this allows the file open to
succeed without registering the user context in the xarray.

Later, when a subsequent hardware page fault occurs for this PASID,
idxd_copy_cr() will call xa_load(), fail to find the context, and silently
drop the completion record.

[Severity: High]
If a process opens multiple file descriptors to a shared IDXD workqueue,
does treating -EBUSY as harmless leave the second FD stranded when the first
FD is closed?

When the first open succeeds, the context is inserted into the upasid_xa
xarray. The second open fails the xa_insert with -EBUSY, which this code
explicitly treats as harmless and continues, leaving the first context in
the xarray.

However, when the process closes the first file descriptor,
idxd_xa_pasid_remove() successfully matches and removes the first context
from the xarray:

drivers/dma/idxd/cdev.c:idxd_xa_pasid_remove() {
	ptr = xa_cmpxchg(&wq->upasid_xa, ctx->pasid, ctx, NULL, GFP_KERNEL);
}

The second file descriptor remains open and active, but its PASID is no
longer mapped in the xarray. If a hardware fault occurs for this PASID,
idxd_copy_cr() will fail to find the context in the xarray, warn
'No user context', and silently drop the completion record.

>  
>  		if (wq_dedicated(wq)) {
>  			rc = idxd_wq_set_pasid(wq, pasid);
> @@ -279,6 +282,7 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp)
>  	idxd_cdev = wq->idxd_cdev;
>  	ctx->id = ida_alloc(&file_ida, GFP_KERNEL);
>  	if (ctx->id < 0) {
> +		rc = ctx->id;
>  		dev_warn(dev, "ida alloc failure\n");
>  		goto failed_ida;
>  	}

[Severity: Medium]
Does the failed_ida error path leak the PASID hardware configuration?

During open(), if the WQ is dedicated, idxd_wq_set_pasid() programs the
hardware WQ with the calling process's PASID. If the subsequent ida_alloc()
fails, the error path (failed_ida) unbinds the PASID from the IOMMU and
removes it from the xarray:

drivers/dma/idxd/cdev.c:idxd_cdev_open() {
    ...
failed_ida:
failed_set_pasid:
	if (device_user_pasid_enabled(idxd))
		idxd_xa_pasid_remove(ctx);
failed_get_pasid:
	if (device_user_pasid_enabled(idxd) && !IS_ERR_OR_NULL(sva))
		iommu_sva_unbind_device(sva);
    ...
}

However, it fails to call idxd_wq_disable_pasid() to clear it from the
hardware WQ. This leaves the hardware WQ in the ENABLED state programmed
with a stale, unbound PASID.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-wt-idxd-fixes-v1-0-30a35a445077@intel.com?part=5

[PATCH 6/6] dmaengine: idxd: Do not release interrupt handles on terminate_all

[Vinicius Costa Gomes]

idxd_wq_flush_descs() is called from the dmaengine .device_terminate_all
path, but besides flushing the pending descriptors it also releases the
interrupt handle. As the workqueue stays enabled, the handle is never
re-armed, and any subsequent submission uses an invalid handle and never
completes.

This only shows up when the same workqueue is used more than once: the
first run leaves the handle released on terminate_all, so any later run
times out on every descriptor. dmatest reproduces it by running twice.

Split the interrupt handle teardown into idxd_wq_release_int_handle() so
that terminate_all only flushes descriptors. The FLR path still needs
both, so call them together there.

Fixes: 2a93f5747d0e ("dmaengine: idxd: Flush all pending descriptors")
Reported-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 drivers/dma/idxd/device.c | 11 ++++++++++-
 drivers/dma/idxd/idxd.h   |  1 +
 drivers/dma/idxd/irq.c    |  1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 131138483b87..06299f40f531 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -1349,6 +1349,16 @@ void idxd_wq_free_irq(struct idxd_wq *wq)
 }
 
 void idxd_wq_flush_descs(struct idxd_wq *wq)
+{
+	guard(mutex)(&wq->wq_lock);
+
+	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
+		return;
+
+	idxd_flush_pending_descs(&wq->ie);
+}
+
+void idxd_wq_release_int_handle(struct idxd_wq *wq)
 {
 	struct idxd_irq_entry *ie = &wq->ie;
 	struct idxd_device *idxd = wq->idxd;
@@ -1358,7 +1368,6 @@ void idxd_wq_flush_descs(struct idxd_wq *wq)
 	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
 		return;
 
-	idxd_flush_pending_descs(ie);
 	if (idxd->request_int_handles)
 		idxd_device_release_int_handle(idxd, ie->int_handle, IDXD_IRQ_MSIX);
 	idxd_device_clear_perm_entry(idxd, ie);
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index ce78b9a7c641..f4aed90560e0 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -804,6 +804,7 @@ int idxd_wq_init_percpu_ref(struct idxd_wq *wq);
 void idxd_wq_free_irq(struct idxd_wq *wq);
 int idxd_wq_request_irq(struct idxd_wq *wq);
 void idxd_wq_flush_descs(struct idxd_wq *wq);
+void idxd_wq_release_int_handle(struct idxd_wq *wq);
 
 /* submission */
 int idxd_submit_desc(struct idxd_wq *wq, struct idxd_desc *desc);
diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c
index 6a25e1fd0e62..26d75d744797 100644
--- a/drivers/dma/idxd/irq.c
+++ b/drivers/dma/idxd/irq.c
@@ -405,6 +405,7 @@ static void idxd_wqs_flush_descs(struct idxd_device *idxd)
 		struct idxd_wq *wq = idxd->wqs[i];
 
 		idxd_wq_flush_descs(wq);
+		idxd_wq_release_int_handle(wq);
 	}
 }
 

-- 
2.56.0

Re: [PATCH 6/6] dmaengine: idxd: Do not release interrupt handles on terminate_all

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dma: idxd: sleep in atomic context in device_terminate_all()
- [High] dmaengine: idxd: Hardware UAF and IOMMU faults due to flushing active WQ without disable
- [Medium] dmaengine: idxd: missing check for INVALID_INT_HANDLE in release
--

commit dc7ea0d446f4b8c4e4f29fa4595acabcca6b297f
Author: Vinicius Costa Gomes <vinicius.gomes@intel.com>

dmaengine: idxd: Do not release interrupt handles on terminate_all

This commit splits the interrupt handle teardown from the descriptor
flushing in idxd_wq_flush_descs(). This prevents the interrupt handle
from being incorrectly released during dmaengine terminate_all, which
was causing subsequent runs to time out.

> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
> index 131138483b877..06299f40f5312 100644
> --- a/drivers/dma/idxd/device.c
> +++ b/drivers/dma/idxd/device.c
> @@ -1349,6 +1349,16 @@ void idxd_wq_free_irq(struct idxd_wq *wq)
>  }
>  
>  void idxd_wq_flush_descs(struct idxd_wq *wq)
> +{
> +	guard(mutex)(&wq->wq_lock);
> +
> +	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
> +		return;
> +
> +	idxd_flush_pending_descs(&wq->ie);
> +}

[Severity: High]
Does this code introduce a sleep-in-atomic bug when called via the
dmaengine termination path?

The dmaengine API allows dmaengine_terminate_async() to be called from
atomic context (such as an interrupt handler or while holding a spinlock).

This routes to idxd_dma_terminate_all(), which then calls
idxd_wq_flush_descs().

Because idxd_wq_flush_descs() unconditionally acquires wq->wq_lock using
a sleepable mutex, terminating an asynchronous transfer from atomic
context could trigger a scheduling-while-atomic panic.

[Severity: High]
Can this flush implementation cause a use-after-free and IOMMU faults?

By calling idxd_flush_pending_descs() without first disabling or halting
the hardware workqueue, the software descriptors are completed, freed,
and client memory is unmapped while the hardware may still be actively
processing them.

Since the hardware continues to execute DMA operations into buffers that
have already been unmapped, can this result in memory corruption and
IOMMU faults?

> +
> +void idxd_wq_release_int_handle(struct idxd_wq *wq)
>  {
>  	struct idxd_irq_entry *ie = &wq->ie;
>  	struct idxd_device *idxd = wq->idxd;
> @@ -1358,7 +1368,6 @@ void idxd_wq_flush_descs(struct idxd_wq *wq)
>  	if (wq->state != IDXD_WQ_ENABLED || wq->type != IDXD_WQT_KERNEL)
>  		return;
>  
> -	idxd_flush_pending_descs(ie);
>  	if (idxd->request_int_handles)
>  		idxd_device_release_int_handle(idxd, ie->int_handle, IDXD_IRQ_MSIX);

[Severity: Medium]
Is it possible for ie->int_handle to be INVALID_INT_HANDLE here?

If a previous operation leaves the handle as INVALID_INT_HANDLE (-1) but
the workqueue remains in the IDXD_WQ_ENABLED state, this code will
unconditionally issue a release command to the hardware using -1 (0xffff).

Should this function include a check for ie->int_handle ==
INVALID_INT_HANDLE before calling idxd_device_release_int_handle(),
similar to how it is handled in idxd_wq_free_irq()?

>  	idxd_device_clear_perm_entry(idxd, ie);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-wt-idxd-fixes-v1-0-30a35a445077@intel.com?part=6