Re: [PATCH] tty: serial: samsung: use dmaengine_terminate_sync() in release_dma

[Marek Szyprowski <m.szyprowski@samsung.com>]

On 30.10.2026 17:23, Samarth Narang wrote:
> s3c24xx_serial_release_dma() stops the RX and TX DMA channels using the
> deprecated dmaengine_terminate_all() before unmapping their buffers and
> freeing the RX buffer. Since this does not guarantee that DMA transfers
> or their completion callbacks have finished, the controller may continue
> accessing memory after it has been unmapped or returned to the
> allocator, particularly when the port is shut down while RX DMA is
> active.
>
> Replace dmaengine_terminate_all() with dmaengine_terminate_sync() to
> ensure that outstanding DMA activity is terminated before the buffers
> are released. Both callers of s3c24xx_serial_release_dma() run in
> process context, where synchronous termination is permitted.
>
> Fixes: 62c37eedb74c ("serial: samsung: add dma reqest/release functions")
> Assisted-by: LLM
> Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>

Reviewed-by: Marek Szyprowski <m.szyprowski@samsung.com>


> ---
> This issue was identified by a research static analyzer for DMA
> lifecycle ordering and has not been validated on hardware. The patch has
> only been compile-tested on arm64 with W=1.
>
> On Exynos systems, the UART uses PL330, which does not implement a
> device_synchronize callback, so this change follows the DMAengine API
> contract without independently establishing that all hardware activity
> has stopped. The PL330 termination behavior is being raised separately
> with the DMAengine maintainers.
>
> Two related questions outside the scope of this patch:
>
> - s3c24xx_serial_rx_dma_complete() unconditionally re-arms RX DMA,
>   potentially racing with release_dma() and submitting another
>   descriptor after termination. Should shutdown prevent further
>   submissions before terminating the channel?

Yes, this need to be fixed.

> - s3c24xx_serial_stop_rx() calls dmaengine_pause(dma->tx_chan) before
>   checking the RX channel's residue. Should this instead use
>   dma->rx_chan?

Right, definitely that's a bug in current code.


>
>  drivers/tty/serial/samsung_tty.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 640f85e2d..4eeef149b 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -1145,7 +1145,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
>  	struct s3c24xx_uart_dma	*dma = p->dma;
>  
>  	if (dma->rx_chan) {
> -		dmaengine_terminate_all(dma->rx_chan);
> +		dmaengine_terminate_sync(dma->rx_chan);
>  		dma_unmap_single(dmaengine_get_dma_device(dma->rx_chan), dma->rx_addr,
>  				 dma->rx_size, DMA_FROM_DEVICE);
>  		kfree(dma->rx_buf);
> @@ -1154,7 +1154,7 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
>  	}
>  
>  	if (dma->tx_chan) {
> -		dmaengine_terminate_all(dma->tx_chan);
> +		dmaengine_terminate_sync(dma->tx_chan);
>  		dma_unmap_single(dmaengine_get_dma_device(dma->tx_chan), dma->tx_addr,
>  				 UART_XMIT_SIZE, DMA_TO_DEVICE);
>  		dma_release_channel(dma->tx_chan);
>
> base-commit: 83f32c773255720a661aa45d83e36278135f23f5
>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland