[PATCH v4] dmaengine: sun6i-dma: Fix use-after-free in error handling paths

[PATCH v4] dmaengine: sun6i-dma: Fix use-after-free in error handling paths

[Hongling Zeng]

In error handling paths, the for loop frees v_lli in the loop body,
then accesses v_lli->v_lli_next and v_lli->p_lli_next in the
increment expression, which is use-after-free.

Fix by saving both the next virtual and physical pointers before
freeing the current node.

Fixes: 555859308723 ("dmaengine: Add driver for Allwinner sun6i DMA")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Suggested-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

---
Changes in v2:
 -Refactored the fix to avoid code duplication by creating a helper function
  sun6i_dma_free_lli_list() that handles LLI list cleanup
 -Add Suggested-by: Jernej Skrabec <jernej.skrabec@gmail.com>

---
Change in v3:
 -Further refactoring to move txd handling into the helper function
  as suggested by Jernej
---
Change in v4:
 -Add reviewed-by
---
 drivers/dma/sun6i-dma.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index fcc88a74d821..e161fc298b86 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -406,16 +406,12 @@ static inline void sun6i_dma_dump_lli(struct sun6i_vchan *vchan,
 		v_lli->len, v_lli->para, v_lli->p_lli_next);
 }
 
-static void sun6i_dma_free_desc(struct virt_dma_desc *vd)
+static void sun6i_dma_free_desc(struct sun6i_dma_dev *sdev,
+				struct sun6i_desc *txd)
 {
-	struct sun6i_desc *txd = to_sun6i_desc(&vd->tx);
-	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vd->tx.chan->device);
 	struct sun6i_dma_lli *v_lli, *v_next;
 	dma_addr_t p_lli, p_next;
 
-	if (unlikely(!txd))
-		return;
-
 	p_lli = txd->p_lli;
 	v_lli = txd->v_lli;
 
@@ -432,6 +428,17 @@ static void sun6i_dma_free_desc(struct virt_dma_desc *vd)
 	kfree(txd);
 }
 
+static void sun6i_dma_free_desc_virt(struct virt_dma_desc *vd)
+{
+	struct sun6i_desc *txd = to_sun6i_desc(&vd->tx);
+	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vd->tx.chan->device);
+
+	if (unlikely(!txd))
+		return;
+
+	sun6i_dma_free_desc(sdev, txd);
+}
+
 static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
 {
 	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vchan->vc.chan.device);
@@ -788,10 +795,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
 
 err_lli_free:
-	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
-	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
-		dma_pool_free(sdev->pool, v_lli, p_lli);
-	kfree(txd);
+	sun6i_dma_free_desc(sdev, txd);
 	return NULL;
 }
 
@@ -869,10 +873,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
 	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
 
 err_lli_free:
-	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
-	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
-		dma_pool_free(sdev->pool, v_lli, p_lli);
-	kfree(txd);
+	sun6i_dma_free_desc(sdev, txd);
 	return NULL;
 }
 
@@ -1432,7 +1433,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 		struct sun6i_vchan *vchan = &sdc->vchans[i];
 
 		INIT_LIST_HEAD(&vchan->node);
-		vchan->vc.desc_free = sun6i_dma_free_desc;
+		vchan->vc.desc_free = sun6i_dma_free_desc_virt;
 		vchan_init(&vchan->vc, &sdc->slave);
 	}
 
-- 
2.25.1

Re: [PATCH v4] dmaengine: sun6i-dma: Fix use-after-free in error handling paths

[sashiko-bot]

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

Pre-existing issues:
- [High] Race condition in IRQ handler leading to NULL pointer dereference
- [High] Missing tasklet_schedule in sun6i_dma_resume causes stalled transfers
--

--- Patch [1]: [PATCH v4] dmaengine: sun6i-dma: Fix use-after-free in error handling paths ---
commit ece21d8f10ca9ff0dd6adbfa89c44eada4329522
Author: Hongling Zeng <zenghongling@kylinos.cn>

dmaengine: sun6i-dma: Fix use-after-free in error handling paths

The commit refactors the error handling paths in the sun6i-dma driver by
introducing the helper functions sun6i_dma_free_desc() and
sun6i_dma_free_desc_virt(). This resolves a use-after-free issue when
iterating and freeing a virtual LLI list in sun6i_dma_prep_slave_sg() and
sun6i_dma_prep_dma_cyclic().

> drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
> ...
> 			if (vchan && (status & vchan->irq_type)) {
> 				if (vchan->cyclic) {
> 					vchan_cyclic_callback(&pchan->desc->vd);
> 				} else {
> 					spin_lock(&vchan->vc.lock);
> 					vchan_cookie_complete(&pchan->desc->vd);
> 					pchan->done = pchan->desc;
> 					spin_unlock(&vchan->vc.lock);
> 				}
> 			}
> ...
> }

[Severity: High]
This is a pre-existing issue, but could this cause a NULL pointer dereference?

Looking at sun6i_dma_terminate_all(), it clears pchan->desc:

> drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
> ...
> 	if (pchan) {
> 		writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
> 		writel(DMA_CHAN_PAUSE_RESUME, pchan->base + DMA_CHAN_PAUSE);
> 
> 		vchan->phy = NULL;
> 		pchan->vchan = NULL;
> 		pchan->desc = NULL;
> 		pchan->done = NULL;
> 	}
> ...
> }

