[PATCH v3 0/8] dmaengine: ti: omap-dma: probe/remove bug fixes and cleanup

[PATCH v3 0/8] dmaengine: ti: omap-dma: probe/remove bug fixes and cleanup

[Rosen Penev]

Fix several bugs in the omap-dma driver's probe error and remove paths:
missing return after failure, CPU PM notifier leaks and missing RCU
synchronization, channels freed without stopping hardware, IRQs left
enabled during teardown, descriptor pool destroyed too early, wrong
interrupt register used in remove, and a flexible array conversion.

v3: Address remaining review comments:
  - Split CPU PM notifier fix into leak fix + RCU sync
  - Add missing return in probe error path
  - Guard IRQENABLE_L1 accesses for legacy platforms
v2: Fix sashiko comments and add extra patch

Rosen Penev (8):
  dmaengine: ti: omap-dma: fix missing return in probe error path
  dmaengine: ti: omap-dma: synchronize CPU PM notifier removal
  dmaengine: ti: omap-dma: fix CPU PM notifier leak
  dmaengine: ti: omap-dma: stop channels during teardown
  dmaengine: ti: omap-dma: disable IRQs on probe failure
  dmaengine: ti: omap-dma: destroy descriptor pool last
  dmaengine: ti: omap-dma: fix interrupt handling in remove
  dmaengine: ti: omap-dma: turn lch_map into a flexible array

 drivers/dma/ti/omap-dma.c | 122 +++++++++++++++++++++++---------------
 1 file changed, 74 insertions(+), 48 deletions(-)

--
2.54.0

[PATCHv3 1/8] dmaengine: ti: omap-dma: fix missing return in probe error path

[Rosen Penev]

If of_dma_controller_register() fails, the error path omits the return
statement, causing probe to continue (and eventually succeed) despite
the DMA controller not being registered. Add the missing return rc;.

Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 55ece7fd0d99..0f6dd6b0a301 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1828,6 +1828,7 @@ static int omap_dma_probe(struct platform_device *pdev)
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
 			omap_dma_free(od);
+			return rc;
 		}
 	}
 
-- 
2.54.0

[PATCHv3 2/8] dmaengine: ti: omap-dma: synchronize CPU PM notifier removal

[Rosen Penev]

cpu_pm_notify() walks the raw notifier chain under rcu_read_lock(),
while cpu_pm_unregister_notifier() only unlinks the notifier block.

The controller is devres allocated and can be freed shortly after
remove returns. Wait for an RCU grace period after unregistering the
CPU PM notifier so concurrent CPU PM readers cannot dereference a
freed notifier block.

Fixes: 4c74ecf79227 ("dmaengine: ti: omap-dma: Add device tree match data and use it for cpu_pm")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 0f6dd6b0a301..15be3c90440a 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/omap-dma.h>
 #include <linux/platform_device.h>
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/of.h>
@@ -1853,8 +1854,10 @@ static void omap_dma_remove(struct platform_device *pdev)
 	struct omap_dmadev *od = platform_get_drvdata(pdev);
 	int irq;
 
-	if (od->cfg->may_lose_context)
+	if (od->cfg->may_lose_context) {
 		cpu_pm_unregister_notifier(&od->nb);
+		synchronize_rcu();
+	}
 
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
-- 
2.54.0

[PATCHv3 3/8] dmaengine: ti: omap-dma: fix CPU PM notifier leak

[Rosen Penev]

The CPU PM notifier may be registered for needs_busy_check on omap2
rather than may_lose_context on omap3. The remove path only checked
may_lose_context, leaving the omap2 notifier registered during driver
removal.

Check both configuration flags before unregistering the notifier.

Fixes: f4cfa36dab67 ("dmaengine: ti: omap-dma: Use cpu notifier to block idle for omap2")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 15be3c90440a..0ad8da8b35f8 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1854,7 +1854,7 @@ static void omap_dma_remove(struct platform_device *pdev)
 	struct omap_dmadev *od = platform_get_drvdata(pdev);
 	int irq;
 
