[PATCH 7/8] serial: core: Prevent unsafe uart port access, part 2

[Peter Hurley <peter@hurleysoftware.com>]

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@hurleysoftware.com>
---
 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