Re: [PATCH v5 07/10] dmaengine: dw-edma: Add non_ll_start() callback

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing spinlock in dw_edma_device_resume leads to list corruption and double-free vulnerabilities.
--

--- Patch [7]: [PATCH v5 07/10] dmaengine: dw-edma: Add non_ll_start() callback ---
commit ea651917c2d3298c559360685146e9b6a9ee99c3
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Add non_ll_start() callback
    
Add a non_ll_start() callback and move the common non-linked-list channel
handling into the EDMA core so it can be shared by both the EDMA and HDMA.
Prepare for the upcoming reorganization of the burst and chunk structures.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index bab4d49c92feb..e18d6e827c2c9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h

[ ... ]

> @@ -201,7 +202,16 @@ dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  static inline
>  void dw_edma_core_start(struct dw_edma *dw, struct dw_edma_chunk *chunk, bool first)
>  {
> -	dw->core->start(chunk, first);
> +	if (chunk->chan->non_ll) {
> +		struct dw_edma_burst *child;
> +
> +		child = list_first_entry_or_null(&chunk->burst->list,
> +						 struct dw_edma_burst, list);
> +		if (child)
> +			dw->core->non_ll_start(chunk->chan, child);

[Severity: High]
This isn't a bug introduced by this patch, but does dw_edma_device_resume()
safely call dw_edma_start_transfer() without holding the virtual channel
spinlock (chan->vc.lock)?

In drivers/dma/dw-edma/dw-edma-core.c:

dw_edma_device_resume()
    chan->status = EDMA_ST_BUSY;
    dw_edma_start_transfer(chan);

When dw_edma_start_transfer() executes, it starts the hardware and then frees
the chunk descriptor:

dw_edma_start_transfer()
    dw_edma_core_start(dw, child, !desc->xfer_sz);
    desc->xfer_sz += child->xfer_sz;
    dw_edma_free_burst(child);
    list_del(&child->list);
    kfree(child);

Since hardware is started before the descriptor is safely removed from
the list and freed, and no lock is held during resume, could the hardware
complete the transfer and fire an interrupt immediately?

This would cause the interrupt handler (dw_edma_done_interrupt) to acquire
the spinlock and concurrently process the same chunk, leading to list
corruption and a double-free on the descriptor memory.

> +	} else {
> +		dw->core->start(chunk, first);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-edma_ll-v5-0-e199053d4300@nxp.com?part=7