[PATCH v4 00/12] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma

[PATCH v4 00/12] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma

[Logan Gunthorpe]

When reviewing the recent switchtec patchset[1], the Sashiko bot noticed
a handful of pre-existing problems in the ioat and switchtec drivers.
I attempted to fix those plus an unrelated issue reported in plxdma but
when I submitted those patches, Sashiko found even more issues[2][3][4]
(it is relentless!).

I've fixed the issues reported with v1, v2 and v3 of this series and
many of the ones for the switchtec driver. But the pre-existing issues
in ioat, plxdma and the dmaengine itself I've punted until I can find
some time to dig into them.

The series is based off of v7.2-rc5.

Thanks,

Logan

[1] https://lore.kernel.org/all/20260707162045.23910-1-logang@deltatee.com
[2] https://sashiko.dev/#/patchset/20260717221001.361421-1-logang@deltatee.com
[3] https://sashiko.dev/#/patchset/20260721155739.62120-1-logang@deltatee.com
[4] https://sashiko.dev/#/patchset/20260727181526.9672-1-logang@deltatee.com

Changes since v3:
 * Add a patch (3) making switchtec_dma_chan_stop() clear the DMA base
   registers even when halt_channel() times out, and return the halt
   result. switchtec_dma_free_chan_resources() (patch 3) and the
   alloc_chan_resources() error path (patch 4) now skip freeing the
   descriptor rings when the halt wasn't confirmed. This will leak some
   memory on tear down but that avoids broken hardware from scribbling
   on memory that may have been freed and reallocated. (Per Sashiko)
 * Remove each channel's list entry in switchtec_dma_chans_free()
   (patch 5), immediately before the memory is freed, instead of in
   switchtec_dma_chans_disable() (patch 8), which now only frees the
   channel status IRQ. (Per Sashiko)
 * Collected Reviewed-by tags from Frank and applied one of his
   commit message suggestions.

Changes since v2:

 * Fixed a race when unlisting the channels in the error path.
   The interrupt needed to be disabled before hand. (Per Sashiko)
 * Picked up Acked-by from Dave Jiang on the two ioat patches.

Changes since v1:

 * Added a fix for switchtec_dma_alloc_chan_resources()'s error path
   calling disable_channel() instead of properly halting the channel
   before freeing the descriptor rings. (Per Sashiko)
 * Added a fix for switchtec-dma channel structs being freed without
   being removed from dma_dev->channels on a registration failure,
   while the channel status IRQ is still live. (Per Sashiko)
 * Added a fix for switchtec_dma_remove() using swdma_dev after it may
   already have been freed by dma_async_device_unregister(). (Per
   Sashiko)
 * Added a fix for chan_status_irq being freed with the wrong API, and
   a valid vector index of 0 being incorrectly treated as unset.
   (Per Sashiko)
 * Made switchtec_dma_chans_release() void, since nothing checked its
   return value. (Noticed while reviewing the code for these changes).

Logan Gunthorpe (12):
  dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()
  dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources
  dmaengine: switchtec-dma: always clear DMA base registers on
    chan_stop()
  dmaengine: switchtec-dma: halt channel on alloc_chan_resources error
  dmaengine: switchtec-dma: fix channel leak on registration failure
  dmaengine: switchtec-dma: make switchtec_dma_chans_release() void
  dmaengine: switchtec-dma: fix chan_status_irq cleanup on create()
    error
  dmaengine: switchtec-dma: disable channels before freeing on
    registration failure
  dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()
  dmaengine: ioat: disable relaxed ordering before registering the
    device
  dmaengine: ioat: use sysfs_emit() in per-channel sysfs show()
  dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()

 drivers/dma/ioat/init.c     |  18 +++---
 drivers/dma/ioat/sysfs.c    |  22 ++++----
 drivers/dma/plx_dma.c       |  10 ++--
 drivers/dma/switchtec_dma.c | 107 ++++++++++++++++++++++++++----------
 4 files changed, 104 insertions(+), 53 deletions(-)


base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
-- 
2.47.3

[PATCH v4 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()

[Logan Gunthorpe]

switchtec_dma_free_desc() frees swdma_chan->hw_sq, hw_cq, and every
desc_ring[] entry without clearing the pointers afterward. If
switchtec_dma_alloc_chan_resources() fails partway through and calls
it during unwind, then a later retry of alloc_chan_resources() fails
in switchtec_dma_alloc_desc() before reallocating one of those
pointers, its own failure path calls switchtec_dma_free_desc() again
and frees the same, already-freed pointers a second time.

NULL out each pointer as it's freed so a subsequent call is a no-op
for anything already released.

Fixes: 30eba9df76ad ("dmaengine: switchtec-dma: Implement hardware initialization and cleanup")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 3ef928640615..a4a7d66d042d 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -886,14 +886,18 @@ static void switchtec_dma_free_desc(struct switchtec_dma_chan *swdma_chan)
 	if (swdma_chan->hw_sq)
 		dma_free_coherent(swdma_dev->dma_dev.dev, size,
 				  swdma_chan->hw_sq, swdma_chan->dma_addr_sq);
+	swdma_chan->hw_sq = NULL;
 
 	size = SWITCHTEC_DMA_CQ_SIZE * sizeof(*swdma_chan->hw_cq);
 	if (swdma_chan->hw_cq)
 		dma_free_coherent(swdma_dev->dma_dev.dev, size,
 				  swdma_chan->hw_cq, swdma_chan->dma_addr_cq);
+	swdma_chan->hw_cq = NULL;
 
-	for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++)
+	for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++) {
 		kfree(swdma_chan->desc_ring[i]);
+		swdma_chan->desc_ring[i] = NULL;
+	}
 }
 
 static int switchtec_dma_alloc_desc(struct switchtec_dma_chan *swdma_chan)
-- 
2.47.3

[PATCH v4 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

[Logan Gunthorpe]

switchtec_dma_alloc_chan_resources() returns directly on any later
failure, without ever freeing the descriptor rings and coherent DMA
memory it just allocated. The dmaengine core does not call
device_free_chan_resources() when device_alloc_chan_resources() fails,
so the driver has to unwind its own partial state.

The device-removed check also runs after ring_active and
comp_ring_active have already been set true, so a failure there left
the channel marked active despite alloc_chan_resources() reporting
failure.

Add an error-unwind path that disables the channel and frees the
descriptor rings on every failure after allocation. ring_active and
comp_ring_active are cleared under the same locks
switchtec_dma_free_chan_resources() already uses, since the completion
tasklet checks comp_ring_active under complete_lock before touching
the completion ring, and a stale IRQ can still be in flight when this
unwind path runs.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260707165555.350951F000E9@smtp.kernel.org
Fixes: 30eba9df76ad ("dmaengine: switchtec-dma: Implement hardware initialization and cleanup")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index a4a7d66d042d..f77da31aeb65 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -988,15 +988,15 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
 
 	rc = enable_channel(swdma_chan);
 	if (rc)
-		return rc;
+		goto err_free_desc;
 
 	rc = reset_channel(swdma_chan);
 	if (rc)
-		return rc;
+		goto err_disable_channel;
 
 	rc = unhalt_channel(swdma_chan);
 	if (rc)
-		return rc;
+		goto err_disable_channel;
 
 	swdma_chan->ring_active = true;
 	swdma_chan->comp_ring_active = true;
@@ -1007,7 +1007,8 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
 	rcu_read_lock();
 	if (!rcu_dereference(swdma_dev->pdev)) {
 		rcu_read_unlock();
-		return -ENODEV;
+		rc = -ENODEV;
+		goto err_ring_inactive;
 	}
 
 	perf_cfg = readl(&swdma_chan->mmio_chan_fw->perf_cfg);
@@ -1029,6 +1030,20 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
 		FIELD_GET(PERF_MRRS_MASK, perf_cfg));
 
 	return SWITCHTEC_DMA_SQ_SIZE;
+
+err_ring_inactive:
+	spin_lock_bh(&swdma_chan->submit_lock);
+	swdma_chan->ring_active = false;
+	spin_unlock_bh(&swdma_chan->submit_lock);
+
+	spin_lock_bh(&swdma_chan->complete_lock);
+	swdma_chan->comp_ring_active = false;
+	spin_unlock_bh(&swdma_chan->complete_lock);
+err_disable_channel:
+	disable_channel(swdma_chan);
+err_free_desc:
+	switchtec_dma_free_desc(swdma_chan);
+	return rc;
 }
 
 static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
-- 
2.47.3

[PATCH v4 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop()

[Logan Gunthorpe]

switchtec_dma_chan_stop() returned early if halt_channel() timed out,
skipping the writes that clear sq_base/cq_base on the channel, and
gave its caller no way to tell the halt hadn't been confirmed.
switchtec_dma_free_chan_resources() unconditionally frees the
descriptor rings right after calling this function, so if the
hardware failed to halt, it could keep writing into memory that had
already been freed.

Attempt the register clear regardless of whether the halt was successful
and have switchtec_dma_chan_stop() return the halt result so callers can
tell when it wasn't confirmed. switchtec_dma_free_chan_resources() now
skips freeing the descriptor rings (leaking them instead) in case the
hardware continues to write into that memory.

But all this is hardening that is pretty unlikely to be hit in the real
world.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260721162531.BA01A1F01560@smtp.kernel.org
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index f77da31aeb65..cbbc9ad08247 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -543,26 +543,33 @@ switchtec_dma_abort_desc(struct switchtec_dma_chan *swdma_chan, int force)
 	spin_unlock_bh(&swdma_chan->complete_lock);
 }
 