-	if (od->cfg->may_lose_context) {
+	if (od->cfg->needs_busy_check || od->cfg->may_lose_context) {
 		cpu_pm_unregister_notifier(&od->nb);
 		synchronize_rcu();
 	}
-- 
2.54.0

[PATCHv3 4/8] dmaengine: ti: omap-dma: stop channels during teardown

[Rosen Penev]

omap_dma_free() removes channels and frees their storage without
first stopping an active transfer. A channel may have moved the
active descriptor out of the virt-dma lists into c->desc, so freeing
only the list state can leave hardware running against descriptor
memory that is about to disappear.

Terminate each channel before removing it, then drain the virt-dma
resource lists before freeing the channel structure.

Fixes: 7bedaa553760 ("dmaengine: add OMAP DMA engine driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 0ad8da8b35f8..cef4e3a38b04 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1521,8 +1521,10 @@ static void omap_dma_free(struct omap_dmadev *od)
 		struct omap_chan *c = list_first_entry(&od->ddev.channels,
 			struct omap_chan, vc.chan.device_node);
 
+		omap_dma_terminate_all(&c->vc.chan);
 		list_del(&c->vc.chan.device_node);
 		tasklet_kill(&c->vc.task);
+		vchan_free_chan_resources(&c->vc);
 		kfree(c);
 	}
 }
-- 
2.54.0

[PATCHv3 5/8] dmaengine: ti: omap-dma: disable IRQs on probe failure

[Rosen Penev]

The probe failure paths after IRQ setup free channel state while
hardware interrupts can still be enabled. A concurrent interrupt can
then walk lch_map[] and access channel memory that teardown is
releasing.

Disable IRQENABLE_L1 and clear irq_enable_mask under irq_lock before
teardown, then read IRQENABLE_L1 back to flush the posted write. Guard
the L1 accesses for legacy platforms where that register is not
mapped.

Fixes: 7bedaa553760 ("dmaengine: add OMAP DMA engine driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index cef4e3a38b04..61a935660341 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1811,6 +1811,13 @@ static int omap_dma_probe(struct platform_device *pdev)
 	if (rc) {
 		pr_warn("OMAP-DMA: failed to register slave DMA engine device: %d\n",
 			rc);
+		if (!omap_dma_legacy(od)) {
+			spin_lock_irq(&od->irq_lock);
+			od->irq_enable_mask = 0;
+			omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+			spin_unlock_irq(&od->irq_lock);
+			omap_dma_glbl_read(od, IRQENABLE_L1);
+		}
 		if (od->ll123_supported)
 			dma_pool_destroy(od->desc_pool);
 		omap_dma_free(od);
@@ -1828,6 +1835,13 @@ static int omap_dma_probe(struct platform_device *pdev)
 		if (rc) {
 			pr_warn("OMAP-DMA: failed to register DMA controller\n");
 			dma_async_device_unregister(&od->ddev);
+			if (!omap_dma_legacy(od)) {
+				spin_lock_irq(&od->irq_lock);
+				od->irq_enable_mask = 0;
+				omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+				spin_unlock_irq(&od->irq_lock);
+				omap_dma_glbl_read(od, IRQENABLE_L1);
+			}
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
 			omap_dma_free(od);
-- 
2.54.0

[PATCHv3 6/8] dmaengine: ti: omap-dma: destroy descriptor pool last

[Rosen Penev]

Linked-list descriptors are allocated from desc_pool and can be
released from omap_dma_free() through the channel descriptor cleanup
path. Destroying desc_pool before freeing channels leaves descriptor
cleanup with a dangling pool pointer.

Free the channels before destroying desc_pool in probe failure paths
and in remove.

Fixes: 1c2e8e6b6429 ("dmaengine: omap-dma: Support for LinkedList transfer of slave_sg")
Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 61a935660341..c0890d8c43ba 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1818,9 +1818,9 @@ static int omap_dma_probe(struct platform_device *pdev)
 			spin_unlock_irq(&od->irq_lock);
 			omap_dma_glbl_read(od, IRQENABLE_L1);
 		}
+		omap_dma_free(od);
 		if (od->ll123_supported)
 			dma_pool_destroy(od->desc_pool);
-		omap_dma_free(od);
 		return rc;
 	}
 
@@ -1842,9 +1842,9 @@ static int omap_dma_probe(struct platform_device *pdev)
 				spin_unlock_irq(&od->irq_lock);
 				omap_dma_glbl_read(od, IRQENABLE_L1);
 			}
+			omap_dma_free(od);
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
-			omap_dma_free(od);
 			return rc;
 		}
 	}
@@ -1888,10 +1888,10 @@ static void omap_dma_remove(struct platform_device *pdev)
 		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
 	}
 
