[PATCH] dmaengine: at_xdmac: handle empty transfer list in tx_status

[PATCH] dmaengine: at_xdmac: handle empty transfer list in tx_status

[Ziheng Wei]

at_xdmac_device_terminate_all() removes every descriptor from
atchan->xfers_list without completing their cookies, so
dma_cookie_status() keeps reporting a terminated cookie as
DMA_IN_PROGRESS. at_xdmac_tx_status() then calls list_first_entry()
on the empty list and uses the list head, which lives inside struct
at_xdmac_chan, as a descriptor: active_xfer and xfer_size are read
from neighbouring channel fields, and if active_xfer happens to be
non-zero the residue loop walks a descs_list made of those fields.

For audio this is a routine path. The generic dmaengine PCM calls
dmaengine_terminate_async() on TRIGGER_STOP and later
dmaengine_tx_status() on the same cookie from
snd_dmaengine_pcm_sync_stop() and on close. What happens depends on
the struct layout. On a 32-bit build active_xfer overlays
sconfig.device_fc, which is normally false, so only a bogus residue
is reported. With CONFIG_DEBUG_SPINLOCK the larger spinlock_t puts
active_xfer on the held lock word and descs_list on the owner_cpu and
owner fields, so the loop follows a pointer that is not a descriptor.

Report a residue of 0 when no descriptor is queued.

Found by LLM-assisted source review. Not tested on hardware; the
layout analysis above is from reading the code only.

Fixes: e1f7c9eee707 ("dmaengine: at_xdmac: creation of the atmel eXtended DMA Controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ziheng Wei <zihengw03@163.com>
---

Notes:
    The changelog was drafted with an AI assistant from my analysis;
    I have reviewed the code, the fix and the text.

 drivers/dma/at_xdmac.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index ef0dfe9d3..bcfbb41ea 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -1553,7 +1553,16 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
 
 	spin_lock_irqsave(&atchan->lock, flags);
 
-	desc = list_first_entry(&atchan->xfers_list, struct at_xdmac_desc, xfer_node);
+	desc = list_first_entry_or_null(&atchan->xfers_list, struct at_xdmac_desc,
+					xfer_node);
+	/*
+	 * The list is empty once the channel has been terminated, while the
+	 * cookie of the terminated transfer is still reported as in progress.
+	 */
+	if (!desc) {
+		dma_set_residue(txstate, 0);
+		goto spin_unlock;
+	}
 
 	/*
 	 * If the transfer has not been started yet, don't need to compute the

base-commit: 97d73c8251226cf27cc6656b4c542869bb8c69fa
-- 
2.43.0

Re: [PATCH] dmaengine: at_xdmac: handle empty transfer list in tx_status

[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:
- [High] dmaengine: at_xdmac: unchecked list_first_entry() in
  at_xdmac_tasklet() [1]
--

commit 7130c1ad653839b18d68794e4d33147aad0531b5
Author: Ziheng Wei <zihengw03@163.com>

dmaengine: at_xdmac: handle empty transfer list in tx_status

This commit uses list_first_entry_or_null() in at_xdmac_tx_status() and
reports a zero residue when atchan->xfers_list is empty after channel
termination.

> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index ef0dfe9d37ba3..bcfbb41ea4bc1 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1553,7 +1553,16 @@ at_xdmac_tx_status(struct dma_chan *chan, dma_cookie_t cookie,
>  
>  	spin_lock_irqsave(&atchan->lock, flags);
>  
> -	desc = list_first_entry(&atchan->xfers_list, struct at_xdmac_desc, xfer_node);
> +	desc = list_first_entry_or_null(&atchan->xfers_list, struct at_xdmac_desc,
> +					xfer_node);
> +	/*
> +	 * The list is empty once the channel has been terminated, while the
> +	 * cookie of the terminated transfer is still reported as in progress.
> +	 */
> +	if (!desc) {
> +		dma_set_residue(txstate, 0);
> +		goto spin_unlock;
> +	}

[Severity: High]
This problem wasn't introduced by this patch, but can at_xdmac_tasklet()
and at_xdmac_handle_error() also call list_first_entry() on an empty
atchan->xfers_list [1]?

CPU0                                  CPU1
at_xdmac_interrupt()
  atchan->irq_status = ...
  tasklet_schedule(&atchan->tasklet)
                                      at_xdmac_device_terminate_all()
                                        spin_lock_irqsave(&atchan->lock)
                                        list_del(&desc->xfer_node)
                                        clear_bit(AT_XDMAC_CHAN_IS_CYCLIC)
                                        spin_unlock_irqrestore(...)
at_xdmac_tasklet()
  spin_lock_irq(&atchan->lock)
  desc = list_first_entry(&atchan->xfers_list, ...)
  if (!desc->active_xfer)             <-- out-of-bounds read
  list_del(&desc->xfer_node)          <-- list corruption

at_xdmac_device_terminate_all() empties atchan->xfers_list and clears the
cyclic flag without clearing atchan->irq_status or stopping the scheduled
tasklet.  When at_xdmac_tasklet() runs, it sees the stale
atchan->irq_status bits and computes a bogus descriptor pointer from the
empty list head, causing an out-of-bounds read and corrupting
atchan->xfers_list and atchan->free_descs_list if desc->active_xfer reads
as non-zero.

[1] https://sashiko.dev/#/bug/linux-bba900ca-f8ae-4ff8-a433-e3d7588cc938

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115080245.910534-1-zihengw03@163.com?part=1