-static void switchtec_dma_chan_stop(struct switchtec_dma_chan *swdma_chan)
+static int switchtec_dma_chan_stop(struct switchtec_dma_chan *swdma_chan)
 {
+	struct pci_dev *pdev;
 	int rc;
 
 	rc = halt_channel(swdma_chan);
-	if (rc)
-		return;
 
 	rcu_read_lock();
-	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
+	pdev = rcu_dereference(swdma_chan->swdma_dev->pdev);
+	if (!pdev) {
 		rcu_read_unlock();
-		return;
+		return rc;
 	}
 
+	if (rc)
+		pci_err(pdev,
+			"Channel %d halt timed out, clearing DMA base registers anyway\n",
+			swdma_chan->index);
+
 	writel(0, &swdma_chan->mmio_chan_fw->sq_base_lo);
 	writel(0, &swdma_chan->mmio_chan_fw->sq_base_hi);
 	writel(0, &swdma_chan->mmio_chan_fw->cq_base_lo);
 	writel(0, &swdma_chan->mmio_chan_fw->cq_base_hi);
 
 	rcu_read_unlock();
+
+	return rc;
 }
 
 static int switchtec_dma_terminate_all(struct dma_chan *chan)
@@ -1050,6 +1057,7 @@ static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct switchtec_dma_chan *swdma_chan =
 		container_of(chan, struct switchtec_dma_chan, dma_chan);
+	int rc;
 
 	spin_lock_bh(&swdma_chan->submit_lock);
 	swdma_chan->ring_active = false;
@@ -1059,9 +1067,10 @@ static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
 	swdma_chan->comp_ring_active = false;
 	spin_unlock_bh(&swdma_chan->complete_lock);
 
-	switchtec_dma_chan_stop(swdma_chan);
+	rc = switchtec_dma_chan_stop(swdma_chan);
 	switchtec_dma_abort_desc(swdma_chan, 0);
-	switchtec_dma_free_desc(swdma_chan);
+	if (!rc)
+		switchtec_dma_free_desc(swdma_chan);
 
 	disable_channel(swdma_chan);
 }
-- 
2.47.3

[PATCH v4 04/12] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

[Logan Gunthorpe]

The error-unwind path called disable_channel() before freeing the
descriptor rings, but that only clears the enable bit with an
unflushed write -- it doesn't halt the channel or clear its DMA base
address registers. If unhalt_channel() timed out, the channel's actual
state is unknown at that point, so nothing guarantees the hardware
isn't still touching the rings when they're freed.

Call switchtec_dma_chan_stop() first, matching what
switchtec_dma_free_chan_resources() already does before freeing
descriptors on the normal teardown path: it synchronously halts the
channel and zeroes the DMA base registers. If the halt itself can't be
confirmed, skip freeing the descriptor rings (leaking them instead).

Fixes: 30eba9df76ad ("dmaengine: switchtec-dma: Implement hardware initialization and cleanup")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223647.F0A051F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260727185101.D221C1F000E9@smtp.kernel.org
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index cbbc9ad08247..25b988e9b4af 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1047,6 +1047,10 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
 	swdma_chan->comp_ring_active = false;
 	spin_unlock_bh(&swdma_chan->complete_lock);
 err_disable_channel:
+	if (switchtec_dma_chan_stop(swdma_chan)) {
+		disable_channel(swdma_chan);
+		return rc;
+	}
 	disable_channel(swdma_chan);
 err_free_desc:
 	switchtec_dma_free_desc(swdma_chan);
-- 
2.47.3

[PATCH v4 05/12] dmaengine: switchtec-dma: fix channel leak on registration failure

[Logan Gunthorpe]

switchtec_dma_chans_release() is called in three places but the
underlying memory is not freed in all of those places. In order to
clean this up, introduce a switchtec_dma_chans_free() helper that
will free the memory.

Ensure each call to switchtec_dma_chans_release() has a corresponding
switchtec_dma_chans_free() call. (The release in switchtec_dma_remove()
pairs with the free in switchtec_dma_release()).

swdma_dev->chan_cnt is now set to the number of channels that succeeded
when one fails to initialise, so switchtec_dma_chans_free() can still
be used if not all channels succeed in being allocated.

switchtec_dma_chans_free() also removes each channel from
dma_dev->channels before freeing it, since the channel status ISR
walks that list and would otherwise dereference freed memory.

Fixes: 30eba9df76ad ("dmaengine: switchtec-dma: Implement hardware initialization and cleanup")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223024.9BB8A1F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260727190604.397DB1F000E9@smtp.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 25b988e9b4af..48da269198f3 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1188,6 +1188,18 @@ static int switchtec_dma_chans_release(struct pci_dev *pdev,
 	return 0;
 }
 
+static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
+{
+	int i;
+
+	for (i = 0; i < swdma_dev->chan_cnt; i++) {
+		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
+		kfree(swdma_dev->swdma_chans[i]);
+	}
+
+	kfree(swdma_dev->swdma_chans);
+}
+
 static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 					 struct pci_dev *pdev, int chan_cnt)
 {
@@ -1213,7 +1225,7 @@ static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 		if (rc) {
 			dev_err(&pdev->dev, "Channel %d: init channel failed\n",
 				i);
-			chan_cnt = i;
+			swdma_dev->chan_cnt = i;
 			goto err_exit;
 		}
 	}
@@ -1221,10 +1233,8 @@ static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 	return chan_cnt;
 
 err_exit:
-	for (i = 0; i < chan_cnt; i++)
-		switchtec_dma_chan_free(pdev, swdma_dev->swdma_chans[i]);
-
-	kfree(swdma_dev->swdma_chans);
+	switchtec_dma_chans_release(pdev, swdma_dev);
+	switchtec_dma_chans_free(swdma_dev);
 
 	return rc;
 }
@@ -1233,12 +1243,8 @@ static void switchtec_dma_release(struct dma_device *dma_dev)
 {
 	struct switchtec_dma_dev *swdma_dev =
 		container_of(dma_dev, struct switchtec_dma_dev, dma_dev);
-	int i;
 
-	for (i = 0; i < swdma_dev->chan_cnt; i++)
-		kfree(swdma_dev->swdma_chans[i]);
-
-	kfree(swdma_dev->swdma_chans);
+	switchtec_dma_chans_free(swdma_dev);
 
 	put_device(dma_dev->dev);
 	kfree(swdma_dev);
@@ -1329,6 +1335,7 @@ static int switchtec_dma_create(struct pci_dev *pdev)
 
 err_chans_release_exit:
 	switchtec_dma_chans_release(pdev, swdma_dev);
+	switchtec_dma_chans_free(swdma_dev);
 
 err_exit:
 	if (swdma_dev->chan_status_irq)
-- 
2.47.3

[PATCH v4 06/12] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

[Logan Gunthorpe]

switchtec_dma_chans_release() always returned 0, and no caller
checked it.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 48da269198f3..63e60de9f86d 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1177,15 +1177,13 @@ static int switchtec_dma_chan_free(struct pci_dev *pdev,
 	return 0;
 }
 
-static int switchtec_dma_chans_release(struct pci_dev *pdev,
-				       struct switchtec_dma_dev *swdma_dev)
+static void switchtec_dma_chans_release(struct pci_dev *pdev,
+					struct switchtec_dma_dev *swdma_dev)
 {
 	int i;
 
 	for (i = 0; i < swdma_dev->chan_cnt; i++)
 		switchtec_dma_chan_free(pdev, swdma_dev->swdma_chans[i]);
-
-	return 0;
 }
 
 static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
-- 
2.47.3

[PATCH v4 07/12] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

[Logan Gunthorpe]

chan_status_irq stores an MSI-X vector index, but err_exit freed it
with plain free_irq() instead of pci_free_irq(), which would free the
wrong Linux IRQ. The guard also treated a valid vector index of 0 as
unset, skipping the free entirely in that case and leaving the handler
registered against soon-to-be-freed swdma_dev.

Initialize chan_status_irq to -1 and use the value being non-negative
to signal when to free it with pci_free_irq().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 63e60de9f86d..6ab2c62b52fd 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1262,6 +1262,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
 	if (!swdma_dev)
 		return -ENOMEM;
 
+	swdma_dev->chan_status_irq = -1;
+
 	swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
 				 pci_resource_len(pdev, 0));
 
@@ -1336,8 +1338,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
 	switchtec_dma_chans_free(swdma_dev);
 
 err_exit:
-	if (swdma_dev->chan_status_irq)
-		free_irq(swdma_dev->chan_status_irq, swdma_dev);
+	if (swdma_dev->chan_status_irq >= 0)
+		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
 
 	iounmap(swdma_dev->bar);
 	kfree(swdma_dev);
-- 
2.47.3

[PATCH v4 08/12] dmaengine: switchtec-dma: disable channels before freeing on registration failure

[Logan Gunthorpe]

