Re: [PATCH v2 1/2] spi: fsl-lpspi: replace dmaengine_terminate_all with dmaengine_terminate_sync

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless initialization of `target_aborted` after `reinit_completion` overwrites concurrent aborts, causing missed DMA termination and memory corruption.
--

commit ad3a6e81718f2fdf6938f3f573651fd3ac718677
Author: Carlos Song <carlos.song@nxp.com>

spi: fsl-lpspi: replace dmaengine_terminate_all with dmaengine_terminate_sync

This commit replaces the deprecated dmaengine_terminate_all API with
dmaengine_terminate_sync to improve the fsl-lpspi driver.

> diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
> index e201309f8aae3..1a94a42fac319 100644
> --- a/drivers/spi/spi-fsl-lpspi.c
> +++ b/drivers/spi/spi-fsl-lpspi.c
[ ... ]
> @@ -688,8 +688,8 @@ static int fsl_lpspi_dma_transfer(struct spi_controller *controller,
>  			fsl_lpspi->target_aborted) {

[Severity: High]
This isn't a bug introduced by this patch, but could this target_aborted check
miss concurrent aborts due to a race condition earlier in the function?

In fsl_lpspi_dma_transfer(), the driver locklessly sets
fsl_lpspi->target_aborted = false after reinitializing the completions and
issuing pending DMA transfers. 

If the SPI core or a slave protocol driver calls spi_target_abort()
concurrently, the abort handler will set target_aborted = true and signal the
completions.

Thread execution ordering could cause the fsl_lpspi->target_aborted = false
assignment to immediately overwrite the true abort state. When the driver
subsequently waits on the completions here, it consumes the abort signal but
fails this check because the flag evaluates to false. 

Since the driver falsely assumes the transfer completed successfully,
it skips calling the dmaengine_terminate_sync() added in this block. The
SPI core would then proceed to unmap the scatterlist buffers while the DMA
is still actively transferring data, which leads to writing into unmapped
or freed memory.

>  			dev_dbg(fsl_lpspi->dev,
>  				"I/O Error in DMA TX interrupted\n");
> -			dmaengine_terminate_all(controller->dma_tx);
> -			dmaengine_terminate_all(controller->dma_rx);
> +			dmaengine_terminate_sync(controller->dma_tx);
> +			dmaengine_terminate_sync(controller->dma_rx);
>  			fsl_lpspi_reset(fsl_lpspi);
>  			return -EINTR;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520094308.2882892-1-carlos.song@oss.nxp.com?part=1