[PATCH v3 00/11] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma

[PATCH v3 00/11] 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] (it is
relentless!).

I've fixed the issues reported with v1 and v2 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-rc4.

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

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 (11):
  dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()
  dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources
  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 | 84 +++++++++++++++++++++++++++----------
 4 files changed, 88 insertions(+), 46 deletions(-)


base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
-- 
2.47.3

[PATCH v3 01/11] 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")
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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Memory leak of DMA channel resources on error paths in switchtec_dma_alloc_chan_resources()
- [High] Memory leak of channel structures (swdma_chan) and pointer array on driver probe error
- [High] Incorrect IRQ teardown using free_irq() with a raw MSI-X vector index instead of pci_free_irq()
--

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

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

switchtec_dma_free_desc() frees swdma_chan->hw_sq, hw_cq, and every
desc_ring[] entry without clearing the pointers afterward. This patch
NULLs out each pointer as it's freed so a subsequent call is a no-op
for anything already released.

> 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;
> +	}
>  }
>  
>  static int switchtec_dma_alloc_desc(struct switchtec_dma_chan *swdma_chan)

[Severity: High]
This isn't a bug introduced by this patch, but does
switchtec_dma_alloc_chan_resources() leak DMA channel resources on error
paths?

If switchtec_dma_alloc_desc() succeeds but a subsequent step like
enable_channel() fails, the driver returns the error code immediately:

switchtec_dma_alloc_chan_resources() {
	...
	rc = enable_channel(swdma_chan);
	if (rc)
		return rc;
	...
}

Does this fail to call switchtec_dma_free_desc() to free the resources
successfully allocated earlier in the function? Since the dmaengine core
does not automatically invoke the cleanup routine for partial failures,
could these resources be permanently leaked?

[Severity: High]
This is a pre-existing issue, but I noticed a potential memory leak of
channel structures on driver probe error.

If switchtec_dma_chans_enumerate() fails partway through, the error path
iterates over initialized channels and calls switchtec_dma_chan_free(),
but it doesn't seem to free the individual channel elements:

switchtec_dma_chans_enumerate() {
	...
err_exit:
	for (i = 0; i < chan_cnt; i++)
		switchtec_dma_chan_free(pdev, swdma_dev->swdma_chans[i]);

	kfree(swdma_dev->swdma_chans);
	...
}

Furthermore, if dma_async_device_register() fails in switchtec_dma_create(),
does the error path forget to free the channel pointer array or the
structures it points to?

switchtec_dma_create() {
	...
err_chans_release_exit:
	switchtec_dma_chans_release(pdev, swdma_dev);
	...
}

[Severity: High]
This is a pre-existing issue, but is the driver using free_irq() incorrectly
with a raw vector index instead of pci_free_irq() during teardown?

If switchtec_dma_create() encounters an error after successfully requesting
the channel status IRQ, it branches to err_exit and calls free_irq():

switchtec_dma_create() {
	...
err_exit:
	if (swdma_dev->chan_status_irq)
		free_irq(swdma_dev->chan_status_irq, swdma_dev);
	...
}

Is swdma_dev->chan_status_irq holding the raw hardware index rather than
the Linux IRQ number? Could this incorrectly free an unrelated system
interrupt and leak the originally requested IRQ?

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

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

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:16PM -0600, Logan Gunthorpe wrote:
> 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")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 v3 02/11] 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")
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

Re: [PATCH v3 02/11] 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:

New issues:
- [High] DMA use-after-free due to missing channel halt in the allocation error path.

Pre-existing issues:
- [High] Race condition in driver teardown leading to MMIO use-after-free and kernel oops.
--

--- Patch [2]: [PATCH v3 02/11] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources ---
commit d5306db114dcdc27e3a80dfb2bb3a42a91ab9a3a
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
by adding an error-unwind path. The new path disables the channel, clears
the active flags, and frees the descriptor rings if an allocation fails.

Link: https://lore.kernel.org/dmaengine/20260707165555.350951F000E9@smtp.kernel.org
Fixes: 30eba9df76ad ("dmaengine: switchtec-dma: Implement hardware initialization and cleanup")

> 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

[ ... ]

> @@ -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);

[Severity: High]
If an error occurs in switchtec_dma_alloc_chan_resources() after
unhalt_channel() succeeds (for example, if the RCU pointer check fails),
the unwind path jumps to err_ring_inactive and falls through to
err_disable_channel.

Since disable_channel() only clears a software configuration bit and does
not explicitly issue a hardware halt command or wait for the hardware to
stop processing, could the hardware channel continue performing DMA accesses?

If so, does calling switchtec_dma_free_desc() immediately after
disable_channel() create a window for a DMA use-after-free, where the device
can still read or write into the newly freed coherent memory pages?

> +	return rc;
>  }

[Severity: High]
This is a pre-existing issue, but looking at the driver teardown sequence
in switchtec_dma_remove():

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);
    iounmap(swdma_dev->bar);
    ...
}

Because the resources and MMIO regions are dismantled before the device is
unregistered from the dmaengine core, is it possible for a concurrent
client to call dma_request_chan() and allocate a channel during this window?

If a concurrent allocation finishes right after the channel was freed,
could the client attempt to submit a DMA operation and trigger a kernel
oops when accessing the unmapped MMIO registers?

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

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

[Logan Gunthorpe]

On 2026-07-27 12:40, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] DMA use-after-free due to missing channel halt in the allocation error path.

Note: this issue I believe is fixed in patch 3 of this series.

Logan

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

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:17PM -0600, Logan Gunthorpe wrote:
> 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")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 v3 03/11] 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.

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
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index f77da31aeb65..107769cca772 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1040,6 +1040,7 @@ 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:
+	switchtec_dma_chan_stop(swdma_chan);
 	disable_channel(swdma_chan);
 err_free_desc:
 	switchtec_dma_free_desc(swdma_chan);