When switchtec_dma_create() fails after channels have been added to
dma_dev->channels (either from switchtec_dma_chans_enumerate()'s own
error path, or from dma_async_device_register() failing), the channel
status IRQ is still live while the channels are released and freed,
so it can fire mid-teardown against channels that are only partially
torn down.

Add switchtec_dma_chans_disable(), which frees the channel status IRQ
before releasing and freeing channels in both error paths.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260721162822.05CDD1F000E9@smtp.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 6ab2c62b52fd..ecf3a44d4dcc 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1198,6 +1198,15 @@ static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
 	kfree(swdma_dev->swdma_chans);
 }
 
+static void switchtec_dma_chans_disable(struct pci_dev *pdev,
+					struct switchtec_dma_dev *swdma_dev)
+{
+	if (swdma_dev->chan_status_irq >= 0) {
+		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
+		swdma_dev->chan_status_irq = -1;
+	}
+}
+
 static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 					 struct pci_dev *pdev, int chan_cnt)
 {
@@ -1231,6 +1240,7 @@ static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 	return chan_cnt;
 
 err_exit:
+	switchtec_dma_chans_disable(pdev, swdma_dev);
 	switchtec_dma_chans_release(pdev, swdma_dev);
 	switchtec_dma_chans_free(swdma_dev);
 
@@ -1334,6 +1344,7 @@ static int switchtec_dma_create(struct pci_dev *pdev)
 	return 0;
 
 err_chans_release_exit:
+	switchtec_dma_chans_disable(pdev, swdma_dev);
 	switchtec_dma_chans_release(pdev, swdma_dev);
 	switchtec_dma_chans_free(swdma_dev);
 
-- 
2.47.3

[PATCH v4 09/12] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

[Logan Gunthorpe]

dma_async_device_unregister() can drop the last reference on dma_dev
and free swdma_dev synchronously via switchtec_dma_release(), but
switchtec_dma_remove() then uses swdma_dev->bar for iounmap().

Cache bar in a local variable before the unregister call.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index ecf3a44d4dcc..babb2bcb6208 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1393,6 +1393,7 @@ static int switchtec_dma_probe(struct pci_dev *pdev,
 static void switchtec_dma_remove(struct pci_dev *pdev)
 {
 	struct switchtec_dma_dev *swdma_dev = pci_get_drvdata(pdev);
+	void __iomem *bar = swdma_dev->bar;
 
 	switchtec_dma_chans_release(pdev, swdma_dev);
 
@@ -1405,7 +1406,7 @@ static void switchtec_dma_remove(struct pci_dev *pdev)
 
 	dma_async_device_unregister(&swdma_dev->dma_dev);
 
-	iounmap(swdma_dev->bar);
+	iounmap(bar);
 	pci_release_mem_regions(pdev);
 	pci_disable_device(pdev);
 }
-- 
2.47.3

[PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device

[Logan Gunthorpe]

ioat3_dma_probe() disabled PCIe relaxed ordering after calling
dma_async_device_register(), so if an error occurs and the code jumps
to err_disable_interrupts, the function returns with the device still
registered in the core's dma_device_list while the caller frees the
ioatdma_device struct, leaving a dangling registration that anything
walking the device list can dereference after it's been freed.

Move the capability read/write ahead of dma_async_device_register()
instead. Nothing after registration depends on relaxed ordering
already being disabled, and nothing before it depends on the device
being registered, so this is a plain reordering. It also means every
remaining step after registration can't fail, so there's no need to
ever have to unregister the device once registered.

Fixes: 511deae0261c ("dmaengine: ioatdma: disable relaxed ordering for ioatdma")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dmaengine/20260707165906.249F41F000E9@smtp.kernel.org
Acked-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/ioat/init.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 737496391109..a57024c4b066 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -1170,15 +1170,6 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
 		       ioat_chan->reg_base + IOAT_DCACTRL_OFFSET);
 	}
 
-	err = dma_async_device_register(&ioat_dma->dma_dev);
-	if (err)
-		goto err_disable_interrupts;
-
-	ioat_kobject_add(ioat_dma, &ioat_ktype);
-
-	if (dca)
-		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
-
 	/* disable relaxed ordering */
 	err = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
 	if (err) {
@@ -1194,6 +1185,15 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
 		goto err_disable_interrupts;
 	}
 
+	err = dma_async_device_register(&ioat_dma->dma_dev);
+	if (err)
+		goto err_disable_interrupts;
+
+	ioat_kobject_add(ioat_dma, &ioat_ktype);
+
+	if (dca)
+		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
+
 	if (ioat_dma->cap & IOAT_CAP_DPS)
 		writeb(ioat_pending_level + 1,
 		       ioat_dma->reg_base + IOAT_PREFETCH_LIMIT_OFFSET);
-- 
2.47.3

[PATCH v4 11/12] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show()

[Logan Gunthorpe]

Convert the sprintf() calls in the per-channel sysfs attribute show()
functions to sysfs_emit().

Acked-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/ioat/sysfs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
index e796ddb5383f..976134df8108 100644
--- a/drivers/dma/ioat/sysfs.c
+++ b/drivers/dma/ioat/sysfs.c
@@ -24,12 +24,12 @@ static ssize_t cap_show(struct dma_chan *c, char *page)
 {
 	struct dma_device *dma = c->device;
 
-	return sprintf(page, "copy%s%s%s%s%s\n",
-		       dma_has_cap(DMA_PQ, dma->cap_mask) ? " pq" : "",
-		       dma_has_cap(DMA_PQ_VAL, dma->cap_mask) ? " pq_val" : "",
-		       dma_has_cap(DMA_XOR, dma->cap_mask) ? " xor" : "",
-		       dma_has_cap(DMA_XOR_VAL, dma->cap_mask) ? " xor_val" : "",
-		       dma_has_cap(DMA_INTERRUPT, dma->cap_mask) ? " intr" : "");
+	return sysfs_emit(page, "copy%s%s%s%s%s\n",
+		dma_has_cap(DMA_PQ, dma->cap_mask) ? " pq" : "",
+		dma_has_cap(DMA_PQ_VAL, dma->cap_mask) ? " pq_val" : "",
+		dma_has_cap(DMA_XOR, dma->cap_mask) ? " xor" : "",
+		dma_has_cap(DMA_XOR_VAL, dma->cap_mask) ? " xor_val" : "",
+		dma_has_cap(DMA_INTERRUPT, dma->cap_mask) ? " intr" : "");
 
 }
 static const struct ioat_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
@@ -39,8 +39,8 @@ static ssize_t version_show(struct dma_chan *c, char *page)
 	struct dma_device *dma = c->device;
 	struct ioatdma_device *ioat_dma = to_ioatdma_device(dma);
 
-	return sprintf(page, "%d.%d\n",
-		       ioat_dma->version >> 4, ioat_dma->version & 0xf);
+	return sysfs_emit(page, "%d.%d\n",
+			   ioat_dma->version >> 4, ioat_dma->version & 0xf);
 }
 static const struct ioat_sysfs_entry ioat_version_attr = __ATTR_RO(version);
 
@@ -118,7 +118,7 @@ static ssize_t ring_size_show(struct dma_chan *c, char *page)
 {
 	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
 
-	return sprintf(page, "%d\n", (1 << ioat_chan->alloc_order) & ~1);
+	return sysfs_emit(page, "%d\n", (1 << ioat_chan->alloc_order) & ~1);
 }
 static const struct ioat_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
 
@@ -127,7 +127,7 @@ static ssize_t ring_active_show(struct dma_chan *c, char *page)
 	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
 
 	/* ...taken outside the lock, no need to be precise */
-	return sprintf(page, "%d\n", ioat_ring_active(ioat_chan));
+	return sysfs_emit(page, "%d\n", ioat_ring_active(ioat_chan));
 }
 static const struct ioat_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
 
