[PATCH] dmaengine: pl330: Fix NULL pointer dereference in tasklet during channel release

[Mayank Mishra]

When a DMA transfer aborts (e.g., due to a translation fault), the
interrupt handler stops the channel, schedules completion callbacks,
and marks the descriptors as DONE with an error status.

Subsequent channel release via pl330_free_chan_resources() clears the
hardware thread by calling pl330_release_channel(), which reschedules
the tasklet, and then sets pch->thread to NULL.

Because pl330_free_chan_resources() and pl330_tasklet() were not
synchronized on the setting of pch->thread (holding pl330->lock and
pch->lock respectively), a TOCTOU race condition occurred:

1. pl330_tasklet() verified that pch->thread was non-NULL.
2. pl330_free_chan_resources() concurrently set pch->thread to NULL.
3. pl330_tasklet() resumed and dereferenced pch->thread->dmac->lock,
   resulting in a kernel NULL pointer dereference at offset 0x10.

Resolve this by acquiring the channel lock pch->lock before the global
pl330->lock inside pl330_free_chan_resources() when setting the thread
pointer to NULL, enforcing the driver's existing locking hierarchy
(pch->lock -> pl330->lock). Additionally, add safety checks inside
pl330_tasklet() to verify that pch->thread is valid before dereferencing.

Fixes: b3040e40675e ("DMA: PL330: Add dma api driver")
Signed-off-by: Mayank Mishra <mayankmishraa@xxxxxxxxxx>
---
 drivers/dma/pl330.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 25ba84b18704..acb9949f4a62 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2086,12 +2086,23 @@ static void pl330_tasklet(struct tasklet_struct *t)
 	fill_queue(pch);
 
 	if (list_empty(&pch->work_list)) {
-		spin_lock(&pch->thread->dmac->lock);
-		_stop(pch->thread);
-		spin_unlock(&pch->thread->dmac->lock);
-		power_down = true;
-		pch->active = false;
-	} else {
+		/*
+		 * Verify pch->thread is still valid before dereferencing
+		 * it, as it could be set to NULL asynchronously during
+		 * channel release after transfer aborts.
+		 */
+		if (pch->thread) {
+			spin_lock(&pch->thread->dmac->lock);
+			_stop(pch->thread);
+			spin_unlock(&pch->thread->dmac->lock);
+			power_down = true;
+			pch->active = false;
+		}
+	} else if (pch->thread) {
+		/*
+		 * Verify pch->thread is valid before starting it, ensuring
+		 * safe abort cleanups when channel resources are released.
+		 */
 		/* Make sure the PL330 Channel thread is active */
 		spin_lock(&pch->thread->dmac->lock);
 		pl330_start_thread(pch->thread);
@@ -2109,7 +2120,8 @@ static void pl330_tasklet(struct tasklet_struct *t)
 		if (pch->cyclic) {
 			desc->status = PREP;
 			list_move_tail(&desc->node, &pch->work_list);
-			if (power_down) {
+			/* Verify thread validity before restarting cyclic channel */
+			if (power_down && pch->thread) {
 				pch->active = true;
 				spin_lock(&pch->thread->dmac->lock);
 				pl330_start_thread(pch->thread);
@@ -2357,7 +2369,14 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
 	tasklet_kill(&pch->task);
 
 	pm_runtime_get_sync(pch->dmac->ddma.dev);
-	spin_lock_irqsave(&pl330->lock, flags);
+	/*
+	 * Acquire pch->lock before pl330->lock to respect the locking hierarchy
+	 * (pch->lock -> pl330->lock) used inside the tasklet. This ensures
+	 * that setting pch->thread to NULL and checking it inside the tasklet
+	 * is fully synchronized, preventing TOCTOU race conditions.
+	 */
+	spin_lock_irqsave(&pch->lock, flags);
+	spin_lock(&pl330->lock);
 
 	pl330_release_channel(pch->thread);
 	pch->thread = NULL;
@@ -2365,7 +2384,8 @@ static void pl330_free_chan_resources(struct dma_chan *chan)
 	if (pch->cyclic)
 		list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool);
 
-	spin_unlock_irqrestore(&pl330->lock, flags);
+	spin_unlock(&pl330->lock);
+	spin_unlock_irqrestore(&pch->lock, flags);
 	pm_runtime_put_autosuspend(pch->dmac->ddma.dev);
 	pl330_unprep_slave_fifo(pch);
 }
-- 
2.55.0.795.g602f6c329a-goog