Re: [PATCH v4 8/9] PCI: epf-mhi: Use dmaengine_prep_config_single() to simplify code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Asymmetric DMA workqueue teardown risks Use-After-Free on late callbacks.
- [High] Synchronous transfer timeouts indiscriminately kill concurrent async transfers, leaking their memory and mappings.
--

commit 05444e0385986a82cafbe547e6d0ad335a0e1fbb
Author: Frank Li <Frank.Li@nxp.com>

PCI: epf-mhi: Use dmaengine_prep_config_single() to simplify code

Use dmaengine_prep_config_single() to simplify pci_epf_mhi_edma_read[_sync]()
and pci_epf_mhi_edma_write[_sync](). No functional change.

> diff --git a/drivers/pci/endpoint/functions/pci-epf-mhi.c b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> index 7f5326925ed54..c3e3b58fb86cd 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-mhi.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-mhi.c
> @@ -342,9 +336,10 @@ static int pci_epf_mhi_edma_read(struct mhi_ep_cntrl *mhi_cntrl,
>  		goto err_unlock;
>  	}
>  
> -	desc = dmaengine_prep_slave_single(chan, dst_addr, buf_info->size,
> -					   DMA_DEV_TO_MEM,
> -					   DMA_CTRL_ACK | DMA_PREP_INTERRUPT);
> +	desc = dmaengine_prep_config_single(chan, dst_addr, buf_info->size,
> +					    DMA_DEV_TO_MEM,
> +					    DMA_CTRL_ACK | DMA_PREP_INTERRUPT,
> +					    &config);
>  	if (!desc) {
>  		dev_err(dev, "Failed to prepare DMA\n");
>  		ret = -EIO;

This isn't a bug introduced by this commit, but in pci_epf_mhi_edma_read() and
pci_epf_mhi_edma_write(), if the synchronous transfer times out (e.g., if the
channel is stuck processing a prior async transfer), the error recovery path
calls dmaengine_terminate_sync(chan):

	ret = wait_for_completion_timeout(&complete, msecs_to_jiffies(1000));
	if (!ret) {
		dev_err(dev, "DMA transfer timeout\n");
		dmaengine_terminate_sync(chan);
		ret = -ETIMEDOUT;
	} else {

Because async transfers run concurrently and share the channel, does this
abruptly abort them without invoking their completion callbacks?

Will this leak the transfer structures and their dma_map_single()
mappings, and stall the MHI endpoint data channel?

[ ... ]

This isn't a bug introduced by this commit, but looking at
pci_epf_mhi_dma_deinit(), it destroys the workqueue before releasing the
active DMA channels:

	destroy_workqueue(epf_mhi->dma_wq);
	dma_release_channel(epf_mhi->dma_chan_tx);
	dma_release_channel(epf_mhi->dma_chan_rx);

If an asynchronous DMA transfer completes right after the workqueue is
destroyed but before the channel is fully released, the dmaengine interrupt
handler will invoke pci_epf_mhi_dma_async_callback(). This callback
unconditionally calls queue_work(epf_mhi->dma_wq, ...).

Does attempting to enqueue work onto a destroyed workqueue cause a
use-after-free memory corruption?

Should the channels be explicitly terminated with dmaengine_terminate_sync()
before the workqueue is destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506-dma_prep_config-v4-0-85b3d22babff@nxp.com?part=8