@@ -135,7 +135,7 @@ static ssize_t intr_coalesce_show(struct dma_chan *c, char *page)
 {
 	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
 
-	return sprintf(page, "%d\n", ioat_chan->intr_coalesce);
+	return sysfs_emit(page, "%d\n", ioat_chan->intr_coalesce);
 }
 
 static ssize_t intr_coalesce_store(struct dma_chan *c, const char *page,
-- 
2.47.3

[PATCH v4 12/12] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()

[Logan Gunthorpe]

plx_dma_create() registered the interrupt handler with request_irq()
before initializing plxdev->bar. If the device raised an interrupt in
that window, plx_dma_isr() would dereference the still-NULL bar.

Move the bar assignment ahead of request_irq() so everything the
handler can touch is initialized before it can run.

Reported-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
Reported-by: Kyungwook Boo <bookyungwook@gmail.com>
Link: https://lore.kernel.org/all/20260610112121.676561-1-jjy600901@snu.ac.kr
Fixes: c2dbcaa8c672 ("dmaengine: plx-dma: Implement hardware initialization and cleanup")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/plx_dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
index 84941a918b01..409898e92c32 100644
--- a/drivers/dma/plx_dma.c
+++ b/drivers/dma/plx_dma.c
@@ -504,17 +504,17 @@ static int plx_dma_create(struct pci_dev *pdev)
 	if (!plxdev)
 		return -ENOMEM;
 
-	rc = request_irq(pci_irq_vector(pdev, 0), plx_dma_isr, 0,
-			 KBUILD_MODNAME, plxdev);
-	if (rc)
-		goto free_plx;
-
 	spin_lock_init(&plxdev->ring_lock);
 	tasklet_setup(&plxdev->desc_task, plx_dma_desc_task);
 
 	RCU_INIT_POINTER(plxdev->pdev, pdev);
 	plxdev->bar = pcim_iomap_table(pdev)[0];
 
+	rc = request_irq(pci_irq_vector(pdev, 0), plx_dma_isr, 0,
+			 KBUILD_MODNAME, plxdev);
+	if (rc)
+		goto free_plx;
+
 	dma = &plxdev->dma_dev;
 	INIT_LIST_HEAD(&dma->channels);
 	dma_cap_set(DMA_MEMCPY, dma->cap_mask);
-- 
2.47.3

Re: [PATCH v4 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing resource cleanup on error paths in switchtec_dma_alloc_chan_resources() causes memory leak of DMA descriptors and rings.
--

--- Patch [1]: [PATCH v4 01/12] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() ---
commit 5f9c3413d8c3e6b409c7cf9c3bd9dfaa071f19f6
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()

This commit fixes a double-free in switchtec_dma_free_desc() by setting the
pointers to NULL after freeing them. This ensures that any subsequent calls
to the function are a no-op for resources that have already been released.

> If switchtec_dma_alloc_chan_resources() fails partway through and calls
> it during unwind, then a later retry of alloc_chan_resources() fails
> in switchtec_dma_alloc_desc() before reallocating one of those

[Severity: High]
This is a pre-existing issue, but does switchtec_dma_alloc_chan_resources()
actually call switchtec_dma_free_desc() during unwind?

If switchtec_dma_alloc_desc() succeeds, but a subsequent step fails:

drivers/dma/switchtec_dma.c:switchtec_dma_alloc_chan_resources() {
    ...
	rc = switchtec_dma_alloc_desc(swdma_chan);
	if (rc)
		return rc;

	rc = enable_channel(swdma_chan);
	if (rc)
		return rc;

	rc = reset_channel(swdma_chan);
	if (rc)
		return rc;

	rc = unhalt_channel(swdma_chan);
	if (rc)
		return rc;
    ...
}

It appears we return early without unwinding the allocations made by
switchtec_dma_alloc_desc(). Since the DMA engine core does not automatically
invoke device_free_chan_resources() when device_alloc_chan_resources()
fails, does this result in a permanent leak of the DMA descriptors and
rings?

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 3ef9286406159..a4a7d66d042de 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
[ ... ]
> @@ -886,14 +886,18 @@ static void switchtec_dma_free_desc(struct switchtec_dma_chan *swdma_chan)
>  	if (swdma_chan->hw_sq)
>  		dma_free_coherent(swdma_dev->dma_dev.dev, size,
>  				  swdma_chan->hw_sq, swdma_chan->dma_addr_sq);
> +	swdma_chan->hw_sq = NULL;
>  
>  	size = SWITCHTEC_DMA_CQ_SIZE * sizeof(*swdma_chan->hw_cq);
>  	if (swdma_chan->hw_cq)
>  		dma_free_coherent(swdma_dev->dma_dev.dev, size,
>  				  swdma_chan->hw_cq, swdma_chan->dma_addr_cq);
> +	swdma_chan->hw_cq = NULL;
>  
> -	for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++)
> +	for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++) {
>  		kfree(swdma_chan->desc_ring[i]);
> +		swdma_chan->desc_ring[i] = NULL;
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=1

Re: [PATCH v4 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unintentional memory leak of DMA descriptor rings during device hot-unplug or driver unbind.
- [High] Zeroing DMA base registers on an unhalted channel directs ongoing DMA to physical address 0, causing memory corruption.
--

--- Patch [3]: [PATCH v4 03/12] dmaengine: switchtec-dma: always clear DMA base registers on chan_stop() ---
commit ba435d6b93a5b911f612d1880a59068a9e918427
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: always clear DMA base registers on chan_stop()

This patch modifies switchtec_dma_chan_stop() to clear DMA base registers
even if the hardware halt times out. It also updates callers to check the
halt result and skip freeing descriptor rings if the halt was unconfirmed.

Link: https://lore.kernel.org/dmaengine/20260721162531.BA01A1F01560@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index f77da31aeb65e..cbbc9ad082471 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -543,26 +543,33 @@ switchtec_dma_abort_desc(struct switchtec_dma_chan *swdma_chan, int force)
>  	spin_unlock_bh(&swdma_chan->complete_lock);
>  }
>  
> -static void switchtec_dma_chan_stop(struct switchtec_dma_chan *swdma_chan)
> +static int switchtec_dma_chan_stop(struct switchtec_dma_chan *swdma_chan)
>  {
> +	struct pci_dev *pdev;
>  	int rc;
>  
>  	rc = halt_channel(swdma_chan);
> -	if (rc)
> -		return;
>  
>  	rcu_read_lock();
> -	if (!rcu_dereference(swdma_chan->swdma_dev->pdev)) {
> +	pdev = rcu_dereference(swdma_chan->swdma_dev->pdev);
> +	if (!pdev) {
>  		rcu_read_unlock();
> -		return;
> +		return rc;
>  	}
>  
> +	if (rc)
> +		pci_err(pdev,
> +			"Channel %d halt timed out, clearing DMA base registers anyway\n",
> +			swdma_chan->index);
> +
>  	writel(0, &swdma_chan->mmio_chan_fw->sq_base_lo);
>  	writel(0, &swdma_chan->mmio_chan_fw->sq_base_hi);
>  	writel(0, &swdma_chan->mmio_chan_fw->cq_base_lo);
>  	writel(0, &swdma_chan->mmio_chan_fw->cq_base_hi);
>  
>  	rcu_read_unlock();
> +
> +	return rc;
>  }

[Severity: High]
If the channel failed to halt and is still actively processing transactions,
could writing 0 to the base address registers redirect ongoing DMA to physical 
address 0x0?

The commit message states the descriptor rings are intentionally leaked so
the hardware can safely continue to write into them. However, by clearing
the base addresses here while the hardware is active, it appears the active
DMA is instead pointed at address 0x0, which might lead to memory corruption.

[ ... ]

> @@ -1050,6 +1057,7 @@ static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
>  {
>  	struct switchtec_dma_chan *swdma_chan =
>  		container_of(chan, struct switchtec_dma_chan, dma_chan);
> +	int rc;
>  
>  	spin_lock_bh(&swdma_chan->submit_lock);
>  	swdma_chan->ring_active = false;
> @@ -1059,9 +1067,10 @@ static void switchtec_dma_free_chan_resources(struct dma_chan *chan)
>  	swdma_chan->comp_ring_active = false;
>  	spin_unlock_bh(&swdma_chan->complete_lock);
>  
> -	switchtec_dma_chan_stop(swdma_chan);
> +	rc = switchtec_dma_chan_stop(swdma_chan);
>  	switchtec_dma_abort_desc(swdma_chan, 0);
> -	switchtec_dma_free_desc(swdma_chan);
> +	if (!rc)
> +		switchtec_dma_free_desc(swdma_chan);
>  
>  	disable_channel(swdma_chan);
>  }

[Severity: High]
Does this logic skip freeing the descriptor rings during normal device unplug?

When a device is hot-unplugged and the driver unbinds, swdma_dev->pdev is set
to NULL. This causes halt_channel() and switchtec_dma_chan_stop() to return
-ENODEV.

Because rc is -ENODEV (non-zero) on a standard unplug, the condition if (!rc)
evaluates to false. This seems to unintentionally bypass
switchtec_dma_free_desc() and permanently leak the descriptor rings and
coherent memory. A similar bypass happens in the error paths of
switchtec_dma_alloc_chan_resources().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=3

Re: [PATCH v4 04/12] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Intentionally leaking descriptors to prevent hardware UAF leaves dangling pointers in the channel structure, causing a UAF if a subsequent allocation fails, or a permanent leak if it succeeds.

Pre-existing issues:
- [Critical] Zeroing DMA base registers while the channel fails to halt redirects active DMA transactions to physical address 0.
- [High] Holding a spinlock while polling for up to 100ms violates core kernel locking rules and can trigger softlockups.
--

--- Patch [4]: [PATCH v4 04/12] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error ---
commit 886fa8092e9d55e67d82b2e1ee9d1125fcadac6d
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

This patch modifies the error unwind path in channel allocation to call
switchtec_dma_chan_stop() before disabling the channel. This prevents a
hardware use-after-free by ensuring the channel is successfully halted
before freeing the descriptor rings. If the halt fails, it skips freeing
the rings.

Link: https://lore.kernel.org/dmaengine/20260717223647.F0A051F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260727185101.D221C1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index cbbc9ad082471..25b988e9b4afb 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1047,6 +1047,10 @@ static int switchtec_dma_alloc_chan_resources(struct dma_chan *chan)
>  	swdma_chan->comp_ring_active = false;
>  	spin_unlock_bh(&swdma_chan->complete_lock);
>  err_disable_channel:
> +	if (switchtec_dma_chan_stop(swdma_chan)) {

[Severity: Critical]
This is a pre-existing issue, but does writing 0 to the DMA base registers
redirect active DMA transactions to physical address 0x0?

Looking at switchtec_dma_chan_stop(), if the halt times out, the registers
are still cleared:

drivers/dma/switchtec_dma.c:switchtec_dma_chan_stop() {
        rc = halt_channel(swdma_chan);
        ...
        if (rc)
                pci_err(pdev, "Channel %d halt timed out...");

        writel(0, &swdma_chan->mmio_chan_fw->sq_base_lo);
        ...
}

Since the channel is still unhalted and active, modifying these live
registers might cause ongoing or pending hardware DMA descriptor fetches to
target physical address 0x0.


[Severity: High]
This is a pre-existing issue, but can this call chain trigger softlockups
by holding a spinlock for up to 100ms?

switchtec_dma_chan_stop() calls halt_channel(), which acquires a spinlock
before polling:

drivers/dma/switchtec_dma.c:halt_channel() {
        spin_lock(&swdma_chan->hw_ctrl_lock);
        writeb(SWITCHTEC_CHAN_CTRL_HALT, &chan_hw->ctrl);
        ret = wait_for_chan_status(chan_hw, SWITCHTEC_CHAN_STS_HALTED, true);
        spin_unlock(&swdma_chan->hw_ctrl_lock);
}

And wait_for_chan_status() performs a 100ms atomic polling loop:

drivers/dma/switchtec_dma.c:wait_for_chan_status() {
        return readl_poll_timeout_atomic(&chan_hw->status, status,
                                         ..., 10, 100 * USEC_PER_MSEC);
}

Holding the spinlock disables preemption for the duration of the timeout,
which might cause severe scheduling latency spikes.

> +		disable_channel(swdma_chan);
> +		return rc;

[Severity: High]
When returning here, the descriptors are intentionally leaked to prevent a
hardware use-after-free, but do the pointers in swdma_chan remain populated?

If a subsequent allocation fails in switchtec_dma_alloc_desc(), it will call
switchtec_dma_free_desc() which frees these exact pointers while the hardware
might still be accessing them. 

If the allocation succeeds, the pointers will be overwritten:

drivers/dma/switchtec_dma.c:switchtec_dma_alloc_desc() {
        swdma_chan->hw_sq = dma_alloc_coherent(..., GFP_NOWAIT);
}

Could this lead to a use-after-free on a subsequent allocation error, or a
permanent memory leak if the pointers are overwritten?

> +	}
>  	disable_channel(swdma_chan);
>  err_free_desc:
>  	switchtec_dma_free_desc(swdma_chan);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=4

Re: [PATCH v4 05/12] dmaengine: switchtec-dma: fix channel leak on registration failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The newly added `switchtec_dma_chans_free()` modifies `dma_dev->channels` locklessly and frees channel objects while the channel status ISR is active, leading to list corruption and Use-After-Free.

Pre-existing issues:
- [Critical] Use-After-Free of `swdma_dev` in `switchtec_dma_remove()` after calling `dma_async_device_unregister()`.
- [High] The `chan_status_irq` is requested before the `dma_dev->channels` list is initialized, leading to a NULL pointer dereference if the ISR runs early.
- [High] The error path in `switchtec_dma_create()` incorrectly uses `free_irq()` instead of `pci_free_irq()`, and uses the flawed condition `if (swdma_dev->chan_status_irq)`.
--

--- Patch [5]: [PATCH v4 05/12] dmaengine: switchtec-dma: fix channel leak on registration failure ---
commit a8d5be8a7824da6cc78512656e71020f75154f88
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix channel leak on registration failure

switchtec_dma_chans_release() is called in three places but the
underlying memory is not freed in all of those places. In order to
clean this up, introduce a switchtec_dma_chans_free() helper that
will free the memory.

Link: https://lore.kernel.org/dmaengine/20260717223024.9BB8A1F000E9@smtp.kernel.org
Link: https://lore.kernel.org/dmaengine/20260727190604.397DB1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 25b988e9b4afb..48da269198f3f 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1188,6 +1188,18 @@ static int switchtec_dma_chans_release(struct pci_dev *pdev,
>  	return 0;
>  }
>  
> +static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < swdma_dev->chan_cnt; i++) {
> +		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
> +		kfree(swdma_dev->swdma_chans[i]);
> +	}
> +
> +	kfree(swdma_dev->swdma_chans);
> +}

