mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma
@ 2026-09-02  6:21 Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 01/10] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe

This is the latest series of fixes that has been rebased onto v7.3-rc1.

After getting more Sashiko feedback on the two problematic patches I
have to eat crow and appologize. Frank was correct about them and I
was a bit too stubborn. Sorry about that. I have dropped those two
patches in this series so hopefully it can go in quickly.

Please note: I'm going to be on vacation starting Friday the 4th until
the 15th so if there is any feedback in that window I'll respond when
I get back.

Thanks,

Logan

Changes since v5:
 * Dropped the two patches that skipped freeing the descriptor rings
   when the channel could not be confirmed halted ("always clear DMA
   base registers on chan_stop()" and "halt channel on
   alloc_chan_resources error"), per Frank's recommendation. All
   remaining patches are unchanged.

Changes since v4:
 * Rebased onto v7.3-rc1.
 * Added paragraph to patches 3 and 4 to make clear that they are
   leaking memory in favour of preventing theoretically buggy hardware
   from trashing re-used memory. I think this is the best thing to do.

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

Changes since v2:

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

Changes since v1:

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

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


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
--
2.47.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 01/10] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc()
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 02/10] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources Logan Gunthorpe
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Frank Li

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

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index c133535d3765..a10818efba4e 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 02/10] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 01/10] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 03/10] dmaengine: switchtec-dma: fix channel leak on registration failure Logan Gunthorpe
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

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

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index a10818efba4e..e55fe9ff7e2c 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 03/10] dmaengine: switchtec-dma: fix channel leak on registration failure
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 01/10] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 02/10] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 04/10] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void Logan Gunthorpe
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

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

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

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index e55fe9ff7e2c..02d7c38363d6 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1175,6 +1175,18 @@ static int switchtec_dma_chans_release(struct pci_dev *pdev,
 	return 0;
 }
 
+static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
+{
+	int i;
+
+	for (i = 0; i < swdma_dev->chan_cnt; i++) {
+		list_del(&swdma_dev->swdma_chans[i]->dma_chan.device_node);
+		kfree(swdma_dev->swdma_chans[i]);
+	}
+
+	kfree(swdma_dev->swdma_chans);
+}
+
 static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 					 struct pci_dev *pdev, int chan_cnt)
 {
@@ -1200,7 +1212,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;
 		}
 	}
@@ -1208,10 +1220,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;
 }
@@ -1220,12 +1230,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);
@@ -1316,6 +1322,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 04/10] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (2 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 03/10] dmaengine: switchtec-dma: fix channel leak on registration failure Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 05/10] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error Logan Gunthorpe
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Frank Li

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 02d7c38363d6..17bbf85bcb16 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1164,15 +1164,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 05/10] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (3 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 04/10] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 06/10] dmaengine: switchtec-dma: disable channels before freeing on registration failure Logan Gunthorpe
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 17bbf85bcb16..430b09fa5102 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1249,6 +1249,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));
 
@@ -1323,8 +1325,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 06/10] dmaengine: switchtec-dma: disable channels before freeing on registration failure
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (4 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 05/10] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 07/10] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() Logan Gunthorpe
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

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

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 430b09fa5102..63f80e2a5b6f 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1185,6 +1185,15 @@ static void switchtec_dma_chans_free(struct switchtec_dma_dev *swdma_dev)
 	kfree(swdma_dev->swdma_chans);
 }
 
+static void switchtec_dma_chans_disable(struct pci_dev *pdev,
+					struct switchtec_dma_dev *swdma_dev)
+{
+	if (swdma_dev->chan_status_irq >= 0) {
+		pci_free_irq(pdev, swdma_dev->chan_status_irq, swdma_dev);
+		swdma_dev->chan_status_irq = -1;
+	}
+}
+
 static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
 					 struct pci_dev *pdev, int chan_cnt)
 {
@@ -1218,6 +1227,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);
 
@@ -1321,6 +1331,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 07/10] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove()
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (5 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 06/10] dmaengine: switchtec-dma: disable channels before freeing on registration failure Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 08/10] dmaengine: ioat: disable relaxed ordering before registering the device Logan Gunthorpe
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

Cache bar in a local variable before the unregister call.

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

diff --git a/drivers/dma/switchtec_dma.c b/drivers/dma/switchtec_dma.c
index 63f80e2a5b6f..d390949b843c 100644
--- a/drivers/dma/switchtec_dma.c
+++ b/drivers/dma/switchtec_dma.c
@@ -1380,6 +1380,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);
 
@@ -1392,7 +1393,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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 08/10] dmaengine: ioat: disable relaxed ordering before registering the device
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (6 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 07/10] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 09/10] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 10/10] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() Logan Gunthorpe
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sashiko, Frank Li

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

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

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 09/10] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show()
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (7 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 08/10] dmaengine: ioat: disable relaxed ordering before registering the device Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  2026-09-02  6:21 ` [PATCH v6 10/10] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() Logan Gunthorpe
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Frank Li

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

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v6 10/10] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr()
  2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
                   ` (8 preceding siblings ...)
  2026-09-02  6:21 ` [PATCH v6 09/10] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() Logan Gunthorpe
@ 2026-09-02  6:21 ` Logan Gunthorpe
  9 siblings, 0 replies; 11+ messages in thread
From: Logan Gunthorpe @ 2026-09-02  6:21 UTC (permalink / raw)
  To: linux-kernel, linux-pci, dmaengine, Vinod Koul
  Cc: Frank Li, Kelvin Cao, Thomas Weißschuh, Dave Jiang,
	George Ge, Jaeyoung Chung, Logan Gunthorpe, Sangyun Kim,
	Kyungwook Boo, Frank Li

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

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

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-02  6:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02  6:21 [PATCH v6 00/10] dmaengine: miscellaneous fixes for ioat, switchtec_dma and plx_dma Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 01/10] dmaengine: switchtec-dma: fix double-free in switchtec_dma_free_desc() Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 02/10] dmaengine: switchtec-dma: fix resource leak in alloc_chan_resources Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 03/10] dmaengine: switchtec-dma: fix channel leak on registration failure Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 04/10] dmaengine: switchtec-dma: make switchtec_dma_chans_release() void Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 05/10] dmaengine: switchtec-dma: fix chan_status_irq cleanup on create() error Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 06/10] dmaengine: switchtec-dma: disable channels before freeing on registration failure Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 07/10] dmaengine: switchtec-dma: fix use-after-free of swdma_dev in remove() Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 08/10] dmaengine: ioat: disable relaxed ordering before registering the device Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 09/10] dmaengine: ioat: use sysfs_emit() in per-channel sysfs show() Logan Gunthorpe
2026-09-02  6:21 ` [PATCH v6 10/10] dmaengine: plx_dma: fix NULL pointer deref in plx_dma_isr() Logan Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®