[PATCH v2 0/5] dmaengine: ti: omap-dma: various bug fixes

[PATCH v2 0/5] dmaengine: ti: omap-dma: various bug fixes

[Rosen Penev]

This series fixes four bugs in the TI OMAP DMA driver:

 - Patch 1: add missing return statement in the probe error path
 - Patch 2: fix a notifier leak in remove
 - Patch 3: fix dma_pool_destroy being called before omap_dma_free
 - Patch 4: fix interrupt handling in the remove path
 - Patch 5: use dmam for dmaengine registration and remove devm_free_irq

v2: fix sashiko comments and add extra patch

Rosen Penev (5):
  dmaengine: ti: omap-dma: fix missing return in probe error path
  dmaengine: ti: omap-dma: fix notifier leak in remove
  dmaengine: ti: omap-dma: fix dma_pool_destroy before omap_dma_free in
    error paths
  dmaengine: ti: omap-dma: fix interrupt handling in remove
  dmaengine: ti: omap-dma: use devm for dmaengine registration

 drivers/dma/ti/omap-dma.c | 50 ++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 17 deletions(-)

--
2.54.0

[PATCH v2 1/5] 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

[PATCH v2 2/5] dmaengine: ti: omap-dma: fix notifier leak in remove

[Rosen Penev]

The notifier may be registered for needs_busy_check (omap2420) rather than
may_lose_context (omap3). The remove path only checks may_lose_context,
leaving the notifier registered during driver removal. Check both flags.

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 | 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 0f6dd6b0a301..839e04f53fc2 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1853,7 +1853,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);
 
 	if (pdev->dev.of_node)
-- 
2.54.0

Re: [PATCH v2 2/5] dmaengine: ti: omap-dma: fix notifier leak in remove

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Potential Use-After-Free of notifier_block due to missing RCU synchronization.
--

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

dmaengine: ti: omap-dma: fix notifier leak in remove

This commit fixes a notifier leak during driver removal. It ensures the CPU PM
notifier is unregistered if either the needs_busy_check or may_lose_context
flag is set, rather than only checking may_lose_context.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1853,7 +1853,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);

[Severity: Critical]
This is a pre-existing issue, but does this unregister path leave the
notifier_block vulnerable to a use-after-free?

In omap_dma_remove(), cpu_pm_unregister_notifier() unlinks the notifier
block from the raw notifier chain but does not wait for an RCU grace period.
Because the od structure is managed by devres (allocated via devm_kzalloc),
it gets freed automatically shortly after omap_dma_remove() returns.

If a concurrent CPU PM transition (like another CPU entering or exiting idle
states) traverses the raw notifier chain using rcu_read_lock(), couldn't the
traversing CPU dereference the freshly freed notifier_block?

Would adding a synchronization mechanism for RCU before the driver is
released prevent this regression?

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

[PATCH v2 3/5] dmaengine: ti: omap-dma: fix dma_pool_destroy before omap_dma_free in error paths

[Rosen Penev]

The probe error paths and remove path called omap_dma_free() after
dma_pool_destroy(), leaving the desc_pool dangling while freeing
channel descriptors that may reference it.  Reorder so omap_dma_free()
comes first.

While here, fix two additional pre-existing issues:

- The probe error paths that run after the IRQ handler is registered
  did not disable hardware interrupts before freeing channels.  If an
  IRQ fires concurrently, the handler could access freed channel
  memory via lch_map[].  Disable IRQENABLE_L1 and clear
  irq_enable_mask under the spinlock, then readback the register to
  flush the posted write, before calling omap_dma_free().

- omap_dma_free() did not drain the virt-dma descriptor lists
  (desc_allocated, desc_submitted, desc_issued, desc_completed) before
  kfree() of the channel.  This leaked pending descriptors when the
  error or remove path tore down channels without going through
  omap_dma_free_chan_resources().  Call vchan_free_chan_resources()
  before freeing the channel structure.

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 | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 839e04f53fc2..dde270646bb9 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1522,6 +1522,7 @@ static void omap_dma_free(struct omap_dmadev *od)
 
 		list_del(&c->vc.chan.device_node);
 		tasklet_kill(&c->vc.task);
+		vchan_free_chan_resources(&c->vc);
 		kfree(c);
 	}
 }