[ ... ]

> @@ -1233,12 +1243,8 @@ static void switchtec_dma_release(struct dma_device *dma_dev)
>  {
>  	struct switchtec_dma_dev *swdma_dev =
>  		container_of(dma_dev, struct switchtec_dma_dev, dma_dev);
> -	int i;
>  
> -	for (i = 0; i < swdma_dev->chan_cnt; i++)
> -		kfree(swdma_dev->swdma_chans[i]);
> -
> -	kfree(swdma_dev->swdma_chans);
> +	switchtec_dma_chans_free(swdma_dev);
>  
>  	put_device(dma_dev->dev);
>  	kfree(swdma_dev);
>  }

[Severity: Critical]
This is a pre-existing issue, but could this lead to a use-after-free when
the driver is unloaded?

If dma_async_device_unregister() is called in switchtec_dma_remove(), it will
drop the reference to the DMA engine device. If there are no active clients,
the release callback switchtec_dma_release() is synchronously invoked, which
calls kfree(swdma_dev) here.

Immediately after the unregister call returns, switchtec_dma_remove() calls
iounmap(swdma_dev->bar), dereferencing the freed swdma_dev pointer:

switchtec_dma_remove() {
    ...
    dma_async_device_unregister(&swdma_dev->dma_dev);
        // eventually calls switchtec_dma_release() -> kfree(swdma_dev)
    iounmap(swdma_dev->bar);
    ...
}

[ ... ]

> @@ -1329,6 +1335,7 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  
>  err_chans_release_exit:
>  	switchtec_dma_chans_release(pdev, swdma_dev);
> +	switchtec_dma_chans_free(swdma_dev);

[Severity: High]
Does this introduce a race with the active interrupt handler?

In switchtec_dma_create(), the error path calls switchtec_dma_chans_free()
before disabling the interrupt:

err_chans_release_exit:
    switchtec_dma_chans_release(pdev, swdma_dev);
    switchtec_dma_chans_free(swdma_dev);
    
err_exit:
    if (swdma_dev->chan_status_irq)
        free_irq(...);

If the interrupt fires concurrently, switchtec_dma_chan_status_isr() will
locklessly iterate over dma_dev->channels. At the same time,
switchtec_dma_chans_free() calls list_del() and kfree() on those channel
objects, which can cause list corruption or a use-after-free in the ISR.

>  err_exit:
>  	if (swdma_dev->chan_status_irq)

[Severity: High]
This is a pre-existing issue, but are we using the correct function and
condition to free the IRQ here?

chan_status_irq stores the MSI-X vector index (e.g., 0, 1), not the system IRQ
number, and it is requested using pci_request_irq().

Calling free_irq(swdma_dev->chan_status_irq, swdma_dev) passes the vector
index as the system IRQ, attempting to free an unrelated system interrupt
instead of the actual hardware interrupt.

Also, checking if (swdma_dev->chan_status_irq) will skip freeing the IRQ
entirely if the vector index happens to be 0. Should this use pci_free_irq()
and check against a valid vector indicator like >= 0 instead?

[Severity: High]
This is another pre-existing issue, but is it possible for the interrupt
handler to run before the channels list is initialized?

In switchtec_dma_create(), swdma_dev is allocated with kzalloc_obj(), which
leaves the dma_dev->channels list head zeroed (next == NULL). The code then
calls pci_request_irq(), enabling the interrupt.

The list head isn't initialized until INIT_LIST_HEAD(&dma->channels) is called
later inside switchtec_dma_chans_enumerate().

If an interrupt fires in this window, switchtec_dma_chan_status_isr() executes
list_for_each_entry() on the zeroed list head. The loop macro evaluates the
position from head->next (NULL), bypassing the termination condition and
dereferencing the pointer, which can cause a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=5

Re: [PATCH v4 08/12] dmaengine: switchtec-dma: disable channels before freeing on registration failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The global channel status IRQ is requested too early in `switchtec_dma_create()`, exposing a NULL pointer dereference and race condition in the ISR.
- [High] Use-after-free of `swdma_dev` in `switchtec_dma_remove()` due to synchronous free in the release callback.
- [High] Missing NULL check after `ioremap()` leads to a potential NULL pointer dereference.
- [High] Missing `put_device()` in the error path of `switchtec_dma_create()` causes a reference count leak of the PCI device.
--

--- Patch [8]: [PATCH v4 08/12] dmaengine: switchtec-dma: disable channels before freeing on registration failure ---
commit a7ad01b09e2b3565af60150b4c01bec6d5cae7ad
Author: Logan Gunthorpe <logang@deltatee.com>

    dmaengine: switchtec-dma: disable channels before freeing on registration failure
    
    When switchtec_dma_create() fails after channels have been added to
    dma_dev->channels (either from switchtec_dma_chans_enumerate()'s own
    error path, or from dma_async_device_register() failing), the channel
    status IRQ is still live while the channels are released and freed,
    so it can fire mid-teardown against channels that are only partially
    torn down.

    Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org
    Link: https://lore.kernel.org/dmaengine/20260721162822.05CDD1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 6ab2c62b52fd8..ecf3a44d4dcc4 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1198,6 +1198,15 @@ static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
