| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230525093159.223817-2-john.ogness@linutronix.de>
Date: Thu, 25 May 2023 11:37:52 +0206
From: John Ogness <john.ogness@...utronix.de>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Petr Mladek <pmladek@...e.com>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org, Al Cooper <alcooperx@...il.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Jiri Slaby <jirislaby@...nel.org>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Lino Sanfilippo <l.sanfilippo@...bus.com>,
Matthew Howell <matthew.howell@...level.com>,
Tony Lindgren <tony@...mide.com>,
Lukas Wunner <lukas@...ner.de>,
Matthias Schiffer <matthias.schiffer@...tq-group.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>, linux-serial@...r.kernel.org
Subject: [PATCH tty v1 1/8] 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@...utronix.de>
---
drivers/tty/serial/8250/8250_bcm7271.c | 4 ++++
drivers/tty/serial/8250/8250_exar.c | 4 ++++
drivers/tty/serial/8250/8250_omap.c | 3 +++
drivers/tty/serial/8250/8250_port.c | 18 ++++++++++++++++--
4 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index f801b1f5b46c..2b38bcc5112e 100644
--- 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 64770c62bbec..ae66ef9d863c 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -194,8 +194,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 5c093dfcee1d..fbca0692aa51 100644
--- 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 0cef9bfd0471..b3971302d8e5 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2149,7 +2149,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);
@@ -2159,12 +2164,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 */
@@ -2275,6 +2287,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);
--
2.30.2
Powered by blists - more mailing lists