@@ -1808,9 +1809,14 @@ 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);
+		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);
+		omap_dma_free(od);
 		if (od->ll123_supported)
 			dma_pool_destroy(od->desc_pool);
-		omap_dma_free(od);
 		return rc;
 	}
 
@@ -1825,9 +1831,14 @@ 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);
+			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);
+			omap_dma_free(od);
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
-			omap_dma_free(od);
 			return rc;
 		}
 	}
@@ -1869,10 +1880,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

Re: [PATCH v2 3/5] dmaengine: ti: omap-dma: fix dma_pool_destroy before omap_dma_free in error paths

[sashiko-bot]

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

New issues:
- [Medium] Unconditional write to `IRQENABLE_L1` in `omap_dma_probe` error paths triggers WARN_ON splats on OMAP1 legacy platforms.

Pre-existing issues:
- [Critical] The driver teardown paths free channel structures and destroy the DMA pool without explicitly stopping actively running hardware DMA channels.
- [High] The patch attempts to fix a memory leak of channel descriptors, but misses the actively executing descriptor `c->desc`.
--

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

dmaengine: ti: omap-dma: fix dma_pool_destroy before omap_dma_free in error paths

The probe error paths and remove path called omap_dma_free() after
dma_pool_destroy(), leaving the desc_pool dangling. This patch reorders
the teardown sequence, disables hardware interrupts in probe error
paths, and drains virt-dma descriptor lists in omap_dma_free().

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 839e04f53fc2d..dde270646bb94 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1522,6 +1522,7 @@ static void omap_dma_free(struct omap_dmadev *od)
>  
>  		list_del(&c->vc.chan.device_node);
>  		tasklet_kill(&c->vc.task);
> +		vchan_free_chan_resources(&c->vc);

[Severity: High]
This is a pre-existing issue, but does this leak the actively executing
descriptor?

When a descriptor is issued to the hardware, omap_dma_start_desc() removes
it from the virt-dma lists and assigns it to c->desc. Since
vchan_free_chan_resources() only drains the virt-dma lists and does not
inspect c->desc, could the active descriptor and its associated hardware
pool allocations be permanently leaked before the channel is freed?

> +		kfree(c);

[Severity: Critical]
This isn't a bug introduced by this patch, but are the channels freed here
without stopping the underlying hardware DMA engine?

If omap_dma_stop() or omap_dma_terminate_all() is never called to halt the
hardware channel, the DMA hardware might continue executing transfers
asynchronously. Could this cause the hardware to scribble over memory when
fetching descriptors, especially since the DMA pool is destroyed shortly
after in omap_dma_remove() and the probe error paths?

>  	}
>  }

[ ... ]

> @@ -1808,9 +1809,14 @@ 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);
> +		spin_lock_irq(&od->irq_lock);
> +		od->irq_enable_mask = 0;
> +		omap_dma_glbl_write(od, IRQENABLE_L1, 0);

[Severity: Medium]
Does this unconditional write to IRQENABLE_L1 trigger a WARN_ON splat on
OMAP1 legacy platforms?

On OMAP1 legacy platforms where omap_dma_legacy(od) is true, IRQENABLE_L1
is not mapped in reg_map and evaluates to a zero-initialized register with
type OMAP_DMA_REG_NONE (0). Attempting to read or write OMAP_DMA_REG_NONE
unconditionally triggers a WARN_ON(1) inside omap_dma_write() and
omap_dma_read().

Should this interrupt disable logic be guarded by a check for
!omap_dma_legacy(od) as done in omap_dma_remove()?

> +		spin_unlock_irq(&od->irq_lock);
> +		omap_dma_glbl_read(od, IRQENABLE_L1);
> +		omap_dma_free(od);

[ ... ]

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

[PATCH v2 4/5] 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.

6. omap_dma_free() unconditionally freed all channel memory without
   checking whether clients still held references. A sysfs unbind of the
   DMA controller does not synchronously unbind consumers, so active
   clients could access freed channel memory. Skip freeing channels
   that still have active clients.

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 | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index dde270646bb9..8c32b7ab50f6 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1516,13 +1516,21 @@ 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);
 
 		list_del(&c->vc.chan.device_node);
 		tasklet_kill(&c->vc.task);
 		vchan_free_chan_resources(&c->vc);