If sun6i_dma_terminate_all() runs concurrently with sun6i_dma_interrupt() on
SMP systems, sun6i_dma_terminate_all() holds vchan->vc.lock, stops the
hardware, and sets pchan->desc = NULL. Meanwhile, the IRQ handler reads
pchan->vchan locklessly.

If the IRQ handler evaluates this before it is cleared, it will proceed. For
non-cyclic transfers, the IRQ handler blocks on vchan->vc.lock. When
sun6i_dma_terminate_all() releases the lock, the IRQ handler continues and
evaluates &pchan->desc->vd. Since pchan->desc is NULL, could this pass a NULL
pointer to vchan_cookie_complete() and cause a kernel panic? For cyclic
transfers, the race appears even wider since the handler evaluates
&pchan->desc->vd completely locklessly.

> drivers/dma/sun6i-dma.c:sun6i_dma_resume() {
> ...
> 	} else if (!list_empty(&vchan->vc.desc_issued)) {
> 		spin_lock(&sdev->lock);
> 		list_add_tail(&vchan->node, &sdev->pending);
> 		spin_unlock(&sdev->lock);
> 	}
> ...
> }

[Severity: High]
This is a pre-existing issue, but does this code leave transfers stalled
indefinitely?

If a DMA channel is paused (sun6i_dma_pause) while it has pending descriptors
but is not yet assigned to a physical hardware channel (pchan == NULL), it is
removed from the sdev->pending list.

When the channel is later resumed here in sun6i_dma_resume(), it is re-added
to sdev->pending. However, sun6i_dma_resume() fails to call
tasklet_schedule(&sdev->task). Since the tasklet is responsible for pulling
virtual channels off the pending list and assigning them to physical channels,
will this channel sit in the pending queue indefinitely until an independent
event schedules the tasklet?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708032518.50886-1-zenghongling@kylinos.cn?part=1

Re: [PATCH v4] dmaengine: sun6i-dma: Fix use-after-free in error handling paths

[Frank Li]

On Wed, Jul 08, 2026 at 11:25:18AM +0800, Hongling Zeng wrote:
> In error handling paths, the for loop frees v_lli in the loop body,
> then accesses v_lli->v_lli_next and v_lli->p_lli_next in the
> increment expression, which is use-after-free.
>
> Fix by saving both the next virtual and physical pointers before
> freeing the current node.
>

Is this statement out of date? You call sun6i_dma_free_desc_virt()
where saving "both the next virtual and physical pointers..."?

Frank

> Fixes: 555859308723 ("dmaengine: Add driver for Allwinner sun6i DMA")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Suggested-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> ---
> Changes in v2:
>  -Refactored the fix to avoid code duplication by creating a helper function
>   sun6i_dma_free_lli_list() that handles LLI list cleanup
>  -Add Suggested-by: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> ---
> Change in v3:
>  -Further refactoring to move txd handling into the helper function
>   as suggested by Jernej
> ---
> Change in v4:
>  -Add reviewed-by
> ---
>  drivers/dma/sun6i-dma.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index fcc88a74d821..e161fc298b86 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -406,16 +406,12 @@ static inline void sun6i_dma_dump_lli(struct sun6i_vchan *vchan,
>  		v_lli->len, v_lli->para, v_lli->p_lli_next);
>  }
>
> -static void sun6i_dma_free_desc(struct virt_dma_desc *vd)
> +static void sun6i_dma_free_desc(struct sun6i_dma_dev *sdev,
> +				struct sun6i_desc *txd)
>  {
> -	struct sun6i_desc *txd = to_sun6i_desc(&vd->tx);
> -	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vd->tx.chan->device);
>  	struct sun6i_dma_lli *v_lli, *v_next;
>  	dma_addr_t p_lli, p_next;
>
> -	if (unlikely(!txd))
> -		return;
> -
>  	p_lli = txd->p_lli;
>  	v_lli = txd->v_lli;
>
> @@ -432,6 +428,17 @@ static void sun6i_dma_free_desc(struct virt_dma_desc *vd)
>  	kfree(txd);
>  }
>
> +static void sun6i_dma_free_desc_virt(struct virt_dma_desc *vd)
> +{
> +	struct sun6i_desc *txd = to_sun6i_desc(&vd->tx);
> +	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vd->tx.chan->device);
> +
> +	if (unlikely(!txd))
> +		return;
> +
> +	sun6i_dma_free_desc(sdev, txd);
> +}
> +
>  static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
>  {
>  	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vchan->vc.chan.device);
> @@ -788,10 +795,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
>  	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
>
>  err_lli_free:
> -	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
> -	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
> -		dma_pool_free(sdev->pool, v_lli, p_lli);
> -	kfree(txd);
> +	sun6i_dma_free_desc(sdev, txd);
>  	return NULL;
>  }
>
> @@ -869,10 +873,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
>  	return vchan_tx_prep(&vchan->vc, &txd->vd, flags);
>
>  err_lli_free:
> -	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
> -	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
> -		dma_pool_free(sdev->pool, v_lli, p_lli);
> -	kfree(txd);
> +	sun6i_dma_free_desc(sdev, txd);
>  	return NULL;
>  }
>
> @@ -1432,7 +1433,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  		struct sun6i_vchan *vchan = &sdc->vchans[i];
>
>  		INIT_LIST_HEAD(&vchan->node);
> -		vchan->vc.desc_free = sun6i_dma_free_desc;
> +		vchan->vc.desc_free = sun6i_dma_free_desc_virt;
>  		vchan_init(&vchan->vc, &sdc->slave);
>  	}
>
> --
> 2.25.1
>