>  	kfree(swdma_dev->swdma_chans);
>  }
>  
> +static void switchtec_dma_chans_disable(struct pci_dev *pdev,
> +					struct switchtec_dma_dev *swdma_dev)
> +{
> +	if (swdma_dev->chan_status_irq >= 0) {
> +		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
> +		swdma_dev->chan_status_irq = -1;
> +	}
> +}
> +

[Severity: High]
This isn't a bug introduced by this patch, but since we're fixing IRQ
teardown here, is it possible the IRQ is requested too early during setup?

Looking at switchtec_dma_create(), the IRQ is requested before the channel
list is initialized:

switchtec_dma_create()
    rc = pci_request_irq(pdev, irq, NULL, switchtec_dma_chan_status_isr, ...);

If an interrupt fires immediately (e.g., from hardware state left by a
previous boot, or a spurious interrupt), the ISR will execute and iterate
over uninitialized memory:

switchtec_dma_chan_status_isr()
    list_for_each_entry(chan, &dma_dev->channels, device_node)

Could this cause a NULL pointer dereference in the ISR?

[ ... ]

> @@ -1334,6 +1344,7 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	return 0;
>  
>  err_chans_release_exit:
> +	switchtec_dma_chans_disable(pdev, swdma_dev);
>  	switchtec_dma_chans_release(pdev, swdma_dev);
>  	switchtec_dma_chans_free(swdma_dev);
>  

[Severity: High]
This is a pre-existing issue, but does this error path leak the PCI device
reference?

Earlier in switchtec_dma_create(), a reference is taken:

switchtec_dma_create()
    dma->dev = get_device(&pdev->dev);

When jumping to err_chans_release_exit or err_exit, the code does not seem
to call put_device() to clean up this reference.


[Severity: High]
This is another pre-existing issue, but looking at the earlier portion of
switchtec_dma_create(), could a missing NULL check lead to a crash?

switchtec_dma_create()
    swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
                             pci_resource_len(pdev, 0));

    RCU_INIT_POINTER(swdma_dev->pdev, pdev);

If ioremap() fails and returns NULL, subsequent MMIO accesses using
swdma_dev->bar will unconditionally dereference it. Should there be a
validation check here?


[Severity: High]
This is also a pre-existing issue, but does switchtec_dma_remove() have a
use-after-free risk during device teardown?

Looking at switchtec_dma_remove():

switchtec_dma_remove()
    dma_async_device_unregister(&swdma_dev->dma_dev);
    iounmap(swdma_dev->bar);

If dma_async_device_unregister() drops the last reference, it will
synchronously call switchtec_dma_release(), which frees swdma_dev:

switchtec_dma_release()
    kfree(swdma_dev);

Is it safe to pass swdma_dev->bar to iounmap() after the unregister call has
potentially freed swdma_dev?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=8

Re: [PATCH v4 07/12] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free of `swdma_dev` in `switchtec_dma_remove()` due to teardown ordering.
- [High] Missing validation of `ioremap()` return value leads to NULL pointer dereference.
- [High] IRQ handler registered before data structures are initialized leading to a NULL pointer dereference.
- [High] Unvalidated hardware register `chan_cnt` leads to out-of-bounds MMIO accesses.
--

--- Patch [7]: [PATCH v4 07/12] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error ---
commit 52eb1f3ef1524cf69a5b20f4b681533c108b4f8f
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

This commit fixes the cleanup of chan_status_irq in the create() error
path. It properly initializes the irq index to -1 and uses pci_free_irq()
instead of free_irq() to ensure the correct Linux IRQ is freed.

Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 63e60de9f86dc..6ab2c62b52fd8 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1262,6 +1262,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	if (!swdma_dev)
>  		return -ENOMEM;
>  
> +	swdma_dev->chan_status_irq = -1;
> +
>  	swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
>  				 pci_resource_len(pdev, 0));
>  

[Severity: High]
This is a pre-existing issue, but is it possible for ioremap() to fail here?

If the mapping fails due to exhausted vmalloc space, it returns a NULL pointer.
Shortly after this, the code calls readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC),
which would result in an unconditional NULL pointer dereference.

Should there be a check for swdma_dev->bar being NULL before proceeding?

> @@ -1336,8 +1338,8 @@ static int switchtec_dma_create(struct pci_dev *pdev)
>  	switchtec_dma_chans_free(swdma_dev);
>  
>  err_exit:
> -	if (swdma_dev->chan_status_irq)
> -		free_irq(swdma_dev->chan_status_irq, swdma_dev);
> +	if (swdma_dev->chan_status_irq >= 0)
> +		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
>  
>  	iounmap(swdma_dev->bar);
>  	kfree(swdma_dev);

While reviewing this patch, I noticed a few other pre-existing issues in the
surrounding code:

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to an
uninitialized list dereference if an interrupt fires early?

In switchtec_dma_create(), pci_request_irq() is called and enables
switchtec_dma_chan_status_isr before the dma->channels list is initialized
by INIT_LIST_HEAD().

If an interrupt fires during this window, switchtec_dma_chan_status_isr() will
iterate over the zero-initialized dma_dev->channels list:

