[PATCH 00/10] dmaengine: Fix teardown races, resource leaks, and error handling

[PATCH 00/10] dmaengine: Fix teardown races, resource leaks, and error handling

[Lee Jones]

These issues were reported by Sashiko.

Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alex Smith <alex@alex-smith.me.uk>
Cc: Andrey Volkov <avolkov@varma-el.com>
Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Dale Farnsworth <dfarnsworth@mvista.com>
Cc: dmaengine@vger.kernel.org
Cc: Frank Li <Frank.Li@kernel.org>
Cc: Gary R Hook <gary.hook@amd.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Jacky Huang <ychuang3@nuvoton.com>
Cc: Joachim Eastwood <manabian@gmail.com>
Cc: Linus Walleij <linusw@kernel.org>
Cc: linux-amlogic@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: Paul Cercueil <paul@crapouillou.net>
Cc: Peter Pearse <peter.pearse@arm.com>
Cc: Philipp Stanner <pstanner@redhat.com>
Cc: Raju Rangoju <Raju.Rangoju@amd.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Sanjay R Mehta <sanju.mehta@amd.com>
Cc: Shan-Chun Hung <schung@nuvoton.com>
Cc: Sylvain Munaut <tnt@246tNt.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Xianwei Zhao <xianwei.zhao@amlogic.com>
Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

Lee Jones (10):
  dmaengine: pl08x: Defer OF DMA controller registration until slave
    setup
  dmaengine: ae4dma: Unregister DMA engine in ae4_pci_remove()
  dmaengine: ptdma: Unmap descriptor in pt_do_cleanup()
  dmaengine: ptdma: Remove completed descriptor under vc.lock in
    pt_cmd_callback_work()
  dmaengine: amlogic: Destroy free_wq after devres DMA device
    unregistration
  dmaengine: arm-dma350: Query dma_cookie_status() under vc.lock in
    d350_tx_status()
  dmaengine: bestcomm: Defer publishing bcom_eng until initialisation
    completes
  dmaengine: bestcomm: Check rh_create() return value in
    bcom_sram_init()
  dmaengine: jz4780: Fix descriptor cleanup on hwdesc setup failure
  dmaengine: dmatest: Avoid unsigned underflow in min_odd()

 drivers/dma/amba-pl08x.c                | 23 +++++++++
 drivers/dma/amd/ae4dma/ae4dma-pci.c     |  1 +
 drivers/dma/amd/ptdma/ptdma-dmaengine.c |  4 +-
 drivers/dma/amlogic-dma.c               |  3 +-
 drivers/dma/arm-dma350.c                |  3 +-
 drivers/dma/bestcomm/bestcomm.c         | 69 +++++++++++++------------
 drivers/dma/bestcomm/sram.c             |  6 +++
 drivers/dma/dma-jz4780.c                |  5 +-
 drivers/dma/dmatest.c                   |  6 +++
 9 files changed, 80 insertions(+), 40 deletions(-)

-- 
2.56.0.567.gdb80ae8cd5-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH 05/10] dmaengine: amlogic: Destroy free_wq after devres DMA device unregistration

[Lee Jones]

aml_dma_probe() registers the DMA device with
dmaenginem_async_device_register(), so dma_async_device_unregister() is
deferred until devres release after aml_dma_remove() returns.  However,
aml_dma_remove() destroys aml_dma->free_wq directly, causing
aml_dma_free_desc() to queue work onto a destroyed workqueue when
channels and descriptors are released during devres teardown, and early
error returns in aml_dma_probe() also leak aml_dma->free_wq.

Allocate aml_dma->free_wq with devm_alloc_ordered_workqueue() so it is
destroyed on probe failure and after dmaenginem_async_device_register()
unwinds on removal.

Fixes: b84b5197e344 ("dmaengine: amlogic: Add general DMA driver for A9")
Closes: https://sashiko.dev/#/bug/linux-c216084a-f89a-48d5-bdea-584522d1bdda
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/dma/amlogic-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/amlogic-dma.c b/drivers/dma/amlogic-dma.c
index 684121d13fca..791bd82743d3 100644
--- a/drivers/dma/amlogic-dma.c
+++ b/drivers/dma/amlogic-dma.c
@@ -620,7 +620,7 @@ static int aml_dma_probe(struct platform_device *pdev)
 	if (!aml_dma)
 		return -ENOMEM;
 
-	aml_dma->free_wq = alloc_ordered_workqueue("aml_dma_free", 0);
+	aml_dma->free_wq = devm_alloc_ordered_workqueue(&pdev->dev, "aml_dma_free", 0);
 	if (!aml_dma->free_wq)
 		return -ENOMEM;
 
@@ -712,7 +712,6 @@ static void aml_dma_remove(struct platform_device *pdev)
 		if (aml_chan)
 			tasklet_kill(&aml_chan->vchan.task);
 	}
-	destroy_workqueue(aml_dma->free_wq);
 }
 
 static const struct of_device_id aml_dma_ids[] = {
-- 
2.56.0.567.gdb80ae8cd5-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH 05/10] dmaengine: amlogic: Destroy free_wq after devres DMA device unregistration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] dmaengine: amlogic: missing platform_get_irq() error check in
  aml_dma_probe() [1]
--

commit 2bde83d0cee966b150690899ee8af15d0fb91330
Author: Lee Jones <lee@kernel.org>

dmaengine: amlogic: Destroy free_wq after devres DMA device unregistration

This commit switches aml_dma->free_wq to devm_alloc_ordered_workqueue() so
that the workqueue is cleaned up on probe failure and destroyed after
devres unregisters the DMA device on removal.

> diff --git a/drivers/dma/amlogic-dma.c b/drivers/dma/amlogic-dma.c
> index 684121d13fcab..791bd82743d31 100644
> --- a/drivers/dma/amlogic-dma.c
> +++ b/drivers/dma/amlogic-dma.c
> @@ -620,7 +620,7 @@ static int aml_dma_probe(struct platform_device *pdev)
>  	if (!aml_dma)
>  		return -ENOMEM;
>  
> -	aml_dma->free_wq = alloc_ordered_workqueue("aml_dma_free", 0);
> +	aml_dma->free_wq = devm_alloc_ordered_workqueue(&pdev->dev, "aml_dma_free", 0);
>  	if (!aml_dma->free_wq)
>  		return -ENOMEM;
>  

[Severity: Medium]
This problem wasn't introduced by this patch, but should aml_dma_probe()
check aml_dma->irq for errors after calling platform_get_irq() [1]?

drivers/dma/amlogic-dma.c:aml_dma_probe() {
    ...
	aml_dma->irq = platform_get_irq(pdev, 0);
    ...
	ret = devm_request_irq(&pdev->dev, aml_dma->irq, aml_dma_interrupt_handler,
			       0, dev_name(&pdev->dev), aml_dma);
}

If platform_get_irq() returns a negative error code such as -EPROBE_DEFER,
passing aml_dma->irq to devm_request_irq() converts it to a large unsigned
IRQ number that fails with -EINVAL, turning probe deferral into a
permanent probe failure.

[1] https://sashiko.dev/#/bug/linux-bec1576a-66f2-4328-8c01-0836c0f63d2a

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111215349.1983909-1-lee@kernel.org?part=5

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic