[PATCH 7/8] serial: core: Prevent unsafe uart port access, part 2
From: Peter Hurley
Date: Sat Dec 12 2015 - 19:37:10 EST
For tty operations which may expect uart port to have been removed
but still have other necessary work to accomplish, check for NULL
uart port; specifically uart_close(), uart_hangup() and sub-functions
(uart_shutdown(), uart_port_shutdown() and uart_wait_until_sent()).
Split uart_wait_until_sent() into the original tty operation (which
now must claim the port->mutex) and __uart_wait_until_sent() (which
performs the timeout computations and waits); the port->mutex is
released during the sleep and the uart port ptr is reloaded after
wakeup.
Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
drivers/tty/serial/serial_core.c | 66 ++++++++++++++++++++++++++++------------
1 file changed, 46 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 2806f52..47a3dc9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -53,7 +53,7 @@ static struct lock_class_key port_lock_key;
static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
struct ktermios *old_termios);
-static void uart_wait_until_sent(struct tty_struct *tty, int timeout);
+static void __uart_wait_until_sent(struct tty_struct *tty, int timeout);
static void uart_change_pm(struct uart_state *state,
enum uart_pm_state pm_state);
@@ -221,6 +221,8 @@ static int uart_startup(struct tty_struct *tty, struct uart_state *state,
* This routine will shutdown a serial port; interrupts are disabled, and
* DTR is dropped if the hangup on close termio flag is on. Calls to
* uart_shutdown are serialised by the per-port semaphore.
+ *
+ * uport == NULL if uart_port has already been removed
*/
static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
{
@@ -237,7 +239,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
/*
* Turn off DTR and RTS early.
*/
- if (uart_console(uport) && tty)
+ if (uport && uart_console(uport) && tty)
uport->cons->cflag = tty->termios.c_cflag;
if (!tty || (tty->termios.c_cflag & HUPCL))
@@ -1406,7 +1408,6 @@ out:
* Calls to uart_close() are serialised via the tty_lock in
* drivers/tty/tty_io.c:tty_release()
* drivers/tty/tty_io.c:do_tty_hangup()
- * This runs from a workqueue and can sleep for a _short_ time only.
*/
static void uart_close(struct tty_struct *tty, struct file *filp)
{
@@ -1425,18 +1426,21 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
return;
}
- uport = state->uart_port;
port = &state->port;
pr_debug("uart_close(%d) called\n", tty->index);
- if (!port->count || tty_port_close_start(port, tty, filp) == 0)
+ if (tty_port_close_start(port, tty, filp) == 0)
return;
+ mutex_lock(&port->mutex);
+ uport = state->uart_port;
+
/*
* At this point, we stop accepting input. To do this, we
* disable the receive line status interrupts.
*/
- if (port->flags & ASYNC_INITIALIZED) {
+ if ((port->flags & ASYNC_INITIALIZED) &&
+ !WARN(!uport, "detached port still initialized!\n")) {
spin_lock_irq(&uport->lock);
uport->ops->stop_rx(uport);
spin_unlock_irq(&uport->lock);
@@ -1445,10 +1449,10 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
* has completely drained; this is especially
* important if there is a transmit FIFO!
*/
- uart_wait_until_sent(tty, uport->timeout);
+ __uart_wait_until_sent(tty, uport->timeout);
+ uport = state->uart_port;
}
- mutex_lock(&port->mutex);
uart_shutdown(tty, state);
tty_port_tty_set(port, NULL);
@@ -1459,7 +1463,7 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
if (port->close_delay)
msleep_interruptible(jiffies_to_msecs(port->close_delay));
spin_lock_irq(&port->lock);
- } else if (!uart_console(uport)) {
+ } else if (uport && !uart_console(uport)) {
spin_unlock_irq(&port->lock);
uart_change_pm(state, UART_PM_STATE_OFF);
spin_lock_irq(&port->lock);
@@ -1475,13 +1479,13 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
mutex_unlock(&port->mutex);
}
-static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
+static void __uart_wait_until_sent(struct tty_struct *tty, int timeout)
{
struct uart_state *state = tty->driver_data;
- struct uart_port *port = state->uart_port;
+ struct uart_port *uport = state->uart_port;
unsigned long char_time, expire;
- if (port->type == PORT_UNKNOWN || port->fifosize == 0)
+ if (uport->type == PORT_UNKNOWN || uport->fifosize == 0)
return;
/*
@@ -1492,7 +1496,7 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
* Note: we have to use pretty tight timings here to satisfy
* the NIST-PCTS.
*/
- char_time = (port->timeout - HZ/50) / port->fifosize;
+ char_time = (uport->timeout - HZ/50) / uport->fifosize;
char_time = char_time / 5;
if (char_time == 0)
char_time = 1;
@@ -1508,21 +1512,26 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
* UART bug of some kind. So, we clamp the timeout parameter at
* 2*port->timeout.
*/
- if (timeout == 0 || timeout > 2 * port->timeout)
- timeout = 2 * port->timeout;
+ if (timeout == 0 || timeout > 2 * uport->timeout)
+ timeout = 2 * uport->timeout;
expire = jiffies + timeout;
pr_debug("uart_wait_until_sent(%d), jiffies=%lu, expire=%lu...\n",
- port->line, jiffies, expire);
+ uport->line, jiffies, expire);
/*
* Check whether the transmitter is empty every 'char_time'.
* 'timeout' / 'expire' give us the maximum amount of time
* we wait.
*/
- while (!port->ops->tx_empty(port)) {
+ while (!uport->ops->tx_empty(uport)) {
+ mutex_unlock(&state->port.mutex);
msleep_interruptible(jiffies_to_msecs(char_time));
+ mutex_lock(&state->port.mutex);
+ uport = state->uart_port;
+ if (!uport)
+ break;
if (signal_pending(current))
break;
if (time_after(jiffies, expire))
@@ -1530,6 +1539,16 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
}
}
+static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
+{
+ struct uart_state *state = tty->driver_data;
+
+ mutex_lock(&state->port.mutex);
+ if (state->uart_port)
+ __uart_wait_until_sent(tty, timeout);
+ mutex_unlock(&state->port.mutex);
+}
+
/*
* Calls to uart_hangup() are serialised by the tty_lock in
* drivers/tty/tty_io.c:do_tty_hangup()
@@ -1539,11 +1558,15 @@ static void uart_hangup(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
+ struct uart_port *uport;
unsigned long flags;
pr_debug("uart_hangup(%d)\n", tty->index);
mutex_lock(&port->mutex);
+ uport = state->uart_port;
+ WARN(!uport, "hangup of detached port!\n");
+
if (port->flags & ASYNC_NORMAL_ACTIVE) {
uart_flush_buffer(tty);
uart_shutdown(tty, state);
@@ -1552,7 +1575,7 @@ static void uart_hangup(struct tty_struct *tty)
clear_bit(ASYNCB_NORMAL_ACTIVE, &port->flags);
spin_unlock_irqrestore(&port->lock, flags);
tty_port_tty_set(port, NULL);
- if (!uart_console(state->uart_port))
+ if (uport && !uart_console(uport))
uart_change_pm(state, UART_PM_STATE_OFF);
wake_up_interruptible(&port->open_wait);
wake_up_interruptible(&port->delta_msr_wait);
@@ -1560,6 +1583,7 @@ static void uart_hangup(struct tty_struct *tty)
mutex_unlock(&port->mutex);
}
+/* uport == NULL if uart_port has already been removed */
static void uart_port_shutdown(struct tty_port *port)
{
struct uart_state *state = container_of(port, struct uart_state, port);
@@ -1577,12 +1601,14 @@ static void uart_port_shutdown(struct tty_port *port)
/*
* Free the IRQ and disable the port.
*/
- uport->ops->shutdown(uport);
+ if (uport)
+ uport->ops->shutdown(uport);
/*
* Ensure that the IRQ handler isn't running on another CPU.
*/
- synchronize_irq(uport->irq);
+ if (uport)
+ synchronize_irq(uport->irq);
}
static int uart_carrier_raised(struct tty_port *port)
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/