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

[sashiko-bot@kernel.org]

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