-- 
2.47.3

Re: [PATCH v3 03/11] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

[sashiko-bot]

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

New issues:
- [High] Incomplete fix / DMA Use-After-Free due to early return in switchtec_dma_chan_stop() on halt timeout.

Pre-existing issues:
- [High] Soft lockup and severe latency spikes due to 100ms atomic busy-waiting under a spinlock.
- [High] Extreme allocation brittleness due to requesting 32,768 descriptors with GFP_NOWAIT.
- [High] DMA Use-After-Free in switchtec_dma_free_chan_resources() due to incorrect teardown ordering.
- [High] Massive Use-After-Free of swdma_dev during driver removal.
--

--- Patch [3]: [PATCH v3 03/11] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error ---
commit 647e1927ea58080fcbb88ef03dd8a74aa7555178
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

The commit adds a call to switchtec_dma_chan_stop() on the error-unwind
path of switchtec_dma_alloc_chan_resources(). This aims to synchronously 
halt the channel and clear DMA base registers before freeing the
descriptors, preventing a potential use-after-free if the hardware 
continues accessing the rings.

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

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index f77da31aeb65e..107769cca772f 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1040,6 +1040,7 @@ 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:
> +	switchtec_dma_chan_stop(swdma_chan);
>  	disable_channel(swdma_chan);
>  err_free_desc:
>  	switchtec_dma_free_desc(swdma_chan);

[Severity: High]
Does this completely resolve the use-after-free if the hardware is 
unresponsive?

Looking at switchtec_dma_chan_stop(), if halt_channel() times out, the 
function returns early without clearing the DMA base registers:

drivers/dma/switchtec_dma.c:switchtec_dma_chan_stop() {
        rc = halt_channel(swdma_chan);
        if (rc)
                return;
        ...
        writel(0, &swdma_chan->mmio_chan_fw->sq_base_lo);
        ...
}

If that happens, execution continues to disable_channel() which does an
unflushed write, and then switchtec_dma_free_desc() unconditionally frees
the descriptors while the hardware might still be accessing them.


[Severity: High]
This is a pre-existing issue, but can this lead to soft lockups or severe 
latency spikes?

In halt_channel(), wait_for_chan_status() is called while holding a spinlock:

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);
        ...
}

wait_for_chan_status() uses readl_poll_timeout_atomic() with a 100ms timeout.
Is it safe to block preemption for up to 100 milliseconds if the hardware 
is slow to respond?


[Severity: High]
This isn't a bug introduced by this patch, but does this allocation strategy 
cause issues under memory pressure?

