Re: [PATCH 2/2] tty: serial: samsung: do not re-arm RX DMA once RX has been stopped

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

On 31.10.2026 01:42, Samarth Narang wrote:
> s3c24xx_serial_rx_dma_complete() unconditionally submits a new RX
> descriptor after each completion, even when reception has been stopped
> or the port is shutting down. A completion racing with
> s3c24xx_serial_stop_rx() or shutdown may therefore restart DMA after the
> channel has been terminated, potentially allowing it to access the RX
> buffer after s3c24xx_serial_release_dma() has unmapped and freed it.
>
> Only re-arm RX DMA while rx_enabled is set, and clear the flag under the
> port lock during shutdown, since the hangup path can reach
> s3c64xx_serial_shutdown() without calling stop_rx(). Because the
> completion callback also holds the port lock, any descriptor submitted
> before rx_enabled is cleared will be covered by the subsequent channel
> termination.
>
> Fixes: b543c301c2c5 ("serial: samsung: add DMA support for RX")
> Link: https://lore.kernel.org/r/20261030162334.3000737-1-samarth.colleges@gmail.com
> Assisted-by: LLM
> Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>


This one requires a bit more work. rx_enabled might be temporarily set to
 0 in s3c24xx_serial_rx_drain_fifo(), when uart_cons_flow_enabled()
returns true, what would result in stopping DMA at all after this patch.

The driver shutdown procedure requires much more checks and fixes, than
such single flag check.


> ---
>  drivers/tty/serial/samsung_tty.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index 2f6c69bec..3f2346f4f 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -622,7 +622,8 @@ static void s3c24xx_serial_rx_dma_complete(void *args)
>  		tty_kref_put(tty);
>  	}
>  
> -	s3c64xx_start_rx_dma(ourport);
> +	if (ourport->rx_enabled)
> +		s3c64xx_start_rx_dma(ourport);
>  
>  	uart_port_unlock_irqrestore(port, flags);
>  }
> @@ -1165,10 +1166,13 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p)
>  static void s3c64xx_serial_shutdown(struct uart_port *port)
>  {
>  	struct s3c24xx_uart_port *ourport = to_ourport(port);
> +	unsigned long flags;
>  
> +	uart_port_lock_irqsave(port, &flags);
>  	ourport->tx_enabled = 0;
>  	ourport->tx_mode = 0;
>  	ourport->rx_enabled = 0;
> +	uart_port_unlock_irqrestore(port, flags);
>  
>  	free_irq(port->irq, ourport);
>  

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland