Re: [PATCH 1/3] dmaengine: dw-axi-dmac: poll the correct enable bit during termination

[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:
- [Critical] dmaengine: dw-axi-dmac: use-after-free in
  dma_chan_terminate_all() [1]
--

commit af0999f332d674abe73e6d4b71aee90f2aef6ba8
Author: James Hilliard <james.hilliard1@gmail.com>

dmaengine: dw-axi-dmac: poll the correct enable bit during termination

This commit updates dma_chan_terminate_all() to poll axi_chan_is_hw_enable()
so that upper-bank channel enable bits (channels 16 through 31) are checked
before freeing descriptors.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index 7520c78f55ccf..49f65486e8626 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -1181,16 +1181,15 @@ static irqreturn_t dw_axi_dma_interrupt(int irq, void *dev_id)
>  static int dma_chan_terminate_all(struct dma_chan *dchan)
>  {
>  	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
> -	u32 chan_active = BIT(chan->id) << DMAC_CHAN_EN_SHIFT;
>  	unsigned long flags;
> -	u32 val;
> +	bool enabled;
>  	int ret;
>  	LIST_HEAD(head);
>  
>  	axi_chan_disable(chan);
>  
> -	ret = readl_poll_timeout_atomic(chan->chip->regs + DMAC_CHEN, val,
> -					!(val & chan_active), 1000, 50000);
> +	ret = read_poll_timeout_atomic(axi_chan_is_hw_enable, enabled, !enabled,
> +				      1000, 50000, false, chan);

[Severity: Critical]
This problem wasn't introduced by this patch, but can dma_chan_terminate_all()
race with dma_chan_issue_pending() [1] and free descriptors while the
channel is active?

axi_chan_disable() and the poll loop run before chan->vc.lock is acquired:

CPU0                                  CPU1
dma_chan_terminate_all()
  axi_chan_disable(chan)
  read_poll_timeout_atomic(...)
                                      dma_chan_issue_pending()
                                        spin_lock_irqsave(&chan->vc.lock, ...)
                                        vchan_issue_pending(&chan->vc)
                                        axi_chan_start_first_queued(chan)
                                        spin_unlock_irqrestore(...)
  spin_lock_irqsave(&chan->vc.lock, ...)
  vchan_get_all_descriptors(&chan->vc, &head)
  spin_unlock_irqrestore(...)
  vchan_dma_desc_free_list(&chan->vc, &head)  <-- use-after-free by DMA

Because CPU1 re-enables the channel after the poll finishes, the hardware
continues processing descriptors that CPU0 then collects and frees.

[1] https://sashiko.dev/#/bug/linux-d3e7b3db-d97f-48be-a317-9ef1a4cd9953

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-submit-dw-axi-dma-stop-pause-v1-v1-0-c48ec19e9ee4@gmail.com?part=1