+		if (c->vc.chan.client_count) {
+			dev_warn(od->ddev.dev,
+				 "chan%d freed with %u client(s)\n",
+				 c->dma_ch, c->vc.chan.client_count);
+			continue;
+		}
 		kfree(c);
 	}
 }
@@ -1870,16 +1878,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

Re: [PATCH v2 4/5] dmaengine: ti: omap-dma: fix interrupt handling in remove

[sashiko-bot]

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

New issues:
- [High] Skipping the deallocation of active DMA channels introduces a memory leak and causes a Use-After-Free of the devres-managed parent controller.
- [Medium] Missing `!omap_dma_legacy(od)` guard in probe error paths triggers WARN_ON(1) on OMAP1 platforms.

Pre-existing issues:
- [High] Virtual channel resources and descriptor pools are unconditionally freed before halting active hardware DMA channels, leading to memory corruption.
--

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

dmaengine: ti: omap-dma: fix interrupt handling in remove

This commit addresses several bugs in the driver's remove path, including
incorrect IRQ disabling and missing locks. It also changes omap_dma_free()
to avoid freeing channel memory if clients still hold references. The goal
is to ensure safe driver removal and prevent resource leaks.

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index dde270646bb94..8c32b7ab50f63 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1516,13 +1516,21 @@ 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);
>  
>  		list_del(&c->vc.chan.device_node);
>  		tasklet_kill(&c->vc.task);
>  		vchan_free_chan_resources(&c->vc);

[Severity: High]
This is a pre-existing issue, but does this prematurely free resources while
the hardware is still active?

In omap_dma_free(), virtual channel resources and descriptors are
unconditionally torn down before verifying if the client is still using the
channel and before halting active hardware DMA channels.

If a channel is actively transferring data, its physical descriptors are
returned to the pool before the hardware is stopped. When dma_pool_destroy()
is later called in omap_dma_remove(), won't the still-running DMA hardware
continue accessing the freed descriptor memory, leading to memory corruption?

> +		if (c->vc.chan.client_count) {
> +			dev_warn(od->ddev.dev,
> +				 "chan%d freed with %u client(s)\n",
> +				 c->dma_ch, c->vc.chan.client_count);
> +			continue;
> +		}

[Severity: High]
Does skipping the channel deallocation here introduce a Use-After-Free for
the parent controller?

By using continue here in omap_dma_free() without freeing the channel, we
leave the channel exposed to the active client. When omap_dma_remove()
finishes, devres will automatically free the parent controller struct
omap_dmadev.

When the client later attempts to release the channel:

drivers/dma/dmaengine.c:dma_release_channel() {
    ...
    if (--chan->device->privatecnt == 0)
        dma_cap_clear(DMA_PRIVATE, chan->device->cap_mask);
    ...
}

Doesn't this inevitably dereference the freed chan->device (which points to
the now-freed omap_dmadev), causing a Use-After-Free?

>  		kfree(c);
>  	}
>  }

[ ... ]

> @@ -1870,16 +1878,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);
>  	}

[Severity: Medium]
While this patch correctly adds the !omap_dma_legacy(od) guard here in
omap_dma_remove(), earlier patches in this series added similar IRQENABLE_L1
writes to the omap_dma_probe() error paths without this guard.

On OMAP1 legacy platforms where IRQENABLE_L1 is not defined in the reg_map
array, passing it to omap_dma_glbl_write() and omap_dma_glbl_read()
unconditionally triggers WARN_ON(1) during probe failure.

Should the probe error paths be updated to include this same
!omap_dma_legacy(od) check?

>  
> +	irq = platform_get_irq(pdev, 1);
> +	if (irq > 0)
> +		devm_free_irq(&pdev->dev, irq, od);
> +
>  	omap_dma_free(od);
>  
>  	if (od->ll123_supported)

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

[PATCH v2 5/5] dmaengine: ti: omap-dma: use devm for dmaengine registration

[Rosen Penev]

Use dmaenginem_async_device_register() instead of
dma_async_device_register() so that unregistration is handled
automatically by devres on probe failure or driver detach. This
lets us drop:
- The explicit dma_async_device_unregister() in the
  of_dma_controller_register error path (devres unwind runs it
  after omap_dma_free() has emptied the channels list, making the
  iteration a safe no-op).
- The explicit dma_async_device_unregister() and the redundant
  devm_free_irq() call in the remove path (devres handles both in
  the correct order after the remove callback returns).

Assisted-by: Opencode:Big-Pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 8c32b7ab50f6..4bf34569d82b 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1813,7 +1813,7 @@ static int omap_dma_probe(struct platform_device *pdev)
 		}
 	}
 
-	rc = dma_async_device_register(&od->ddev);
+	rc = dmaenginem_async_device_register(&od->ddev);
 	if (rc) {
 		pr_warn("OMAP-DMA: failed to register slave DMA engine device: %d\n",
 			rc);
@@ -1838,7 +1838,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 				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);
 			spin_lock_irq(&od->irq_lock);
 			od->irq_enable_mask = 0;
 			omap_dma_glbl_write(od, IRQENABLE_L1, 0);
@@ -1870,7 +1869,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 static void omap_dma_remove(struct platform_device *pdev)
 {
 	struct omap_dmadev *od = platform_get_drvdata(pdev);
-	int irq;
 
 	if (od->cfg->needs_busy_check || od->cfg->may_lose_context)
 		cpu_pm_unregister_notifier(&od->nb);
@@ -1878,8 +1876,6 @@ static void omap_dma_remove(struct platform_device *pdev)
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
 
-	dma_async_device_unregister(&od->ddev);
-
 	if (!omap_dma_legacy(od)) {
 		spin_lock_irq(&od->irq_lock);
 		od->irq_enable_mask = 0;
@@ -1888,10 +1884,6 @@ static void omap_dma_remove(struct platform_device *pdev)
 		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

Re: [PATCH v2 5/5] dmaengine: ti: omap-dma: use devm for dmaengine registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deferring `dma_async_device_unregister()` to devres causes channel unregistration to be bypassed, leading to massive memory/IDA leaks and use-after-free vulnerabilities via sysfs.
--

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

dmaengine: ti: omap-dma: use devm for dmaengine registration

This commit switches to dmaenginem_async_device_register() to handle
unregistration automatically via devres. It removes the explicit
dma_async_device_unregister() calls from the probe error path and remove
function.

>     - The explicit dma_async_device_unregister() in the
>       of_dma_controller_register error path (devres unwind runs it
>       after omap_dma_free() has emptied the channels list, making the
>       iteration a safe no-op).

[Severity: High]
Is it actually safe for the channels list to be emptied before
dma_async_device_unregister() runs?

The commit message mentions this is a safe no-op, but looking at
omap_dma_free(), it empties the channel list and frees the structs:

drivers/dma/ti/omap-dma.c:omap_dma_free() {
    ...
    while (!list_empty(&od->ddev.channels)) {
        c = list_first_entry(&od->ddev.channels, ...);
        list_del(&c->vc.chan.device_node);
        ...
        kfree(c);
    }
}

Later, when the devres-managed dma_async_device_unregister() executes, it
iterates over the now-empty list:

drivers/dma/dmaengine.c:dma_async_device_unregister() {
    ...
    list_for_each_entry_safe(chan, n, &device->channels, device_node)
        __dma_async_device_channel_unregister(device, chan);
    ...
}

Since the core unregistration iteration is bypassed, won't this silently
skip channel teardown? It looks like this would leak IDAs and leave active
sysfs nodes with dangling pointers to the freed channel memory. If userspace
accesses these sysfs attributes later, could it lead to a use-after-free?

> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index 8c32b7ab50f63..4bf34569d82bb 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c

[ ... ]

> @@ -1878,8 +1876,6 @@ static void omap_dma_remove(struct platform_device *pdev)
>  	if (pdev->dev.of_node)
>  		of_dma_controller_free(pdev->dev.of_node);
>  
> -	dma_async_device_unregister(&od->ddev);
> -
>  	if (!omap_dma_legacy(od)) {
>  		spin_lock_irq(&od->irq_lock);
>  		od->irq_enable_mask = 0;

[ ... ]

> @@ -1888,10 +1884,6 @@ static void omap_dma_remove(struct platform_device *pdev)
>  		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)

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