[PATCH v2] mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

[PATCH v2] mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

[Samarth Narang]

When DMA completion times out in gpmi_nfc_exec_op(), the driver unmaps
the operation's buffers and returns -ETIMEDOUT without stopping the APBH
DMA channel. Since mxs-dma leaves the channel in DMA_IN_PROGRESS, the
pending descriptor chain may resume after the timeout, while a
subsequent operation can append new descriptors to the stale chain and
cause the old DATA_IN transfer to execute again against an unmapped
buffer.

This can result in DMA writes to memory that has already been released,
including buffers freed after ONFI parameter page detection in
nand_onfi_detect().

Terminate the channel with dmaengine_terminate_sync() before unmapping
so that outstanding transfers are stopped and subsequent operations
begin with a fresh descriptor chain.

Fixes: ef347c0cfd61 ("mtd: rawnand: gpmi: Implement exec_op")
Assisted-by: LLM
Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
---
Changes in v2:
- Drop the code comment

v1: https://lore.kernel.org/all/20261030143945.2923554-1-samarth.colleges@gmail.com/

This issue was identified by a research static analyzer for DMA
lifecycle ordering and has not been validated on hardware.
Compile-tested only.

 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index 527165ccc..a8106de2c 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -2659,6 +2659,7 @@ static int gpmi_nfc_exec_op(struct nand_chip *chip,
 	if (!to) {
 		dev_err(this->dev, "DMA timeout, last DMA\n");
 		gpmi_dump_info(this);
+		dmaengine_terminate_sync(get_dma_chan(this));
 		ret = -ETIMEDOUT;
 		goto unmap;
 	}

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907

Re: [PATCH v2] mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: rawnand: gpmi: unsynchronized DMA termination in gpmi_nfc_exec_op()
--

commit f9aac792f98d50292f01d08b01dd1b06dcf84ac0
Author: Samarth Narang <samarth.colleges@gmail.com>

mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

This commit calls dmaengine_terminate_sync() before unmapping DMA buffers
when a DMA completion timeout occurs in gpmi_nfc_exec_op(), intending to
stop the APBH DMA channel and prevent stale descriptors from running against
unmapped buffers.

> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 527165ccc839d..86163a552aad8 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -2659,6 +2659,7 @@ static int gpmi_nfc_exec_op(struct nand_chip *chip,
>  	if (!to) {
>  		dev_err(this->dev, "DMA timeout, last DMA\n");
>  		gpmi_dump_info(this);
> +		dmaengine_terminate_sync(get_dma_chan(this));

[Severity: Medium]
Does dmaengine_terminate_sync() actually synchronize with the channel IRQ
and completion tasklet on mxs-dma?

In drivers/dma/mxs-dma.c, mxs_dma_probe() does not implement
device_synchronize, so dmaengine_synchronize() is a no-op, and
mxs_dma_terminate_all() resets the channel without killing mxs_chan->tasklet
or clearing mxs_chan->desc.callback:

drivers/dma/mxs-dma.c:mxs_dma_terminate_all() {
	mxs_dma_reset_chan(chan);
	mxs_dma_disable_chan(chan);

	return 0;
}

Furthermore, when mxs_dma_reset_chan() resets an active APBH channel (or if
a late completion interrupt races with the timeout), mxs_dma_int_handler()
fires for the termination error (err == 0x01) or completion and schedules
mxs_chan->tasklet:

drivers/dma/mxs-dma.c:mxs_dma_int_handler() {
	...
	if (err) {
		dev_dbg(mxs_dma->dma_device.dev,
			"%s: error in channel %d\n", __func__,
			chan);
		mxs_chan->status = DMA_ERROR;
		mxs_dma_reset_chan(&mxs_chan->chan);
	}
	...
	/* schedule tasklet on this channel */
	tasklet_schedule(&mxs_chan->tasklet);
}

Because mxs_chan->tasklet is not synchronized before
dmaengine_terminate_sync() returns, could mxs_dma_tasklet() run
dma_irq_callback(this) and complete this->dma_done after a subsequent
gpmi_nfc_exec_op() call has already reinitialized this->dma_done and
submitted a new transfer, causing the new operation to complete prematurely
and unmap its buffers while DMA is still active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150929.2944717-1-samarth.colleges@gmail.com?part=1