Commit b1207d86 authored by John Ogness's avatar John Ogness Committed by Greg Kroah-Hartman
Browse files

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: default avatarJohn Ogness <john.ogness@linutronix.de>
Link: https://lore.kernel.org/r/20230525093159.223817-2-john.ogness@linutronix.de


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 0e4daea3

drivers/tty/serial/8250/8250_bcm7271.c

+4 −0
Original line number Diff line number Diff line
@@ -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;

drivers/tty/serial/8250/8250_exar.c

+4 −0
Original line number Diff line number Diff line
@@ -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);

}

drivers/tty/serial/8250/8250_omap.c

+3 −0
Original line number Diff line number Diff line
@@ -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;

drivers/tty/serial/8250/8250_port.c

+16 −2
Original line number Diff line number Diff line
@@ -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);