drivers/dma/switchtec_dma.c:switchtec_dma_alloc_desc() {
        for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++) {
                desc = kzalloc_obj(*desc, GFP_NOWAIT);
                if (!desc) {
        ...
}

SWITCHTEC_DMA_RING_SIZE is 32,768. Iteratively requesting 32,768 allocations 
with GFP_NOWAIT forbids sleeping and memory reclaim. Could this rapidly 
drain atomic memory pools and fail on slightly fragmented systems?


[Severity: High]
This is a pre-existing issue, but could this teardown ordering result in 
a use-after-free?

drivers/dma/switchtec_dma.c:switchtec_dma_free_chan_resources() {
        switchtec_dma_chan_stop(swdma_chan);
        switchtec_dma_abort_desc(swdma_chan, 0);
        switchtec_dma_free_desc(swdma_chan);
        
        disable_channel(swdma_chan);
        ...
}

If switchtec_dma_chan_stop() times out and fails to halt the channel, 
the descriptors are freed by switchtec_dma_free_desc() before the 
channel enable bit is cleared by disable_channel(). Could the active 
hardware continue executing and corrupt the freed descriptor memory?


[Severity: High]
This is a pre-existing issue, but does this sequence cause a use-after-free 
of swdma_dev?

drivers/dma/switchtec_dma.c:switchtec_dma_remove() {
        ...
        dma_async_device_unregister(&swdma_dev->dma_dev);
        
        iounmap(swdma_dev->bar);
        ...
}

If dma_async_device_unregister() drops the last reference to the DMA device,
it synchronously invokes the release callback, switchtec_dma_release(), 
which frees swdma_dev. Does this mean iounmap(swdma_dev->bar) will 
dereference the freed swdma_dev pointer?

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

Re: [PATCH v3 03/11] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

[Frank Li]

On Mon, Jul 27, 2026 at 06:51:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> New issues:
> - [High] Incomplete fix / DMA Use-After-Free due to early return in switchtec_dma_chan_stop() on halt timeout.
>

Is it true?

Frank
> Pre-existing issues:
> - [High] Soft lockup and severe latency spikes due to 100ms atomic busy-waiting under a spinlock.
> - [High] Extreme allocation brittleness due to requesting 32,768 descriptors with GFP_NOWAIT.
> - [High] DMA Use-After-Free in switchtec_dma_free_chan_resources() due to incorrect teardown ordering.
> - [High] Massive Use-After-Free of swdma_dev during driver removal.
> --
>
> --- Patch [3]: [PATCH v3 03/11] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error ---
> commit 647e1927ea58080fcbb88ef03dd8a74aa7555178
> Author: Logan Gunthorpe <logang@deltatee.com>
>
> dmaengine: switchtec-dma: halt channel on alloc_chan_resources error
>
> The commit adds a call to switchtec_dma_chan_stop() on the error-unwind
> path of switchtec_dma_alloc_chan_resources(). This aims to synchronously
> halt the channel and clear DMA base registers before freeing the
> descriptors, preventing a potential use-after-free if the hardware
> continues accessing the rings.
>
> Link: https://lore.kernel.org/dmaengine/20260717223647.F0A051F000E9@smtp.kernel.org
>
> > diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> > index f77da31aeb65e..107769cca772f 100644
> > --- a/drivers/dma/switchtec_dma.c
> > +++ b/drivers/dma/switchtec_dma.c
> > @@ -1040,6 +1040,7 @@ 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:
> > +	switchtec_dma_chan_stop(swdma_chan);
> >  	disable_channel(swdma_chan);
> >  err_free_desc:
> >  	switchtec_dma_free_desc(swdma_chan);
>
> [Severity: High]
> Does this completely resolve the use-after-free if the hardware is
> unresponsive?
>
> Looking at switchtec_dma_chan_stop(), if halt_channel() times out, the
> function returns early without clearing the DMA base registers:
>
> drivers/dma/switchtec_dma.c:switchtec_dma_chan_stop() {
>         rc = halt_channel(swdma_chan);
>         if (rc)
>                 return;
>         ...
>         writel(0, &swdma_chan->mmio_chan_fw->sq_base_lo);
>         ...
> }
>
> If that happens, execution continues to disable_channel() which does an
> unflushed write, and then switchtec_dma_free_desc() unconditionally frees
> the descriptors while the hardware might still be accessing them.
>
>
> [Severity: High]
> This is a pre-existing issue, but can this lead to soft lockups or severe
> latency spikes?
>
> In halt_channel(), wait_for_chan_status() is called while holding a spinlock:
>
> 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);
>         ...
> }
>
> wait_for_chan_status() uses readl_poll_timeout_atomic() with a 100ms timeout.
> Is it safe to block preemption for up to 100 milliseconds if the hardware
> is slow to respond?
>
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this allocation strategy
> cause issues under memory pressure?
>
> drivers/dma/switchtec_dma.c:switchtec_dma_alloc_desc() {
>         for (i = 0; i < SWITCHTEC_DMA_RING_SIZE; i++) {
>                 desc = kzalloc_obj(*desc, GFP_NOWAIT);
>                 if (!desc) {
>         ...
> }
>
> SWITCHTEC_DMA_RING_SIZE is 32,768. Iteratively requesting 32,768 allocations
> with GFP_NOWAIT forbids sleeping and memory reclaim. Could this rapidly
> drain atomic memory pools and fail on slightly fragmented systems?
>
>
> [Severity: High]
> This is a pre-existing issue, but could this teardown ordering result in
> a use-after-free?
>
> drivers/dma/switchtec_dma.c:switchtec_dma_free_chan_resources() {
>         switchtec_dma_chan_stop(swdma_chan);
>         switchtec_dma_abort_desc(swdma_chan, 0);
>         switchtec_dma_free_desc(swdma_chan);
>
>         disable_channel(swdma_chan);
>         ...
> }
>
> If switchtec_dma_chan_stop() times out and fails to halt the channel,
> the descriptors are freed by switchtec_dma_free_desc() before the
> channel enable bit is cleared by disable_channel(). Could the active
> hardware continue executing and corrupt the freed descriptor memory?
>
>
> [Severity: High]
> This is a pre-existing issue, but does this sequence cause a use-after-free
> of swdma_dev?
>
> drivers/dma/switchtec_dma.c:switchtec_dma_remove() {
>         ...
>         dma_async_device_unregister(&swdma_dev->dma_dev);
>
>         iounmap(swdma_dev->bar);
>         ...
> }
>
> If dma_async_device_unregister() drops the last reference to the DMA device,
> it synchronously invokes the release callback, switchtec_dma_release(),
> which frees swdma_dev. Does this mean iounmap(swdma_dev->bar) will
> dereference the freed swdma_dev pointer?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260727181526.9672-1-logang@deltatee.com?part=3

Re: [PATCH v3 03/11] dmaengine: switchtec-dma: halt channel on alloc_chan_resources error

[Logan Gunthorpe]

On 2026-07-27 14:47, Frank Li wrote:
> On Mon, Jul 27, 2026 at 06:51:00PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>>
>> New issues:
>> - [High] Incomplete fix / DMA Use-After-Free due to early return in switchtec_dma_chan_stop() on halt timeout.
>>
> 
> Is it true?

Yes, unfortunately. I'm working on v4 now that fixes this and maybe the
issue Sashiko pointed out in patch 4 as well, but I haven't dug into
that one yet.

Logan

[PATCH v3 04/11] 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.

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
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 107769cca772..13efd4189bbb 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1176,6 +1176,16 @@ 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++)
+		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)
 {
@@ -1201,7 +1211,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;
 		}
 	}
@@ -1209,10 +1219,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;
 }
@@ -1221,12 +1229,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);
@@ -1317,6 +1321,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

Re: [PATCH v3 04/11] dmaengine: switchtec-dma: fix channel leak on registration failure

[sashiko-bot]

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

New issues:
- [High] Freeing DMA channels in error paths leaves dangling pointers in the `dma_dev->channels` list, leading to a Use-After-Free when accessed by the active channel status ISR.

Pre-existing issues:
- [High] The `switchtec_dma_remove()` function dereferences `swdma_dev` after it has been freed by the `device_release` callback.
- [High] Registering the channel status ISR before initializing the channel list head causes a NULL pointer dereference.
- [High] Pre-existing deadlock if a client callback attempts to abort or terminate the DMA channel.
- [High] Incorrect IRQ free API in `switchtec_dma_create()` error path guarantees an IRQ leak and subsequent Use-After-Free.
- [Medium] Missing `put_device()` in `switchtec_dma_create()` error path causes a reference count leak for the PCI device.
- [Medium] Missing `pci_free_irq_vectors()` on error paths in `switchtec_dma_create()` leaks MSI-X vectors.
--

--- Patch [4]: [PATCH v3 04/11] dmaengine: switchtec-dma: fix channel leak on registration failure ---
commit 58dd78b5d9196a368d9b2914fdfb2256ea81eef1
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

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 107769cca772f..13efd4189bbb4 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1176,6 +1176,16 @@ 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++)
> +		kfree(swdma_dev->swdma_chans[i]);