+	omap_dma_free(od);
+
 	if (od->ll123_supported)
 		dma_pool_destroy(od->desc_pool);
-
-	omap_dma_free(od);
 }
 
 static const struct omap_dma_config omap2420_data = {
-- 
2.54.0

[PATCHv3 7/8] dmaengine: ti: omap-dma: fix interrupt handling in remove

[Rosen Penev]

The remove path had several pre-existing bugs:

1. Interrupts are enabled via IRQENABLE_L1 in probe and
   alloc_chan_resources, but the remove path writes to IRQENABLE_L0,
   which has no effect on the L1 interrupt line. The DMA engine can
   continue asserting its IRQ during removal. Write to IRQENABLE_L1
   instead.

2. devm_free_irq() was called before disabling hardware interrupts.
   With IRQF_SHARED, the hardware may still assert the IRQ line after
   the handler is freed, causing unhandled interrupts that can lead to
   the kernel permanently disabling the shared IRQ line. Disable
   interrupts first.

3. platform_get_irq() return value was not checked before
   devm_free_irq(). If it returns an error code (<= 0), passing it to
   devm_free_irq() is incorrect. Add a guard.

4. Clearing od->irq_enable_mask and writing to IRQENABLE_L1 raced with
   the interrupt handler, which reads irq_enable_mask under the
   spinlock. Hold irq_lock around the disable.

5. The posted write to IRQENABLE_L1 used _relaxed accessors with no
   readback to drain the write buffer. Add a readback flush before
   devm_free_irq() to ensure the hardware has actually disabled the
   interrupt line.

Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index c0890d8c43ba..7343325ce2b1 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1517,9 +1517,11 @@ static int omap_dma_chan_init(struct omap_dmadev *od)
 
 static void omap_dma_free(struct omap_dmadev *od)
 {
+	struct omap_chan *c;
+
 	while (!list_empty(&od->ddev.channels)) {
-		struct omap_chan *c = list_first_entry(&od->ddev.channels,
-			struct omap_chan, vc.chan.device_node);
+		c = list_first_entry(&od->ddev.channels,
+				     struct omap_chan, vc.chan.device_node);
 
 		omap_dma_terminate_all(&c->vc.chan);
 		list_del(&c->vc.chan.device_node);
@@ -1878,16 +1880,20 @@ static void omap_dma_remove(struct platform_device *pdev)
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
 
-	irq = platform_get_irq(pdev, 1);
-	devm_free_irq(&pdev->dev, irq, od);
-
 	dma_async_device_unregister(&od->ddev);
 
 	if (!omap_dma_legacy(od)) {
-		/* Disable all interrupts */
-		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
+		spin_lock_irq(&od->irq_lock);
+		od->irq_enable_mask = 0;
+		omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+		spin_unlock_irq(&od->irq_lock);
+		omap_dma_glbl_read(od, IRQENABLE_L1);
 	}
 
+	irq = platform_get_irq(pdev, 1);
+	if (irq > 0)
+		devm_free_irq(&pdev->dev, irq, od);
+
 	omap_dma_free(od);
 
 	if (od->ll123_supported)
-- 
2.54.0

[PATCHv3 8/8] dmaengine: ti: omap-dma: turn lch_map into a flexible array

[Rosen Penev]

Convert the separately-allocated lch_map pointer array to a C99
flexible array member at the end of struct omap_dmadev and annotate it
with __counted_by(lch_count). The probe is reordered so platform_data
lookup and the lch_count determination happen before the parent
allocation, letting struct_size() size the FAM and the dedicated
devm_kcalloc() for lch_map go away.

Two allocations collapse into one and the runtime bounds checks from
__counted_by now apply to every lch_map[] access.

Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 72 +++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 7343325ce2b1..dab600604572 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -49,7 +49,7 @@ struct omap_dmadev {
 	const struct omap_dma_config *cfg;
 	struct notifier_block nb;
 	struct omap_dma_context context;
-	int lch_count;
+	u32 lch_count;
 	DECLARE_BITMAP(lch_bitmap, OMAP_SDMA_CHANNELS);
 	struct mutex lch_lock;		/* for assigning logical channels */
 	bool legacy;
@@ -58,7 +58,7 @@ struct omap_dmadev {
 	unsigned dma_requests;
 	spinlock_t irq_lock;
 	uint32_t irq_enable_mask;
-	struct omap_chan **lch_map;
+	struct omap_chan *lch_map[] __counted_by(lch_count);
 };
 
 struct omap_chan {
@@ -1661,36 +1661,55 @@ static const struct omap_dma_config default_cfg;
 static int omap_dma_probe(struct platform_device *pdev)
 {
 	const struct omap_dma_config *conf;
+	struct omap_system_dma_plat_info *plat;
 	struct omap_dmadev *od;
+	u32 lch_count;
 	int rc, i, irq;
 	u32 val;
 
-	od = devm_kzalloc(&pdev->dev, sizeof(*od), GFP_KERNEL);
-	if (!od)
-		return -ENOMEM;
-
-	od->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(od->base))
-		return PTR_ERR(od->base);
-
 	conf = of_device_get_match_data(&pdev->dev);
 	if (conf) {
-		od->cfg = conf;
-		od->plat = dev_get_platdata(&pdev->dev);
-		if (!od->plat) {
+		plat = dev_get_platdata(&pdev->dev);
+		if (!plat) {
 			dev_err(&pdev->dev, "omap_system_dma_plat_info is missing");
 			return -ENODEV;
 		}
 	} else if (IS_ENABLED(CONFIG_ARCH_OMAP1)) {
-		od->cfg = &default_cfg;
-
-		od->plat = omap_get_plat_info();
-		if (!od->plat)
+		plat = omap_get_plat_info();
+		if (!plat)
 			return -EPROBE_DEFER;
 	} else {
 		return -ENODEV;
 	}
 
+	/* Number of available logical channels */
+	if (!pdev->dev.of_node) {
+		lch_count = plat->dma_attr->lch_count;
+		if (unlikely(!lch_count))
+			lch_count = OMAP_SDMA_CHANNELS;
+	} else if (of_property_read_u32(pdev->dev.of_node, "dma-channels", &lch_count)) {
+		dev_info(&pdev->dev, "Missing dma-channels property, using %u.\n",
+			 OMAP_SDMA_CHANNELS);
+		lch_count = OMAP_SDMA_CHANNELS;
+	}
+
+	if (lch_count > OMAP_SDMA_CHANNELS) {
+		dev_err(&pdev->dev, "invalid dma-channels value %u\n", lch_count);
+		return -EINVAL;
+	}
+
+	od = devm_kzalloc(&pdev->dev, struct_size(od, lch_map, lch_count), GFP_KERNEL);
+	if (!od)
+		return -ENOMEM;
+
+	od->lch_count = lch_count;
+	od->plat = plat;
+	od->cfg = conf ? conf : &default_cfg;
+
+	od->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(od->base))
+		return PTR_ERR(od->base);
+
 	od->reg_map = od->plat->reg_map;
 
 	dma_cap_set(DMA_SLAVE, od->ddev.cap_mask);
@@ -1735,19 +1754,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 			 OMAP_SDMA_REQUESTS);
 	}
 
-	/* Number of available logical channels */
-	if (!pdev->dev.of_node) {
-		od->lch_count = od->plat->dma_attr->lch_count;
-		if (unlikely(!od->lch_count))
-			od->lch_count = OMAP_SDMA_CHANNELS;
-	} else if (of_property_read_u32(pdev->dev.of_node, "dma-channels",
-					&od->lch_count)) {
-		dev_info(&pdev->dev,
-			 "Missing dma-channels property, using %u.\n",
-			 OMAP_SDMA_CHANNELS);
-		od->lch_count = OMAP_SDMA_CHANNELS;
-	}
-
 	/* Mask of allowed logical channels */
 	if (pdev->dev.of_node && !of_property_read_u32(pdev->dev.of_node,
 						       "dma-channel-mask",
@@ -1759,12 +1765,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 	if (od->plat->dma_attr->dev_caps & HS_CHANNELS_RESERVED)
 		bitmap_set(od->lch_bitmap, 0, 2);
 
-	od->lch_map = devm_kcalloc(&pdev->dev, od->lch_count,
-				   sizeof(*od->lch_map),
-				   GFP_KERNEL);
-	if (!od->lch_map)
-		return -ENOMEM;
-
 	for (i = 0; i < od->dma_requests; i++) {
 		rc = omap_dma_chan_init(od);
 		if (rc) {
-- 
2.54.0

Re: [PATCHv3 1/8] dmaengine: ti: omap-dma: fix missing return in probe error path

[sashiko-bot]

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

New issues:
- [Medium] The commit message factually misstates the behavior of the bug, claiming the probe function 'eventually succeeds' without the return statement.

Pre-existing issues:
- [High] The CPU PM notifier is registered during probe if `needs_busy_check` is true, but `omap_dma_remove` only unregisters it if `may_lose_context` is true, causing a resource leak and potential use-after-free.
- [High] `platform_get_irq` returning an error (e.g., `-EPROBE_DEFER`) on non-OMAP1 platforms incorrectly allows the driver to probe without an interrupt handler.
- [Medium] Unconditional call to `devm_free_irq` in `omap_dma_remove` can use an invalid or unrequested IRQ if `platform_get_irq` returned an error during probe.
--

commit d8616fa8712a6dcc8171a1ac6113861b509b7b33
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ti: omap-dma: fix missing return in probe error path

If of_dma_controller_register() fails, the error path omits the return
statement, causing probe to continue (and eventually succeed) despite
the DMA controller not being registered. Add the missing return rc;.

Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>

> If of_dma_controller_register() fails, the error path omits the return
> statement, causing probe to continue (and eventually succeed) despite
> the DMA controller not being registered. Add the missing return rc;.

[Severity: Medium]
Is this description accurate?

The variable rc retains the non-zero error code returned by
of_dma_controller_register(). Since there are no further assignments to rc in
the remainder of omap_dma_probe(), the function ultimately reaches return rc;
and fails. 

The actual impact of the missing return was that the driver erroneously executed
subsequent initialization (like registering the CPU PM notifier) and then
failed.

Since the failure path after that point does not unregister the notifier, this
left a dangling pointer and caused an immediate use-after-free when the device
structure was freed by devres.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 55ece7fd0d99a..0f6dd6b0a301f 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1828,6 +1828,7 @@ static int omap_dma_probe(struct platform_device *pdev)
>  			if (od->ll123_supported)
>  				dma_pool_destroy(od->desc_pool);
>  			omap_dma_free(od);
> +			return rc;
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing unregister
for the CPU PM notifier in omap_dma_remove()?

In omap_dma_probe(), the notifier is registered if either needs_busy_check or
may_lose_context is true:

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
    ...
	if (od->cfg->needs_busy_check) {
		od->nb.notifier_call = omap_dma_busy_notifier;
		cpu_pm_register_notifier(&od->nb);
	} else if (od->cfg->may_lose_context) {
		od->nb.notifier_call = omap_dma_context_notifier;
		cpu_pm_register_notifier(&od->nb);
	}
    ...
}

However, omap_dma_remove() strictly guards the unregistration with
may_lose_context:

drivers/dma/ti/omap-dma.c:omap_dma_remove() {
    ...
	if (od->cfg->may_lose_context)
		cpu_pm_unregister_notifier(&od->nb);
    ...
}

For SoC configurations where needs_busy_check is true but may_lose_context is
false, the notifier is never unregistered upon module removal, leading to a
use-after-free panic upon the next PM transition when the CPU PM subsystem
calls the freed notifier.

[Severity: High]
This is a pre-existing issue, but does omap_dma_probe() incorrectly ignore
errors from platform_get_irq() on non-OMAP1 platforms?

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
    ...
	irq = platform_get_irq(pdev, 1);
	if (irq <= 0) {
		dev_info(&pdev->dev, "failed to get L1 IRQ: %d\n", irq);
		od->legacy = true;
	} else {
    ...
}

If platform_get_irq() returns an error like -EPROBE_DEFER, the driver sets
legacy mode and continues without an IRQ.

On non-OMAP1 platforms, the legacy macro evaluates to false, causing the driver
to operate as a modern DMA controller configuring interrupts, but without an
IRQ handler registered.

This results in silent probe success followed by permanently hung DMA transfers.

[Severity: Medium]
This is a pre-existing issue, but can omap_dma_remove() pass an invalid IRQ
number to devm_free_irq()?

drivers/dma/ti/omap-dma.c:omap_dma_remove() {
    ...
	irq = platform_get_irq(pdev, 1);
	devm_free_irq(&pdev->dev, irq, od);
    ...
}

If platform_get_irq() returned an error during probe, or returns an error here
in remove, devm_free_irq() is called with an invalid IRQ.

This causes a warning in devres_release() since the IRQ was never requested via
devres, followed by calling free_irq() with an invalid IRQ number.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603030754.288757-2-rosenp@gmail.com?part=1

Re: [PATCHv3 2/8] dmaengine: ti: omap-dma: synchronize CPU PM notifier removal

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the CPU PM notifier is not unregistered in omap_dma_remove() if the driver is configured with `needs_busy_check` but not `may_lose_context`, causing a use-after-free.
- [High] This problem wasn't introduced by this patch, but a missing `return rc;` in the `of_dma_controller_register()` error path causes the CPU PM notifier to be incorrectly registered before returning an error, leading to a use-after-free.
- [High] This problem wasn't introduced by this patch, but the hardware IRQ handler is prematurely freed before the DMA device is unregistered and before hardware interrupts are properly disabled.
--

commit dc72bbd762c67afa3770b4d714901b84c5498c2f
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ti: omap-dma: synchronize CPU PM notifier removal

cpu_pm_notify() walks the raw notifier chain under rcu_read_lock(),
while cpu_pm_unregister_notifier() only unlinks the notifier block.
This patch waits for an RCU grace period after unregistering the CPU PM
notifier to prevent concurrent readers from dereferencing a freed block.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 55ece7fd0d99a..6d1fb39956d7e 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c

[ ... ]

> @@ -1852,8 +1853,10 @@ static void omap_dma_remove(struct platform_device *pdev)
>  	struct omap_dmadev *od = platform_get_drvdata(pdev);
>  	int irq;
>  
> -	if (od->cfg->may_lose_context)
> +	if (od->cfg->may_lose_context) {
>  		cpu_pm_unregister_notifier(&od->nb);
> +		synchronize_rcu();
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but does this leak the CPU PM 
notifier if the driver is configured with needs_busy_check but not 
may_lose_context?

In omap_dma_probe(), the notifier is registered if either needs_busy_check 
or may_lose_context is true:

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
	...
	if (od->cfg->needs_busy_check) {
		od->nb.notifier_call = omap_dma_busy_notifier;
		cpu_pm_register_notifier(&od->nb);
	} else if (od->cfg->may_lose_context) {
	...
}

If a platform configuration sets only needs_busy_check, the notifier is 
never unregistered during remove. Because the od structure is devm-allocated,
will this leave a dangling pointer in the CPU PM notifier chain that causes 
a use-after-free on the next PM event?

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing return 
statement in the probe error path that could lead to a similar use-after-free?

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
	...
		rc = of_dma_controller_register(pdev->dev.of_node,
				of_dma_simple_xlate, &omap_dma_info);
		if (rc) {
			pr_warn("OMAP-DMA: failed to register DMA controller\n");
			dma_async_device_unregister(&od->ddev);
			if (od->ll123_supported)
				dma_pool_destroy(od->desc_pool);
			omap_dma_free(od);
		}
	}
	
	if (od->cfg->needs_busy_check) {
		od->nb.notifier_call = omap_dma_busy_notifier;
		cpu_pm_register_notifier(&od->nb);
	...
	return rc;
}

If of_dma_controller_register() fails, execution falls through, potentially
registers the CPU PM notifier using the already-freed od structure, and then
returns the error code. 

When devres cleans up the failed probe, will the notifier remain registered 
and point to freed memory?

[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition
during teardown if an interrupt fires after devm_free_irq() is called?

Looking at the rest of omap_dma_remove():

drivers/dma/ti/omap-dma.c:omap_dma_remove() {
	...
	irq = platform_get_irq(pdev, 1);
	devm_free_irq(&pdev->dev, irq, od);

	dma_async_device_unregister(&od->ddev);

	if (!omap_dma_legacy(od)) {
		/* Disable all interrupts */
		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
	}
	...
}

Since devm_free_irq() is called before dma_async_device_unregister(), if a 
DMA transfer completes while waiting for device unregistration, will the 
hardware assert the IRQ line without a handler, causing an unhandled 
interrupt storm?

Additionally, the teardown logic disables IRQENABLE_L0 instead of 
IRQENABLE_L1. Could this leave the device capable of firing interrupts 
even after the driver is unloaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603030754.288757-3-rosenp@gmail.com?part=1

Re: [PATCHv3 5/8] dmaengine: ti: omap-dma: disable IRQs on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing `return rc;` in the `of_dma_controller_register` error path causes probe to fall through and register a CPU PM notifier using memory that will subsequently be freed.
- [High] Missing `cpu_pm_unregister_notifier` on module removal for configurations with `needs_busy_check`, leading to a dangling PM notifier.
- [High] Wrong interrupt enable register (`IRQENABLE_L0`) is cleared during `omap_dma_remove`, risking an unhandled IRQ storm.
- [High] Division by zero in SG transfer preparation if `maxburst` is configured to 0.
--

commit 2fb0d3495f0f7caa9bb739f8dcae6f87cb0dcd07
Author: Rosen Penev <rosenp@gmail.com>
Subject: dmaengine: ti: omap-dma: disable IRQs on probe failure

The probe failure paths after IRQ setup free channel state while hardware
interrupts can still be enabled. A concurrent interrupt can then walk
lch_map[] and access channel memory that teardown is releasing. Disable
IRQENABLE_L1 and clear irq_enable_mask under irq_lock before teardown,
then read IRQENABLE_L1 back to flush the posted write.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 55ece7fd0d99a..a5dc07bcbeb87 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c

[ ... ]

> @@ -1825,6 +1832,13 @@ static int omap_dma_probe(struct platform_device *pdev)
>  		if (rc) {
>  			pr_warn("OMAP-DMA: failed to register DMA controller\n");
>  			dma_async_device_unregister(&od->ddev);
> +			if (!omap_dma_legacy(od)) {
> +				spin_lock_irq(&od->irq_lock);
> +				od->irq_enable_mask = 0;
> +				omap_dma_glbl_write(od, IRQENABLE_L1, 0);
> +				spin_unlock_irq(&od->irq_lock);
> +				omap_dma_glbl_read(od, IRQENABLE_L1);
> +			}
>  			if (od->ll123_supported)
>  				dma_pool_destroy(od->desc_pool);
>  			omap_dma_free(od);
>  		}

[Severity: High]
This is a pre-existing issue, but shouldn't this error path in
omap_dma_probe() return the error code?

If of_dma_controller_register() fails, the code frees the resources and
cleans up, but execution falls through instead of returning. It then proceeds
to register a CPU PM notifier on the newly freed memory:

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
    ...
	if (od->cfg->needs_busy_check) {
		od->nb.notifier_call = omap_dma_busy_notifier;
		cpu_pm_register_notifier(&od->nb);
    ...
}

When devres releases the od structure, od->nb will become a dangling
pointer, which could cause a use-after-free on the next CPU power transition.

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does omap_dma_remove()
correctly unregister the CPU PM notifier in all configurations?

In omap_dma_probe(), the notifier is conditionally registered like this:

drivers/dma/ti/omap-dma.c:omap_dma_probe() {
    ...
	if (od->cfg->needs_busy_check) {
		od->nb.notifier_call = omap_dma_busy_notifier;
		cpu_pm_register_notifier(&od->nb);
	} else if (od->cfg->may_lose_context) {
		od->nb.notifier_call = omap_dma_context_notifier;
		cpu_pm_register_notifier(&od->nb);
	}
    ...
}

But in omap_dma_remove(), it only unregisters if may_lose_context is true:

drivers/dma/ti/omap-dma.c:omap_dma_remove() {
    ...
	if (od->cfg->may_lose_context)
		cpu_pm_unregister_notifier(&od->nb);
    ...
}

If needs_busy_check is true and may_lose_context is false, the notifier is
never unregistered, leaving a dangling pointer when the module is removed.

[ ... ]

[Severity: High]
This is a pre-existing issue, but does omap_dma_remove() disable the correct
interrupt enable register?

During omap_dma_remove(), the code attempts to disable interrupts:

drivers/dma/ti/omap-dma.c:omap_dma_remove() {
    ...
	if (!omap_dma_legacy(od)) {
		/* Disable all interrupts */
		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
	}
    ...
}

The driver configures interrupts on IRQENABLE_L1 during probe and channel
allocation. By clearing IRQENABLE_L0 instead, active channels remain enabled in
IRQENABLE_L1. After the IRQ handler is unregistered, any pending hardware
interrupt could cause an unhandled IRQ storm.

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but could a client passing 0 for
maxburst cause a division by zero in omap_dma_prep_slave_sg()?

In omap_dma_prep_slave_sg(), the burst variable is initialized directly from
the client's configuration:

drivers/dma/ti/omap-dma.c:omap_dma_prep_slave_sg() {
    ...
	if (dir == DMA_DEV_TO_MEM) {
        ...
		burst = c->cfg.src_maxburst;
    ...
	en = burst;
	frame_bytes = es_bytes[es] * en;
    ...
}

Later in the scatterlist loop, frame_bytes is used in a division:

drivers/dma/ti/omap-dma.c:omap_dma_prep_slave_sg() {
    ...
	for_each_sg(sgl, sgent, sglen, i) {
		struct omap_sg *osg = &d->sg[i];
        ...
		osg->fn = sg_dma_len(sgent) / frame_bytes;
    ...
}

If a client passes 0 for maxburst, frame_bytes will be 0, leading to an
unhandled division by zero.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603030754.288757-6-rosenp@gmail.com?part=1

Re: [PATCHv3 8/8] dmaengine: ti: omap-dma: turn lch_map into a flexible array

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free due to dangling CPU PM notifier on probe failure or device unbind
--

Note: The format of this review has been altered due to recitation restrictions.
No code from the patch is directly quoted below.

commit 8df6bfe3cea1126b3e862e90d788d6cedc19c9bf
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ti: omap-dma: turn lch_map into a flexible array

This commit converts the lch_map pointer array to a C99 flexible array member 
at the end of struct omap_dmadev. The probe function is reordered to look up 
platform data and determine the logical channel count before allocation, 
collapsing two allocations into one and enabling bounds checking.

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing 
issue with CPU PM notifier registration in omap_dma_probe() and cleanup 
in omap_dma_remove().

In omap_dma_probe(), if of_dma_controller_register() fails, the code cleans
up some resources, but it doesn't return early. Instead, execution drops 
through to the end of the function, where it registers the CPU PM notifier 
by calling cpu_pm_register_notifier(&od->nb) before finally returning the 
error code. Since the probe failed, the memory for the device structure will 
be freed, but the notifier remains in the global chain.

Additionally, in omap_dma_remove(), cpu_pm_unregister_notifier() is only 
called if od->cfg->may_lose_context is true. However, in omap_dma_probe(), 
the notifier is registered if either needs_busy_check or may_lose_context 
is true. If a platform has needs_busy_check set but may_lose_context false, 
the notifier is never unregistered during device removal.

Could these paths leave a dangling pointer in the CPU PM notifier chain, 
resulting in a use-after-free when a subsequent power management event 
triggers the callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603030754.288757-9-rosenp@gmail.com?part=1

Re: [PATCHv3 4/8] dmaengine: ti: omap-dma: stop channels during teardown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-after-free on `od->desc_pool` during driver teardown.
--

commit cb697a8a2b4f2a6d911676cfaf5fc92a182d4653
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ti: omap-dma: stop channels during teardown

omap_dma_free() removes channels and frees their storage without first
stopping an active transfer. This patch terminates each channel before
removing it, and drains the virt-dma resource lists before freeing the
channel structure.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 55ece7fd0d99a..3ed2e8d91ba6d 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1520,8 +1520,10 @@ static void omap_dma_free(struct omap_dmadev *od)
>  		struct omap_chan *c = list_first_entry(&od->ddev.channels,
>  			struct omap_chan, vc.chan.device_node);
>  
> +		omap_dma_terminate_all(&c->vc.chan);
>  		list_del(&c->vc.chan.device_node);
>  		tasklet_kill(&c->vc.task);
> +		vchan_free_chan_resources(&c->vc);
[Severity: High]
Does this code introduce a use-after-free regression during driver teardown?

Looking at omap_dma_remove(), the dma pool is explicitly destroyed before
omap_dma_free() is called:

omap_dma_remove() {
    ...
    if (od->ll123_supported)
        dma_pool_destroy(od->desc_pool);

    omap_dma_free(od);
}

This new call to vchan_free_chan_resources() actively drains the virt-dma
resource lists, which invokes the driver's descriptor freeing callback,
omap_dma_desc_free(), for any pending or reusable descriptors. 

If a descriptor uses linked lists, omap_dma_desc_free() will attempt to
free it to the pool:

omap_dma_desc_free() {
    ...
    if (d->using_ll) {
        ...
        dma_pool_free(od->desc_pool, d->sg[i].t2_desc,
                      d->sg[i].t2_desc_paddr);
    }
}

Since od->desc_pool has already been destroyed, will this result in a
use-after-free when dma_pool_free() is called? Should the dma pool be
destroyed after omap_dma_free() completes?

>  		kfree(c);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603030754.288757-5-rosenp@gmail.com?part=1