switchtec_dma_chan_status_isr()
    list_for_each_entry(chan, &dma_dev->channels, device_node) {

Since swdma_dev was allocated with kzalloc_obj, the next pointer is NULL,
which would cause an invalid pointer dereference when evaluating
chan->dev->device.

Should the IRQ be requested after the channel list is fully initialized?

[Severity: High]
This isn't a bug introduced by this patch, but does this code trust the
hardware register chan_cnt too much?

The driver reads chan_cnt directly from SWITCHTEC_REG_CHAN_CNT:

switchtec_dma_create()
    chan_cnt = readl(swdma_dev->bar + SWITCHTEC_REG_CHAN_CNT);

This count is then used in switchtec_dma_chans_enumerate() to calculate MMIO
offsets via pointer arithmetic:

switchtec_dma_chan_init()
    swdma_chan->mmio_chan_fw = swdma_dev->bar + SWITCHTEC_DMAC_CHAN_CFG_STS_OFFSET +
                               i * SWITCHTEC_DMA_CHAN_FW_REGS_SIZE;

If a faulty or maliciously crafted PCI endpoint provides a dangerously large
chan_cnt, could this cause the driver to read and write far outside the safely
allocated ioremap region?

Should chan_cnt be validated against the actual size of the mapped PCI BAR?

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
during device teardown?

During module unload or device removal, switchtec_dma_remove() calls
dma_async_device_unregister(&swdma_dev->dma_dev).

This synchronously drops the reference count of the DMA device to 0 if there
are no active clients, which triggers the switchtec_dma_release() callback.

switchtec_dma_release()
    kfree(swdma_dev);

Immediately following this, switchtec_dma_remove() continues teardown:

switchtec_dma_remove()
    dma_async_device_unregister(&swdma_dev->dma_dev);
    iounmap(swdma_dev->bar);

Because swdma_dev has already been freed, does accessing the bar member for
unmapping result in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=7

Re: [PATCH v4 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Improper teardown order in `switchtec_dma_remove` enables race conditions and use-after-free with dmaengine clients.
- [High] The driver holds a spinlock across dmaengine API boundaries, causing guaranteed deadlocks if multiple descriptors are batched.
--

--- Patch [2]: [PATCH v4 02/12] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources ---
commit c17dbf5966e1848ec7a26210950a24038c80d7e5
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources

This commit fixes a resource leak in switchtec_dma_alloc_chan_resources()
where descriptor rings and coherent memory weren't freed on failure. It adds
an error-unwind path to properly disable the channel and clean up active
flags.

Link: https://lore.kernel.org/dmaengine/20260707165555.350951F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index a4a7d66d042de..f77da31aeb65e 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does this teardown order
allow active dmaengine clients to access destroyed hardware state or wait
on freed IRQs?

drivers/dma/switchtec_dma.c:switchtec_dma_remove() {
...
	switchtec_dma_chans_release(pdev, swdma_dev);

	rcu_assign_pointer(swdma_dev->pdev, NULL);
	synchronize_rcu();

	pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);

	pci_free_irq_vectors(pdev);

	dma_async_device_unregister(&swdma_dev->dma_dev);
...
}

Because switchtec_dma_remove() disables channels and frees IRQs before
calling dma_async_device_unregister(), a concurrent client could attempt
to allocate or submit jobs to the exposed device before unregistration
completes.

[Severity: High]
This is also a pre-existing issue, but does returning to a generic API
client with a spinlock held cause deadlocks?

drivers/dma/switchtec_dma.c:switchtec_dma_prep_desc() {
...
	/* return with the lock held, it will be released in tx_submit */

	return &desc->txd;
}

If a client batches multiple descriptors (e.g., calling
dmaengine_prep_dma_memcpy() twice for the same channel before submit),
wouldn't the second call attempt to recursively acquire the same
non-recursive submit_lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=2

Re: [PATCH v4 06/12] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Leaking DMA descriptors and ring buffers when channel stop fails (e.g., during device hot-unplug or module unload).

Pre-existing issues:
- [High] Hardware resources (IRQs and tasklets) are released before the DMA device is unregistered, allowing clients to request or use unbacked channels.
--

--- Patch [6]: [PATCH v4 06/12] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void ---
commit 4ae99d5104edb85994c22547e64122fc62df38cb
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

switchtec_dma_chans_release() always returned 0, and no caller
checked it.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 48da269198f3f..63e60de9f86dc 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1177,15 +1177,13 @@ static int switchtec_dma_chan_free(struct pci_dev *pdev,
>  	return 0;
>  }
>  
> -static int switchtec_dma_chans_release(struct pci_dev *pdev,
> -				       struct switchtec_dma_dev *swdma_dev)
> +static void switchtec_dma_chans_release(struct pci_dev *pdev,
> +					struct switchtec_dma_dev *swdma_dev)
>  {
>  	int i;
>  
>  	for (i = 0; i < swdma_dev->chan_cnt; i++)
>  		switchtec_dma_chan_free(pdev, swdma_dev->swdma_chans[i]);
> -
> -	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but hardware resources (IRQs and tasklets) are
released before the DMA device is unregistered, allowing clients to request or
use unbacked channels.

In switchtec_dma_remove(), switchtec_dma_chans_release() is called first, which
frees all per-channel IRQs and kills their descriptor tasklets:

drivers/dma/switchtec_dma.c:switchtec_dma_remove() {
	...
	switchtec_dma_chans_release(pdev, swdma_dev);
	rcu_assign_pointer(swdma_dev->pdev, NULL);
	synchronize_rcu();
	pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
	pci_free_irq_vectors(pdev);
	dma_async_device_unregister(&swdma_dev->dma_dev);
}

Could this cause active DMA transfers to hang forever without completion, since
the DMA device is not unregistered from the core until much later?

Because the device is still in the DMA engine channel table, could a client
theoretically request a new channel in this window?

[Severity: High]
Does this code leak DMA descriptors and ring buffers when channel stop fails?

In switchtec_dma_free_chan_resources(), if switchtec_dma_chan_stop() returns a
non-zero error (like -ENODEV during hot-unplug), the cleanup of descriptor
memory is completely skipped:

drivers/dma/switchtec_dma.c:switchtec_dma_free_chan_resources() {
	...
	rc = switchtec_dma_chan_stop(swdma_chan);
	switchtec_dma_abort_desc(swdma_chan, 0);
	if (!rc)
		switchtec_dma_free_desc(swdma_chan);
}

During switchtec_dma_remove(), swdma_dev->pdev is set to NULL before
dma_async_device_unregister() is called. When dma_async_device_unregister()
subsequently cleans up active channels, it invokes
switchtec_dma_free_chan_resources().

Because pdev is NULL, switchtec_dma_chan_stop() returns -ENODEV. The code checks
if (!rc) and skips switchtec_dma_free_desc(), leaking the resources.

A similar issue appears to exist in the error path of
switchtec_dma_alloc_chan_resources() if the channel stop returns an error:

drivers/dma/switchtec_dma.c:switchtec_dma_alloc_chan_resources() {
	...
err_disable_channel:
	if (switchtec_dma_chan_stop(swdma_chan)) {
		disable_channel(swdma_chan);
		return rc;
	}
	disable_channel(swdma_chan);
err_free_desc:
	switchtec_dma_free_desc(swdma_chan);
}

Can this leak memory for every active channel on standard device teardown or
hot-unplug?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=6

Re: [PATCH v4 09/12] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] DMA coherent buffers and descriptor structures are permanently leaked in `switchtec_dma_free_chan_resources()` if a channel is released after device hot-unplug.
- [High] Missing NULL check for the result of `ioremap()` in `switchtec_dma_create()`.
--

--- Patch [9]: [PATCH v4 09/12] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() ---
commit 07d288b1e2418ad94452e6749769e4b45a0f0978
Author: Logan Gunthorpe <logang@deltatee.com>
Subject: dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

This commit addresses a use-after-free by caching the bar address before
unregistering the device. During dma_async_device_unregister(), the last
reference to dma_dev can be dropped, synchronously freeing swdma_dev before
the original code used it for iounmap().

Link: https://lore.kernel.org/dmaengine/20260717223431.625EE1F000E9@smtp.kernel.org

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index ecf3a44d4dcc4..babb2bcb62084 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1393,6 +1393,7 @@ static int switchtec_dma_probe(struct pci_dev *pdev,
>  static void switchtec_dma_remove(struct pci_dev *pdev)
>  {
>  	struct switchtec_dma_dev *swdma_dev = pci_get_drvdata(pdev);
> +	void __iomem *bar = swdma_dev->bar;
>  
>  	switchtec_dma_chans_release(pdev, swdma_dev);
>  
> @@ -1405,7 +1406,7 @@ static void switchtec_dma_remove(struct pci_dev *pdev)
>  
>  	dma_async_device_unregister(&swdma_dev->dma_dev);
>  
> -	iounmap(swdma_dev->bar);
> +	iounmap(bar);
>  	pci_release_mem_regions(pdev);
>  	pci_disable_device(pdev);
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a missing NULL check for the
result of ioremap() in switchtec_dma_create():

drivers/dma/switchtec_dma.c:switchtec_dma_create() {
    swdma_dev->bar = ioremap(pci_resource_start(pdev, 0),
                             pci_resource_len(pdev, 0));
    ...
    irq = readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC);
}

If ioremap() fails and returns NULL, could this result in a NULL pointer
dereference when we try to read from it?

[Severity: High]
This is a pre-existing issue, but does this code permanently leak DMA
coherent buffers and descriptor structures if a channel is released after
device hot-unplug?

drivers/dma/switchtec_dma.c:halt_channel() {
    pdev = rcu_dereference(swdma_chan->swdma_dev->pdev);
    if (!pdev) {
        ret = -ENODEV;
        goto unlock_and_exit;
    }
}

When pdev is NULL, halt_channel() returns -ENODEV, which propagates to
switchtec_dma_chan_stop(). 

Then in switchtec_dma_free_chan_resources():

    rc = switchtec_dma_chan_stop(swdma_chan);
    switchtec_dma_abort_desc(swdma_chan, 0);
    if (!rc)
        switchtec_dma_free_desc(swdma_chan);

Since switchtec_dma_chan_stop() returned -ENODEV (which is non-zero), the
call to switchtec_dma_free_desc() is skipped. While skipping cleanup might
make sense if hardware times out, is this intentional for hot-unplug cases
where the hardware is physically absent?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=9

Re: [PATCH v4 11/12] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The embedded kobject in `struct ioatdma_chan` lacks a `.release` method, decoupling its lifecycle from the containing object and creating a potential use-after-free via sysfs.
--

--- Patch [11]: [PATCH v4 11/12] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() ---
commit af292acf66777b7c34a85fccbbbae1f36425b555
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: ioat: use sysfs_emit() in per-channel sysfs show()

Convert the sprintf() calls in the per-channel sysfs attribute show()
functions to sysfs_emit().

> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
> index e796ddb5383f8..976134df81084 100644
> --- a/drivers/dma/ioat/sysfs.c
> +++ b/drivers/dma/ioat/sysfs.c
[ ... ]
> @@ -135,7 +135,7 @@ static ssize_t intr_coalesce_show(struct dma_chan *c, char *page)
>  {
>  	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>  
> -	return sprintf(page, "%d\n", ioat_chan->intr_coalesce);
> +	return sysfs_emit(page, "%d\n", ioat_chan->intr_coalesce);
>  }
>  
>  static ssize_t intr_coalesce_store(struct dma_chan *c, const char *page,

[Severity: High]
This is a pre-existing issue, but since we are modifying sysfs behavior here,
does the embedded kobject in struct ioatdma_chan lack a proper lifecycle?

Looking at ioat_ktype further down in this file, it does not define a release
callback:

const struct kobj_type ioat_ktype = {
	.sysfs_ops = &ioat_sysfs_ops,
	.default_groups = ioat_groups,
};

If a sysfs file is kept open by a userspace process and the driver is unbound,
release_ioatdma() unconditionally frees the channel:

drivers/dma/ioat/init.c:release_ioatdma() {
    ...
	for (i = 0; i < IOAT_MAX_CHANS; i++)
		kfree(d->idx[i]);
    ...
}

Could this result in a use-after-free when the userspace process eventually
closes the file descriptor, dropping the kernfs reference and causing
kobject_put() to dereference the freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=11

Re: [PATCH v4 12/12] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Lock is held across DMA API boundaries (`prep` to `submit`), preventing descriptor batching and causing deadlocks.
- [High] Recursive deadlock when invoking client callbacks from `plx_dma_process_desc`.
- [High] Softirq preemption deadlock in `plx_dma_process_desc` due to missing `spin_lock_bh`.
- [High] 1-Second busy-wait loop inside an RCU read-side critical section.
- [Medium] Memory leak of `desc_ring` array and descriptors in `plx_dma_alloc_chan_resources` error path.
--

--- Patch [12]: [PATCH v4 12/12] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() ---
commit 255ac134d3a6a355b0f9fd6a9df9b4e6a75d09d7
Author: Logan Gunthorpe <logang@deltatee.com>
Subject: dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()

plx_dma_create() registered the interrupt handler with request_irq()
before initializing plxdev->bar. If the device raised an interrupt in
that window, plx_dma_isr() would dereference the still-NULL bar.
This patch moves the bar assignment ahead of request_irq() so everything the
handler can touch is initialized before it can run.

Link: https://lore.kernel.org/all/20260610112121.676561-1-jjy600901@snu.ac.kr

> diff --git a/drivers/dma/plx_dma.c b/drivers/dma/plx_dma.c
> index 84941a918b01f..409898e92c320 100644
> --- a/drivers/dma/plx_dma.c
> +++ b/drivers/dma/plx_dma.c

[ ... ]

> @@ -504,17 +504,17 @@ static int plx_dma_create(struct pci_dev *pdev)
>  	if (!plxdev)
>  		return -ENOMEM;
>  
> -	rc = request_irq(pci_irq_vector(pdev, 0), plx_dma_isr, 0,
> -			 KBUILD_MODNAME, plxdev);
> -	if (rc)
> -		goto free_plx;
> -
>  	spin_lock_init(&plxdev->ring_lock);
>  	tasklet_setup(&plxdev->desc_task, plx_dma_desc_task);
>  
>  	RCU_INIT_POINTER(plxdev->pdev, pdev);
>  	plxdev->bar = pcim_iomap_table(pdev)[0];
>  
> +	rc = request_irq(pci_irq_vector(pdev, 0), plx_dma_isr, 0,
> +			 KBUILD_MODNAME, plxdev);
> +	if (rc)
> +		goto free_plx;
> +
>  	dma = &plxdev->dma_dev;

[ ... ]

While reviewing this patch, I noticed a few pre-existing issues in the same
file that might be worth addressing:

In drivers/dma/plx_dma.c:plx_dma_prep_memcpy():
> 	spin_lock_bh(&plxdev->ring_lock);
> ...
> 	return &plxdesc->txd;

In drivers/dma/plx_dma.c:plx_dma_tx_submit():
> 	spin_unlock_bh(&plxdev->ring_lock);
> 
> 	return cookie;

[Severity: High]
This is a pre-existing issue, but does holding ring_lock across DMA API
boundaries prevent descriptor batching?

If a client prepares two consecutive transfers before submitting them, wouldn't
the second plx_dma_prep_memcpy() call recursively attempt to acquire the
non-recursive ring_lock held by the first call, causing an immediate deadlock?

In drivers/dma/plx_dma.c:plx_dma_process_desc():
> 	spin_lock(&plxdev->ring_lock);
> 
> 	while (plxdev->tail != plxdev->head) {
> ...
> 		dmaengine_desc_get_callback_invoke(&desc->txd, &res);
> ...
> 	}
> 
> 	spin_unlock(&plxdev->ring_lock);

[Severity: High]
This is a pre-existing issue, but is it safe to invoke client callbacks while
holding ring_lock?

If a completion callback attempts to prepare a new transfer via
dmaengine_prep_dma_memcpy(), it will invoke plx_dma_prep_memcpy() which
attempts to acquire spin_lock_bh(&plxdev->ring_lock).

Would this cause an immediate recursive deadlock?

In drivers/dma/plx_dma.c:plx_dma_process_desc():
> 	spin_lock(&plxdev->ring_lock);

[Severity: High]
This is a pre-existing issue, but could acquiring ring_lock without disabling
softirqs lead to a preemption deadlock?

If plx_dma_process_desc() is called from process context via
plx_dma_tx_status(), a hardware interrupt could preempt the thread while the
lock is held and schedule the tasklet.

Would the softirq tasklet then spin forever in plx_dma_desc_task() trying
to acquire ring_lock?

In drivers/dma/plx_dma.c:plx_dma_stop():
> 	rcu_read_lock();
> 	if (!rcu_dereference(plxdev->pdev)) {
> 		rcu_read_unlock();
> 		return;
> 	}
> 
> 	__plx_dma_stop(plxdev);
> 
> 	rcu_read_unlock();

In drivers/dma/plx_dma.c:__plx_dma_stop():
> 	unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> ...
> 	while (!time_after(jiffies, timeout)) {
> 		val = readl(plxdev->bar + PLX_REG_CTRL);
> 		if (val & PLX_REG_CTRL_GRACEFUL_PAUSE_DONE)
> 			break;
> 
> 		cpu_relax();
> 	}

[Severity: High]
This is a pre-existing issue, but does __plx_dma_stop() spin for up to a second
inside an RCU read-side critical section?

Since plx_dma_stop() takes rcu_read_lock() and calls __plx_dma_stop(), waiting
for up to 1000 milliseconds using cpu_relax() could cause massive scheduling
latency spikes.

Could this explicitly block RCU grace periods and preemptions on
non-PREEMPT_RCU systems, potentially triggering RCU stall panics?

In drivers/dma/plx_dma.c:plx_dma_alloc_chan_resources():
> 	rc = plx_dma_alloc_desc(plxdev);
> 	if (rc)
> 		goto out_free_hw_ring;
> 
> 	rcu_read_lock();
> 	if (!rcu_dereference(plxdev->pdev)) {
> 		rcu_read_unlock();
> 		rc = -ENODEV;
> 		goto out_free_hw_ring;
> 	}
> ...
> out_free_hw_ring:
> 	dma_free_coherent(plxdev->dma_dev.dev, ring_sz, plxdev->hw_ring,
> 			  plxdev->hw_ring_dma);
> 	return rc;

[Severity: Medium]
This is a pre-existing issue, but does this error path leak the desc_ring array
and its associated descriptors?

If the check for rcu_dereference(plxdev->pdev) fails, it jumps to
out_free_hw_ring.

Since this only frees the DMA coherent memory hw_ring, are the allocated
descriptors completely leaked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=12

Re: [PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free (UAF) in shared IRQ handler due to unsafe mixing of manual memory management and devres IRQ registration.
- [High] Global IDA resource leak in the core DMA channel registration error path.
- [Medium] Race condition exposing DMA channels to userspace before their sysfs attributes are fully created.
--

--- Patch [10]: [PATCH v4 10/12] dmaengine: ioat: disable relaxed ordering before registering the device ---
commit f2b93cc9001f8f226c2e33f07702973d69e902de
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: ioat: disable relaxed ordering before registering the device

ioat3_dma_probe() disabled PCIe relaxed ordering after calling
dma_async_device_register(), so if an error occurs and the code jumps
to err_disable_interrupts, the function returns with a dangling registration.
This commit moves the capability read/write ahead of registration to fix
the issue.

Link: https://lore.kernel.org/dmaengine/20260707165906.249F41F000E9@smtp.kernel.org

> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 737496391109e..a57024c4b066e 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -1170,15 +1170,6 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  		       ioat_chan->reg_base + IOAT_DCACTRL_OFFSET);
>  	}
>  
> -	err = dma_async_device_register(&ioat_dma->dma_dev);
> -	if (err)
> -		goto err_disable_interrupts;
> -
> -	ioat_kobject_add(ioat_dma, &ioat_ktype);
> -
> -	if (dca)
> -		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> -
>  	/* disable relaxed ordering */
>  	err = pcie_capability_read_word(pdev, PCI_EXP_DEVCTL, &val16);
>  	if (err) {
> @@ -1194,6 +1185,15 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  		goto err_disable_interrupts;
>  	}
>  
> +	err = dma_async_device_register(&ioat_dma->dma_dev);

[Severity: High]
This is a pre-existing issue, but does dma_async_device_register() leak the
global dma_ida resource when it fails?

If dma_async_device_register() fails after allocating a global ID and
populating the channel list, the error path in drivers/dma/dmaengine.c handles
the cleanup:

	if (!device->chancnt) {
		ida_free(&dma_ida, device->dev_id);
		return rc;
	}

Because it skips ida_free() when chancnt is greater than zero, does this
permanently leak an ID from the global dma_ida pool every time registration
fails with populated channels?

> +	if (err)
> +		goto err_disable_interrupts;

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free risk
on this error path?

If ioat3_dma_probe() fails and the code takes the err_disable_interrupts
path, ioat_pci_probe() will manually free the primary device data structure.
However, because ioat_dma_setup_interrupts() registers a shared interrupt
using devm_request_irq(), the interrupt cleanup is deferred until after the
probe function completely returns.

Could a shared interrupt firing during this brief window invoke the handler,
causing ioat_dma_do_interrupt() to blindly dereference the already freed data
pointer?

> +
> +	ioat_kobject_add(ioat_dma, &ioat_ktype);

[Severity: Medium]
This is a pre-existing issue, but does this sequence expose DMA channels to
userspace before their sysfs attributes are fully created?

Because dma_async_device_register() completes core registration and sends
add uevents before ioat_kobject_add() appends the specific quickdata
sysfs attributes, can a userspace process reacting to the uevent attempt to
read device properties in the window before they are created?

> +
> +	if (dca)
> +		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> +
>  	if (ioat_dma->cap & IOAT_CAP_DPS)
>  		writeb(ioat_pending_level + 1,
>  		       ioat_dma->reg_base + IOAT_PREFETCH_LIMIT_OFFSET);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728171523.112244-1-logang@deltatee.com?part=10