[Severity: High]
Does this code leak dangling pointers in the dma_dev->channels list?

The newly introduced switchtec_dma_chans_free() frees each swdma_chan but fails
to call list_del(&chan->device_node). If an interrupt occurs (e.g. spurious or
shared) before the ISR is disabled (or indefinitely if the ISR cleanup fails),
switchtec_dma_chan_status_isr() will iterate over this list and deference the
freed memory, leading to a Use-After-Free.

[ ... ]

> @@ -1317,6 +1321,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: Medium]
This is a pre-existing issue, but does this error path leak the PCI device
reference count?

During probe, switchtec_dma_create() acquires the reference via
get_device(&pdev->dev). If dma_async_device_register() fails, the execution
jumps here. This path cleanly frees memory but fails to call put_device(dma->dev),
which leaks the reference count and prevents clean teardown on hot-unplug.

>  
>  err_exit:
>  	if (swdma_dev->chan_status_irq)

[Severity: High]
This is a pre-existing issue, but does this incorrect IRQ free API guarantee
an IRQ leak and subsequent Use-After-Free?

switchtec_dma_create() requests an MSI-X vector via pci_request_irq() using
the hardware vector index (e.g. 0). However, this error path calls free_irq()
instead of pci_free_irq().

This attempts to free the Linux IRQ number corresponding to the index (0),
which fails silently. Furthermore, because it's index 0, the
if (swdma_dev->chan_status_irq) condition prevents the free from even executing.
The ISR remains registered after swdma_dev is kfreed, leading to a UAF when
the IRQ fires.

[Severity: Medium]
This is a pre-existing issue, but does this error path also leak MSI-X vectors?

If pci_alloc_irq_vectors() succeeds, but subsequent steps like pci_request_irq()
or switchtec_dma_chans_enumerate() fail, the execution jumps to the err_exit
label. The error cleanup omits pci_free_irq_vectors(), leaking the allocated
vectors.

[ ... ]

For switchtec_dma_remove():

> static void switchtec_dma_remove(struct pci_dev *pdev)
> {
> 	struct switchtec_dma_dev *swdma_dev = pci_get_drvdata(pdev);
> 
> 	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);
> 
> 	iounmap(swdma_dev->bar);

[Severity: High]
This is a pre-existing issue, but does this dereference swdma_dev after it has
been freed?

During driver unbind, dma_async_device_unregister() drops the last reference
to the DMA device, which synchronously invokes switchtec_dma_release(). This
callback frees swdma_dev via kfree(swdma_dev). Immediately after, this
function attempts to access swdma_dev->bar in the iounmap() call, causing a
Use-After-Free.

For switchtec_dma_create():

> 	rc = pci_request_irq(pdev, irq, NULL, switchtec_dma_chan_status_isr,
> 			     swdma_dev, KBUILD_MODNAME);
> 	if (rc)
> 		goto err_exit;
> 
> 	swdma_dev->chan_status_irq = irq;
> 
> 	chan_cnt = readl(swdma_dev->bar + SWITCHTEC_REG_CHAN_CNT);
> 	if (!chan_cnt) {
> 		pci_err(pdev, "No channel configured.\n");
> 		rc = -ENXIO;
> 		goto err_exit;
> 	}
> 
> 	chan_cnt = switchtec_dma_chans_enumerate(swdma_dev, pdev, chan_cnt);

[Severity: High]
This is a pre-existing issue, but does registering the channel status ISR
before initializing the channel list head cause a NULL pointer dereference?

switchtec_dma_create() calls pci_request_irq() to register the ISR, then
proceeds to switchtec_dma_chans_enumerate() which calls
INIT_LIST_HEAD(&dma->channels).

Because dma_dev is allocated with kzalloc_obj, the channels.next pointer is
initially NULL. If an interrupt fires during this narrow window,
list_for_each_entry() in the ISR will compute a negative offset from NULL and
attempt to dereference it, leading to an immediate kernel panic.

For switchtec_dma_cleanup_completed():

> static void
> switchtec_dma_cleanup_completed(struct switchtec_dma_chan *swdma_chan)
> {
> [ ... ]
> 		do {
> 			dma_cookie_complete(&desc->txd);
> 			dma_descriptor_unmap(&desc->txd);
> 			dmaengine_desc_get_callback_invoke(&desc->txd, &res);

[Severity: High]
This is a pre-existing issue, but can this cause a deadlock if a client callback
attempts to abort or terminate the DMA channel?

switchtec_dma_cleanup_completed() acquires swdma_chan->complete_lock with
spin_lock_bh() and invokes the client callback synchronously here.

If the client calls dmaengine_terminate_all(), it invokes:

static int switchtec_dma_terminate_all(struct dma_chan *chan)
{
	struct switchtec_dma_chan *swdma_chan =
		container_of(chan, struct switchtec_dma_chan, dma_chan);

	spin_lock_bh(&swdma_chan->complete_lock);

This attempts to re-acquire the exact same complete_lock unconditionally, causing
a self-deadlock on a single CPU core.

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

Re: [PATCH v3 04/11] dmaengine: switchtec-dma: fix channel leak on registration failure

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:19PM -0600, Logan Gunthorpe wrote:
> 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.
>
> 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
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/switchtec_dma.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 107769cca772..13efd4189bbb 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1176,6 +1176,16 @@ 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++)
> +		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)
>  {
> @@ -1201,7 +1211,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;
>  		}
>  	}
> @@ -1209,10 +1219,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;
>  }
> @@ -1221,12 +1229,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);
> @@ -1317,6 +1321,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 v3 05/11] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

[Logan Gunthorpe]

It always returned 0, and no caller checked it.

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 13efd4189bbb..c752a1b05871 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1165,15 +1165,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

Re: [PATCH v3 05/11] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a missing NULL check for ioremap() in switchtec_dma_create().
--

