Re: [PATCH 15/24] serial: 8250: omap: acquire rx_dma_lock in DMA completion callback

[Ilpo Järvinen]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@xxxxxxxxx>
> 
> The __dma_rx_do_complete() helper requires priv->rx_dma_lock to be held,
> but the DMA completion callback __dma_rx_complete() calls it without
> that lock, only holding the UART port lock. This allows a race between
> the callback and omap_8250_rx_dma_flush() which properly holds
> rx_dma_lock, leading to concurrent tty_insert_flip_string() calls that
> can corrupt flip-buffer state.

When the commit message mentions flush and buffer corruption, I 
immediately ask why James Hilliard needs so many lines to fix the flush 
issues [1] while this change just wraps one call with a lock?? I'd tend to 
think this change lacks most of the understanding for the relevant problem 
space.

I think this should be deferred to be part of James' series.

[1] https://lore.kernel.org/linux-serial/20261024-submit-serial-8250-dma-cancel-v1-v6-5-6f366591f566@xxxxxxxxx/

> Acquire rx_dma_lock around the call,
> matching the established lock ordering (port lock then rx_dma_lock).
>
> Cc: Jiri Slaby <jirislaby@xxxxxxxxxx>
> Cc: Moteen Shah <m-shah@xxxxxx>
> Cc: Kendall Willis <k-willis@xxxxxx>
> Cc: Frank Li <Frank.Li@xxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxx>
> Signed-off-by: Joshua Rogers <linux@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/tty/serial/8250/8250_omap.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 07ebed6ac6c9..0afd648733da 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -975,7 +975,9 @@ static void __dma_rx_complete(void *param)
>  	if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) != DMA_COMPLETE)
>  		return;
>  
> +	spin_lock(&priv->rx_dma_lock);
>  	__dma_rx_do_complete(p);
> +	spin_unlock(&priv->rx_dma_lock);
>  	if (priv->throttled)
>  		return;



-- 
 i.