[PATCH] dmaengine: mmp_tdma: Release IRQs before channel tasklets and storage

[PATCH] dmaengine: mmp_tdma: Release IRQs before channel tasklets and storage

[Myeonghun Pak]

The shared IRQ is requested before the channels are initialized and
released after their storage is freed. Its handler can therefore
dereference NULL pointers or freed channel state. Pending tasklets can
also outlive the channels.

Initialize both channels before requesting the shared IRQ and add managed
tasklet cleanup for each channel. This releases IRQs before draining each
tasklet and freeing its channel storage.

This issue was found by static analysis.

Fixes: c6da0ba8dfc8 ("dmaengine: mmp_tdma: add mmp tdma support")
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Compile-tested with GCC, W=1 (arm). No runtime testing.

This fix overlaps with Allen Pais's DMAengine BH v3 conversion. If that
series lands first, the cleanup should use dmaengine_kill_bh().
Link: https://lore.kernel.org/r/3cd07c8bcab908e352bf1570fb7a3c24cc0fe516.1786384168.git.allen.lkml@gmail.com/

 drivers/dma/mmp_tdma.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
index ba03321..965cdd6 100644
--- a/drivers/dma/mmp_tdma.c
+++ b/drivers/dma/mmp_tdma.c
@@ -353,6 +353,13 @@ static void dma_do_tasklet(struct tasklet_struct *t)
 	dmaengine_desc_get_callback_invoke(&tdmac->desc, NULL);
 }
 
+static void mmp_tdma_kill_tasklet(void *data)
+{
+	struct mmp_tdma_chan *tdmac = data;
+
+	tasklet_kill(&tdmac->tasklet);
+}
+
 static void mmp_tdma_free_descriptor(struct mmp_tdma_chan *tdmac)
 {
 	struct gen_pool *gpool;
@@ -562,6 +569,7 @@ static int mmp_tdma_chan_init(struct mmp_tdma_device *tdev,
 					int type, struct gen_pool *pool)
 {
 	struct mmp_tdma_chan *tdmac;
+	int ret;
 
 	if (idx >= TDMA_CHANNEL_NUM) {
 		dev_err(tdev->dev, "too many channels for device!\n");
@@ -584,6 +592,9 @@ static int mmp_tdma_chan_init(struct mmp_tdma_device *tdev,
 	tdmac->status = DMA_COMPLETE;
 	tdev->tdmac[tdmac->idx] = tdmac;
 	tasklet_setup(&tdmac->tasklet, dma_do_tasklet);
+	ret = devm_add_action_or_reset(tdev->dev, mmp_tdma_kill_tasklet, tdmac);
+	if (ret)
+		return ret;
 
 	/* add the channel to tdma_chan list */
 	list_add_tail(&tdmac->chan.device_node,
@@ -666,14 +677,6 @@ static int mmp_tdma_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	if (irq_num != chan_num) {
-		irq = platform_get_irq(pdev, 0);
-		ret = devm_request_irq(&pdev->dev, irq,
-			mmp_tdma_int_handler, IRQF_SHARED, "tdma", tdev);
-		if (ret)
-			return ret;
-	}
-
 	/* initialize channel parameters */
 	for (i = 0; i < chan_num; i++) {
 		irq = (irq_num != chan_num) ? 0 : platform_get_irq(pdev, i);
@@ -682,6 +685,15 @@ static int mmp_tdma_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	/* Release the shared IRQ before draining and freeing the channels. */
+	if (irq_num != chan_num) {
+		irq = platform_get_irq(pdev, 0);
+		ret = devm_request_irq(&pdev->dev, irq, mmp_tdma_int_handler,
+				       IRQF_SHARED, "tdma", tdev);
+		if (ret)
+			return ret;
+	}
+
 	dma_cap_set(DMA_SLAVE, tdev->device.cap_mask);
 	dma_cap_set(DMA_CYCLIC, tdev->device.cap_mask);
 	tdev->device.dev = &pdev->dev;