--- Patch [5]: [PATCH v3 05/11] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void ---
commit a207649bc05f965ee7cc3627c9764631e52253cb
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

This commit updates the return type of switchtec_dma_chans_release() to void,
as the function always returned 0 and its return value was never checked by
callers.

> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 13efd4189bbb4..c752a1b05871d 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1165,15 +1165,13 @@ static int switchtec_dma_chan_free(struct pci_dev *pdev,

[ ... ]

> -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 while reviewing this file I noticed a
missing NULL check for ioremap() in switchtec_dma_create():

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

	RCU_INIT_POINTER(swdma_dev->pdev, pdev);

	nr_vecs = pci_msix_vec_count(pdev);
	rc = pci_alloc_irq_vectors(pdev, nr_vecs, nr_vecs, PCI_IRQ_MSIX);
	if (rc < 0)
		goto err_exit;

	irq = readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC);

If ioremap() fails during device probe and returns NULL, does this cause a
kernel panic when swdma_dev->bar is dereferenced by readw()?

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

Re: [PATCH v3 05/11] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:20PM -0600, Logan Gunthorpe wrote:
> It always returned 0, and no caller checked it.
>

Nit: s/It/switchtec_dma_chans_release()

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 13efd4189bbb..c752a1b05871 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1165,15 +1165,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 v3 06/11] 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
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 c752a1b05871..31feb2816e79 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1248,6 +1248,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));
 
@@ -1322,8 +1324,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

Re: [PATCH v3 06/11] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

[sashiko-bot]

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

Pre-existing issues:
- [High] Unmapping `swdma_dev->bar` after `dma_async_device_unregister` causes a use-after-free.
- [High] Missing NULL check after `ioremap()` in `switchtec_dma_create` leads to a NULL pointer dereference on failure.
- [High] The DMA channel list is initialized after the status IRQ is requested, allowing an early interrupt to trigger a NULL pointer dereference.
- [High] In the error paths of `switchtec_dma_create()`, DMA channels are freed while the status IRQ remains registered, causing a Use-After-Free.
- [High] Invoking client callbacks while holding a driver spinlock causes a recursive locking deadlock if the callback re-enters the driver.
- [High] Incorrect teardown sequence in device remove risks hangs or crashes for active DMA clients.
--

--- Patch [6]: [PATCH v3 06/11] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error ---
commit 4abb826d6d25e80cfa344cde79ed065a727edad5
Author: Logan Gunthorpe <logang@deltatee.com>

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

This patch initializes chan_status_irq to -1 and uses the non-negative
value to determine when to call pci_free_irq() instead of free_irq().
This prevents freeing the wrong Linux IRQ and ensures the handler is
properly unregistered when the vector index is 0.

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 c752a1b05871d..31feb2816e79b 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1248,6 +1248,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 does ioremap() need a NULL check here?

If it fails under memory pressure and returns NULL, swdma_dev->bar will be
assigned NULL.

Shortly after, the code executes:

    irq = readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC);

Will this lead to an immediate NULL pointer dereference?

[ ... ]

> @@ -1322,8 +1324,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);

[Severity: High]
This isn't a bug introduced by this patch, but can this error path lead to a
use-after-free?

If dma_async_device_register() fails, the code jumps to
err_chans_release_exit, which calls switchtec_dma_chans_free(). This frees
the channel structures but leaves them linked in dma_dev->channels.

The status IRQ remains registered until pci_free_irq() is called here.

If an interrupt fires during this window, will
switchtec_dma_chan_status_isr() iterate the list and access the freed
channel objects?

>  
>  	iounmap(swdma_dev->bar);
>  	kfree(swdma_dev);

While reviewing this file, I noticed a few other pre-existing issues.

[Severity: High]
This is a pre-existing issue, but could an early interrupt trigger a NULL
pointer dereference?

In switchtec_dma_create(), pci_request_irq() is called 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, switchtec_dma_chan_status_isr() will
execute:

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

Since swdma_dev was allocated with kzalloc_obj(), the list head is NULL.

Does iterating this uninitialized list bypass termination checks and crash?


[Severity: High]
This isn't a bug introduced by this patch, but can unmapping swdma_dev->bar
after unregistering the DMA device cause a use-after-free?

Looking at switchtec_dma_remove(), dma_async_device_unregister() is called
before iounmap():

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

When dma_async_device_unregister() drops the final reference to the DMA
device, it triggers the release callback:

switchtec_dma_release()
    ...
    kfree(swdma_dev);

Will this sequence cause iounmap() to dereference the freed swdma_dev
structure?


[Severity: High]
This is a pre-existing issue, but can invoking client callbacks while
holding the driver spinlock cause a recursive locking deadlock?

Looking at switchtec_dma_cleanup_completed():

    spin_lock_bh(&swdma_chan->complete_lock);
    ...
    dmaengine_desc_get_callback_invoke(&desc->txd, &res);

If the client callback re-enters the driver API, for example by calling
dmaengine_tx_status(), it will execute switchtec_dma_tx_status(), which
again calls switchtec_dma_cleanup_completed().

Since complete_lock is a non-recursive spinlock, will this inner call
attempt to acquire it and lead to an immediate self-deadlock?


[Severity: High]
This isn't a bug introduced by this patch, but is the teardown sequence in
switchtec_dma_remove() correct?

switchtec_dma_remove()
    ...
    switchtec_dma_chans_release(pdev, swdma_dev);
    ...
    dma_async_device_unregister(&swdma_dev->dma_dev);

Here, switchtec_dma_chans_release() halts the hardware and frees channel IRQs
before dma_async_device_unregister() safely quiesces the DMA engine.

Will this inverted teardown destroy the hardware state while the DMA core
still considers the device active, causing active clients to hang waiting
for completions or crash if they submit new requests?

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

Re: [PATCH v3 06/11] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:21PM -0600, Logan Gunthorpe wrote:
> 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
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 c752a1b05871..31feb2816e79 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1248,6 +1248,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));
>
> @@ -1322,8 +1324,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 v3 07/11] 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 channels
are released and freed but never removed from dma_dev->channels.

Add switchtec_dma_chans_disable() which disables interrupts and
removes the channels from the list.. Call it 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
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/dma/switchtec_dma.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 31feb2816e79..800d8ecd0717 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1184,6 +1184,20 @@ 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)
+{
+	int i;
+
+	if (swdma_dev->chan_status_irq >= 0) {
+		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
+		swdma_dev->chan_status_irq = -1;
+	}
+
+	for (i = 0; i < swdma_dev->chan_cnt; i++)
+		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
+}
+
 static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 					 struct pci_dev *pdev, int chan_cnt)
 {
@@ -1217,6 +1231,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);
 
@@ -1320,6 +1335,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

Re: [PATCH v3 07/11] 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] Accessing `swdma_dev->bar` in `iounmap()` after the structure has been kfreed.
- [High] The `chan_status_irq` interrupt is requested before channel initialization is complete, leading to a NULL pointer dereference in the ISR.
- [High] The driver invokes the client's DMA completion callback while holding the internal `complete_lock` spinlock, creating a self-deadlock risk.
- [High] Missing check on the return value of `ioremap()` leads to a NULL pointer dereference.
--

--- Patch [7]: [PATCH v3 07/11] dmaengine: switchtec-dma: disable channels before freeing on registration failure ---
commit eb5cc523afcf50094dfae39f87e70f0557894b90
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 channels
are released and freed but never removed from dma_dev->channels.

Add switchtec_dma_chans_disable() which disables interrupts and
removes the channels from the list.. Call it before releasing and
freeing channels in both error paths.

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 31feb2816e79b..800d8ecd07171 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c

[ ... ]

> @@ -1217,6 +1231,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);
>  

[Severity: High]
This is a pre-existing issue, but could an early interrupt cause a NULL
pointer dereference here?

In switchtec_dma_create(), pci_request_irq() is called to enable
switchtec_dma_chan_status_isr() before switchtec_dma_chans_enumerate()
is called.

The enumerate function is what initializes the list head:

    INIT_LIST_HEAD(&dma->channels);

If an interrupt fires before the list is initialized, the ISR will execute
list_for_each_entry() over a NULL channels.next pointer. 

Should the interrupt request be deferred until after the channel enumeration
is complete?

[ ... ]

> @@ -1320,6 +1335,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 isn't a bug introduced by this patch, but does the ioremap call in
switchtec_dma_create() need a return value check?

In switchtec_dma_create(), the BAR is mapped and then immediately used:

switchtec_dma_create() {
    ...
    swdma_dev->bar = ioremap(pci_resource_start(pdev, 0), ...);
    ...
    irq = readw(swdma_dev->bar + SWITCHTEC_REG_CHAN_STS_VEC);
}

If ioremap() fails and returns NULL, readw() will dereference it. 

Should this include an error check before accessing swdma_dev->bar?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk during
driver unbind?

When switchtec_dma_remove() unregisters the device, it can drop the final
reference:

switchtec_dma_remove()
    dma_async_device_unregister()
        switchtec_dma_release()
            kfree(swdma_dev);

Immediately after dma_async_device_unregister() returns, the remove function
calls iounmap(swdma_dev->bar), dereferencing the freed pointer. 

Can this cause a kernel panic when the module is removed?

[Severity: High]
This is also a pre-existing issue, but does this locking pattern introduce a
CPU deadlock risk?

In switchtec_dma_cleanup_completed(), a spinlock is held while invoking an
opaque client callback:

switchtec_dma_cleanup_completed()
    spin_lock_bh(&swdma_chan->complete_lock);
    ...
    dmaengine_desc_get_callback_invoke(&desc->txd, &res);

If the client's callback invokes dmaengine_terminate_all(), the core will
call switchtec_dma_terminate_all(), which unconditionally attempts to acquire
the exact same complete_lock. 

Could this cause the CPU to deadlock?

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

Re: [PATCH v3 07/11] dmaengine: switchtec-dma: disable channels before freeing on registration failure

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:22PM -0600, Logan Gunthorpe wrote:
> 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 channels
> are released and freed but never removed from dma_dev->channels.
>
> Add switchtec_dma_chans_disable() which disables interrupts and
> removes the channels from the list.. Call it 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
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/dma/switchtec_dma.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
> index 31feb2816e79..800d8ecd0717 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1184,6 +1184,20 @@ 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)
> +{
> +	int i;
> +
> +	if (swdma_dev->chan_status_irq >= 0) {
> +		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
> +		swdma_dev->chan_status_irq = -1;
> +	}
> +
> +	for (i = 0; i < swdma_dev->chan_cnt; i++)
> +		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
> +}
> +

Maybe historic reason, it is not good to touch dma_chan::device_node.
Suppose some hepler function to manage channels.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
>  					 struct pci_dev *pdev, int chan_cnt)
>  {
> @@ -1217,6 +1231,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);
>
> @@ -1320,6 +1335,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
>

Re: [PATCH v3 07/11] dmaengine: switchtec-dma: disable channels before freeing on registration failure

[Logan Gunthorpe]

On 2026-07-27 15:21, Frank Li wrote:
> On Mon, Jul 27, 2026 at 12:15:22PM -0600, Logan Gunthorpe wrote:
>> +static void switchtec_dma_chans_disable(struct pci_dev *pdev,
>> +					struct switchtec_dma_dev *swdma_dev)
>> +{
>> +	int i;
>> +
>> +	if (swdma_dev->chan_status_irq >= 0) {
>> +		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
>> +		swdma_dev->chan_status_irq = -1;
>> +	}
>> +
>> +	for (i = 0; i < swdma_dev->chan_cnt; i++)
>> +		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
>> +}
>> +
> 
> Maybe historic reason, it is not good to touch dma_chan::device_node.
> Suppose some hepler function to manage channels.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Ok, thanks. When I have some free time I'll see if I can clean that up,
but not for this series.

