Moving RX DMA locking out of __dma_rx_do_complete() left the DMA
completion callback calling it without rx_dma_lock. Shutdown and runtime
suspend flush RX under rx_dma_lock but without the port lock, allowing
these paths to overlap the callback's buffer accounting and flip-buffer
insertion. RX submission and UART interrupt handling also require the
port lock.

Take rx_dma_lock inside the port lock in the completion callback. Hold
the port lock around the shutdown and runtime-suspend flushes, matching
the existing interrupt-path lock order. Assert both locks in the common
completion helper and the port lock at the flush entry point.

Fixes: 789898416749 ("serial: 8250: 8250_omap: Move locking out from 
__dma_rx_do_complete()")
Signed-off-by: James Hilliard <[email protected]>
---
 drivers/tty/serial/8250/8250_omap.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c 
b/drivers/tty/serial/8250/8250_omap.c
index b6f108d34854..322403cec2de 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -771,8 +771,10 @@ static void omap_8250_shutdown(struct uart_port *port)
        guard(serial8250_rpm)(up);
 
        flush_work(&priv->qos_work);
-       if (up->dma)
+       if (up->dma) {
+               guard(uart_port_lock_irq)(port);
                omap_8250_rx_dma_flush(up);
+       }
 
        serial_out(up, UART_OMAP_WER, 0);
        if (priv->habit & UART_HAS_EFR2)
@@ -898,7 +900,7 @@ static int omap8250_rs485_config(struct uart_port *port,
 #ifdef CONFIG_SERIAL_8250_DMA
 static int omap_8250_rx_dma(struct uart_8250_port *p);
 
-/* Must be called while priv->rx_dma_lock is held */
+/* Must be called with the port lock and priv->rx_dma_lock held. */
 static void __dma_rx_do_complete(struct uart_8250_port *p)
 {
        struct uart_8250_dma    *dma = p->dma;
@@ -911,6 +913,9 @@ static void __dma_rx_do_complete(struct uart_8250_port *p)
        int                     ret;
        u32                     reg;
 
+       lockdep_assert_held_once(&p->port.lock);
+       lockdep_assert_held_once(&priv->rx_dma_lock);
+
        if (!dma->rx_running)
                goto out;
 
@@ -977,7 +982,8 @@ static void __dma_rx_complete(void *param)
        if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) != 
DMA_COMPLETE)
                return;
 
-       __dma_rx_do_complete(p);
+       scoped_guard(spinlock, &priv->rx_dma_lock)
+               __dma_rx_do_complete(p);
        if (priv->throttled)
                return;
 
@@ -995,6 +1001,8 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port 
*p)
        unsigned long           flags;
        int ret;
 
+       lockdep_assert_held_once(&p->port.lock);
+
        spin_lock_irqsave(&priv->rx_dma_lock, flags);
 
        if (!dma->rx_running) {
@@ -1811,8 +1819,10 @@ static int omap8250_runtime_suspend(struct device *dev)
                }
        }
 
-       if (up && up->dma && up->dma->rxchan)
+       if (up && up->dma && up->dma->rxchan) {
+               guard(uart_port_lock_irqsave)(&up->port);
                omap_8250_rx_dma_flush(up);
+       }
 
        priv->latency = PM_QOS_CPU_LATENCY_DEFAULT_VALUE;
        schedule_work(&priv->qos_work);

-- 
2.53.0


Reply via email to