serial: 8250: lock port in startup() callbacks

uart_ops startup() callback is called without interrupts
disabled and without port->lock locked, relatively late during the
boot process (from the call path of console_on_rootfs()). If the
device is a console, it was already previously registered and could
be actively printing messages.

The console printing function serial8250_console_write() modifies
the interrupt register (UART_IER) under the port->lock with the
pattern: read, clear, restore.

Since some startup() callbacks are modifying UART_IER without the
port->lock locked, it is possible that the value intended to be
written by the startup() callback will get overwritten and be
lost.

CPU0                           CPU1
serial8250_console_write       omap_8250_startup
--------------------------     -----------------
spin_lock(port->lock)
oldval = read(UART_IER)
uart_console_write()
                               write(newval, UART_IER)
write(oldval, UART_IER)
spin_unlock(port->lock)

Add port->lock synchronization to the 8250 startup() callbacks
where they need to access UART_IER. This avoids racing with
serial8250_console_write().

Signed-off-by: John Ogness <john.ogness@linutronix.de>
Link: https://lore.kernel.org/r/20230525093159.223817-2-john.ogness@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index af0e1c07018795d406bab65eea73181f79920c57..d4b05d7ad9e8de8f9ee3ffe2c3a96ff3db5bd542 100644 (file)
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -605,9 +605,13 @@ static int brcmuart_startup(struct uart_port *port)
        /*
         * Disable the Receive Data Interrupt because the DMA engine
         * will handle this.
+        *
+        * Synchronize UART_IER access against the console.
         */
+       spin_lock_irq(&port->lock);
        up->ier &= ~UART_IER_RDI;
        serial_port_out(port, UART_IER, up->ier);
+       spin_unlock_irq(&port->lock);
 
        priv->tx_running = false;
        priv->dma.rx_dma = NULL;

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b406cba10b0eb4b6bb97aacb18d938000efb1903..077c3ba3539e6887dfe67d0ed95a864cc341bdc0 100644 (file)
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -198,8 +198,12 @@ static int xr17v35x_startup(struct uart_port *port)
        /*
         * Make sure all interrups are masked until initialization is
         * complete and the FIFOs are cleared
+        *
+        * Synchronize UART_IER access against the console.
         */
+       spin_lock_irq(&port->lock);
        serial_port_out(port, UART_IER, 0);
+       spin_unlock_irq(&port->lock);
 
        return serial8250_do_startup(port);
 }

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 5c093dfcee1d53bb7fa4aea6c20851d8cd8744e2..fbca0692aa51b11a349e475b0bbe3a3e64f07b5e 100644 (file)
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -710,8 +710,11 @@ static int omap_8250_startup(struct uart_port *port)
                up->dma = NULL;
        }
 
+       /* Synchronize UART_IER access against the console. */
+       spin_lock_irq(&port->lock);
        up->ier = UART_IER_RLSI | UART_IER_RDI;
        serial_out(up, UART_IER, up->ier);
+       spin_unlock_irq(&port->lock);
 
 #ifdef CONFIG_PM
        up->capabilities |= UART_CAP_RPM;

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2d8f87f4e40f5aa4efd4fe4cfc70104aaf9b25a4..ecfcdc02eeaf2d2c2470f03280327dd62bfca3cb 100644 (file)
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2150,7 +2150,12 @@ int serial8250_do_startup(struct uart_port *port)
 
        serial8250_rpm_get(up);
        if (port->type == PORT_16C950) {
-               /* Wake up and initialize UART */
+               /*
+                * Wake up and initialize UART
+                *
+                * Synchronize UART_IER access against the console.
+                */
+               spin_lock_irqsave(&port->lock, flags);
                up->acr = 0;
                serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B);
                serial_port_out(port, UART_EFR, UART_EFR_ECB);
@@ -2160,12 +2165,19 @@ int serial8250_do_startup(struct uart_port *port)
                serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B);
                serial_port_out(port, UART_EFR, UART_EFR_ECB);
                serial_port_out(port, UART_LCR, 0);
+               spin_unlock_irqrestore(&port->lock, flags);
        }
 
        if (port->type == PORT_DA830) {
-               /* Reset the port */
+               /*
+                * Reset the port
+                *
+                * Synchronize UART_IER access against the console.
+                */
+               spin_lock_irqsave(&port->lock, flags);
                serial_port_out(port, UART_IER, 0);
                serial_port_out(port, UART_DA830_PWREMU_MGMT, 0);
+               spin_unlock_irqrestore(&port->lock, flags);
                mdelay(10);
 
                /* Enable Tx, Rx and free run mode */
@@ -2276,6 +2288,8 @@ int serial8250_do_startup(struct uart_port *port)
                 * this interrupt whenever the transmitter is idle and
                 * the interrupt is enabled.  Delays are necessary to
                 * allow register changes to become visible.
+                *
+                * Synchronize UART_IER access against the console.
                 */
                spin_lock_irqsave(&port->lock, flags);