Logan

[PATCH v3 08/11] 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
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 800d8ecd0717..d73506b5cabc 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1384,6 +1384,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);
 
@@ -1396,7 +1397,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

Re: [PATCH v3 08/11] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing check for ioremap() failure in switchtec_dma_create() can cause a null pointer dereference.
- [High] Client callbacks are invoked while holding the channel's `complete_lock`, leading to a self-deadlock if the callback calls DMA engine APIs.
- [High] The channel status IRQ is requested before the `dma_dev->channels` list is initialized, which will cause a NULL pointer dereference if an interrupt fires early.
--

--- Patch [8]: [PATCH v3 08/11] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() ---
commit 29e71ea6cf1f57028cf682c7cc5c705361a852e2
Author: Logan Gunthorpe <logang@deltatee.com>

dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

This commit fixes a use-after-free of swdma_dev in switchtec_dma_remove().
It caches the bar variable locally before calling
dma_async_device_unregister(), which can synchronously free swdma_dev.

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 800d8ecd07171..d73506b5cabc1 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 switchtec_dma_create()
check if ioremap() fails?

If ioremap() fails due to resource exhaustion, it returns NULL. The code
appears to immediately dereference this pointer:

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);
    ...
}

Can this lead to a NULL pointer dereference during device probe?

[Severity: High]
This is a pre-existing issue, but does the driver hold complete_lock while
invoking client callbacks?

In switchtec_dma_cleanup_completed(), the lock is acquired:

drivers/dma/switchtec_dma.c:switchtec_dma_cleanup_completed() {
    spin_lock_bh(&swdma_chan->complete_lock);
    ...
    dmaengine_desc_get_callback_invoke(&desc->txd, &res);
    ...
}

The same pattern appears in switchtec_dma_abort_desc().

If the client callback calls back into the DMA engine API (such as 
dmaengine_terminate_all()), could this cause a self-deadlock when the 
driver attempts to acquire complete_lock again?

[Severity: High]
This is a pre-existing issue, but is it safe to request the channel status
IRQ before the channels list is initialized?

In switchtec_dma_create(), pci_request_irq() is called before the channel
list is populated:

drivers/dma/switchtec_dma.c:switchtec_dma_create() {
    ...
    rc = pci_request_irq(pdev, irq, NULL, switchtec_dma_chan_status_isr,
                         swdma_dev, KBUILD_MODNAME);
    ...
    chan_cnt = switchtec_dma_chans_enumerate(swdma_dev, pdev, chan_cnt);
    ...
}

If an interrupt fires early, switchtec_dma_chan_status_isr() will iterate
over the uninitialized dma_dev->channels list:

drivers/dma/switchtec_dma.c:switchtec_dma_chan_status_isr() {
    ...
    list_for_each_entry(chan, &dma_dev->channels, device_node) {
    ...
}

Could this loop dereference a NULL pointer and crash if the list head is
completely zeroed before initialization?

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

Re: [PATCH v3 08/11] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:23PM -0600, Logan Gunthorpe wrote:
> 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
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 800d8ecd0717..d73506b5cabc 100644
> --- a/drivers/dma/switchtec_dma.c
> +++ b/drivers/dma/switchtec_dma.c
> @@ -1384,6 +1384,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);
>
> @@ -1396,7 +1397,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 v3 09/11] 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>
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

Re: [PATCH v3 09/11] dmaengine: ioat: disable relaxed ordering before registering the device

[sashiko-bot]

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

Pre-existing issues:
- [High] Fallback from MSI-X to MSI leaves MSI-X enabled, breaking interrupt delivery.
- [High] Possible Use-After-Free of `device` in shared interrupt handler on probe failure.
- [High] Global IDA leak in the dma_async_device_register error path.
- [Medium] Hardware prefetch limit is configured after the DMA device is exposed.
- [Medium] Sysfs kobject 'quickdata' is added after device uevent (TOCTOU race).
--

--- Patch [9]: [PATCH v3 09/11] dmaengine: ioat: disable relaxed ordering before registering the device ---
commit bae11619501364346652f2b925166039da65e184
Author: Logan Gunthorpe <logang@deltatee.com>
Subject: dmaengine: ioat: disable relaxed ordering before registering the device

Moves the capability read/write ahead of dma_async_device_register() so
failure doesn't leave dangling registered devices.

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
[ ... ]
> @@ -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);
> +

[Severity: Medium]
This isn't a bug introduced by this patch, but doesn't this create a
TOCTOU race?  The sysfs kobject 'quickdata' and its attributes are added
after dma_async_device_register() has already emitted the KOBJ_ADD uevent.
Could userspace tools reacting to the uevent miss these sysfs attributes?

> +	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);

[Severity: Medium]
This isn't a bug introduced by this patch, but is it safe to configure the
hardware prefetch limit after the DMA device has already been exposed?
Since dma_async_device_register() makes the channels available, could
concurrent DMA clients submit and execute operations with uninitialized or
stale prefetch hardware states before this writeb() occurs?


Additionally, while reviewing this code, I noticed a few pre-existing issues
outside the modified context:

[Severity: High]
This isn't a bug introduced by this patch, but in ioat_pci_probe(), the
device struct is manually freed if ioat3_dma_probe() fails:

drivers/dma/ioat/init.c:ioat_pci_probe() {
    ...
	err = ioat3_dma_probe(device, ioat_dca_enabled);
	if (err) {
		for (i = 0; i < IOAT_MAX_CHANS; i++)
			kfree(device->idx[i]);
		kfree(device);
		dev_err(dev, "Intel(R) I/OAT DMA Engine init failed\n");
		return -ENODEV;
	}
    ...
}

However, ioat_dma_setup_interrupts() allocates IRQs using
devm_request_irq(). Because the IRQs are devm-managed, they are not actually
unregistered until ioat_pci_probe() returns. Doesn't this create a window
where a shared interrupt could trigger ioat_dma_do_interrupt(), which
then casts and dereferences the already freed device pointer?

drivers/dma/ioat/dma.c:ioat_dma_do_interrupt() {
	struct ioatdma_device *instance = data;
    ...
	intrctrl = readb(instance->reg_base + IOAT_INTRCTRL_OFFSET);
    ...
}


[Severity: High]
This isn't a bug introduced by this patch, but when falling back from MSI-X
to MSI in ioat_dma_setup_interrupts(), is MSI-X properly disabled?

drivers/dma/ioat/init.c:ioat_dma_setup_interrupts() {
    ...
	err = pci_enable_msix_exact(pdev, ioat_dma->msix_entries, msixcnt);
	if (err)
		goto msi;

	for (i = 0; i < msixcnt; i++) {
        ...
		err = devm_request_irq(dev, msix->vector, ...);
		if (err) {
			for (j = 0; j < i; j++) {
                ...
				devm_free_irq(dev, msix->vector, ioat_chan);
			}
			goto msi;
		}
	}
    ...
}

If devm_request_irq() fails, the code frees the requested IRQs and jumps
to the msi label. However, it doesn't call pci_disable_msix(). Since the
device is still programmed for MSI-X, pci_enable_msi() will fail, and
the fallback will continue to INTx. Will the device ever generate INTx
interrupts if it was left with MSI-X enabled?


[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
dma_async_device_register() leak the global device IDA?

drivers/dma/dmaengine.c:dma_async_device_register() {
    ...
err_out:
	/* if we never registered a channel just release the idr */
	if (!device->chancnt) {
		ida_free(&dma_ida, device->dev_id);
		return rc;
	}

	list_for_each_entry(chan, &device->channels, device_node) {
    ...
}

If dma_async_device_register() successfully registers at least one channel
but fails on a subsequent one (so device->chancnt is greater than 0), it jumps
to err_out. The error path explicitly skips calling ida_free() because
device->chancnt is greater than 0. Does this permanently leak the global
IDA for this device?

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

Re: [PATCH v3 09/11] dmaengine: ioat: disable relaxed ordering before registering the device

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:24PM -0600, Logan Gunthorpe wrote:
> 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>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 v3 10/11] 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>
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

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

[sashiko-bot]

> Convert the sprintf() calls in the per-channel sysfs attribute show()
> functions to sysfs_emit().
> 
> Acked-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:25PM -0600, Logan Gunthorpe wrote:
> Convert the sprintf() calls in the per-channel sysfs attribute show()
> functions to sysfs_emit().
>
> Acked-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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 v3 11/11] 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")
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 v3 11/11] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()

[sashiko-bot]

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

Pre-existing issues:
- [High] Client callbacks are invoked with the driver's primary spinlock held, causing deadlocks if the callback initiates a new DMA transaction.
- [High] `plx_dma_prep_memcpy` returns with a spinlock held, violating API boundaries and causing deadlocks on multiple descriptor preparations.
- [High] Softirq deadlock due to failing to disable bottom halves before acquiring a spinlock in process context.
- [High] Missing read memory barrier after checking DMA descriptor status can lead to reading stale payload data.
--

--- Patch [11]: [PATCH v3 11/11] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() ---
commit 95d7de134f15bc6f73a67b043a21039f2fc9965a
Author: Logan Gunthorpe <logang@deltatee.com>

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.

Move 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;
>  	INIT_LIST_HEAD(&dma->channels);
>  	dma_cap_set(DMA_MEMCPY, dma->cap_mask);

While reviewing this patch, I noticed a few pre-existing issues in this driver:

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

[Severity: High]
This isn't a bug introduced by this patch, but can this cause a softirq
deadlock?

plx_dma_tx_status() is typically called from process context and invokes
plx_dma_process_desc(), which acquires this lock without disabling bottom
halves.

If a hardware interrupt occurs, plx_dma_isr() schedules a tasklet. Upon
returning from the interrupt, the tasklet executes in softirq context on the
same CPU and calls plx_dma_process_desc(), which attempts to acquire the
already-held non-recursive spinlock.

[ ... ]
		flags = le32_to_cpu(READ_ONCE(desc->hw->flags_and_size));

		if (flags & PLX_DESC_FLAG_VALID)
			break;

[Severity: High]
This isn't a bug introduced by this patch, but is a dma_rmb() needed here?

Without a read memory barrier after verifying the descriptor is marked valid
by the device, the CPU could speculatively reorder subsequent memory reads
before the status check completes. Could this lead to a client callback
reading stale payload data from the DMA destination buffer?

[ ... ]
		dmaengine_desc_get_callback_invoke(&desc->txd, &res);

[Severity: High]
This is a pre-existing issue, but could this cause a deadlock?

The client callback is invoked synchronously while plxdev->ring_lock is
held. If the client completion callback issues a new DMA transaction by
calling dmaengine_prep_*, it will call into plx_dma_prep_memcpy() which
attempts to acquire the same spinlock.

}


drivers/dma/plx_dma.c:plx_dma_prep_memcpy() {
[ ... ]
	spin_lock_bh(&plxdev->ring_lock);
[ ... ]
	/* return with the lock held, it will be released in tx_submit */

	return &plxdesc->txd;
}

[Severity: High]
This isn't a bug introduced by this patch, but does returning with the
spinlock held violate the dmaengine API contract?

If a client prepares two descriptors sequentially before submitting (e.g.,
calling prep() then prep() again), the second call will deadlock trying to
acquire the already-held lock. This also forces clients into an atomic
context between prep and submit.

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

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

[Frank Li]

On Mon, Jul 27, 2026 at 12:15:26PM -0600, Logan Gunthorpe wrote:
> 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")
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.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
>