serial: core: Prevent unsafe uart port access, part 2

For serial core operations not already excluded by holding port->mutex,
use reference counting to protect deferencing the state->uart_port.

Introduce helper functions, uart_port_ref() and uart_port_deref(), to
wrap uart_port access, and helper macros, uart_port_lock() and
uart_port_unlock(), to wrap combination uart_port access with uart
port lock sections.

Port removal in uart_remove_one_port() waits for reference count to
drop to zero before detaching the uart port from struct uart_state.

For functions only reading the tx circular buffer indexes (where the
uart port lock is claimed to prevent concurrent users), a NULL uart
port is simply ignored and the operation completes normally.

For functions change the tx circular buffer indexes (where the uart
port lock is claimed to prevent concurrent users), the operation is
aborted if the uart port is NULL (ie., has been detached).
Signed-off-by: NPeter Hurley <peter@hurleysoftware.com>
Signed-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/tty/serial/serial_core.c

@@ -64,6 +64,35 @@ static int uart_dcd_enabled(struct uart_port *uport)
 	return !!(uport->status & UPSTAT_DCD_ENABLE);
 }
 
+static inline struct uart_port *uart_port_ref(struct uart_state *state)
+{
+	if (atomic_add_unless(&state->refcount, 1, 0))
+		return state->uart_port;
+	return NULL;
+}
+
+static inline void uart_port_deref(struct uart_port *uport)
+{
+	if (uport && atomic_dec_and_test(&uport->state->refcount))
+		wake_up(&uport->state->remove_wait);
+}
+
+#define uart_port_lock(state, flags)					\
+	({								\
+		struct uart_port *__uport = uart_port_ref(state);	\
+		if (__uport)						\
+			spin_lock_irqsave(&__uport->lock, flags);	\
+		__uport;						\
+	})
+
+#define uart_port_unlock(uport, flags)					\
+	({								\
+		struct uart_port *__uport = uport;			\
+		if (__uport)						\
+			spin_unlock_irqrestore(&__uport->lock, flags);	\
+		uart_port_deref(__uport);				\
+	})
+
 static inline struct uart_port *uart_port_check(struct uart_state *state)
 {
 #ifdef CONFIG_LOCKDEP
@@ -90,12 +119,13 @@ void uart_write_wakeup(struct uart_port *port)
 static void uart_stop(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
-	port->ops->stop_tx(port);
-	spin_unlock_irqrestore(&port->lock, flags);
+	port = uart_port_lock(state, flags);
+	if (port)
+		port->ops->stop_tx(port);
+	uart_port_unlock(port, flags);
 }
 
 static void __uart_start(struct tty_struct *tty)
@@ -103,19 +133,19 @@ static void __uart_start(struct tty_struct *tty)
 	struct uart_state *state = tty->driver_data;
 	struct uart_port *port = state->uart_port;
 
-	if (!uart_tx_stopped(port))
+	if (port && !uart_tx_stopped(port))
 		port->ops->start_tx(port);
 }
 
 static void uart_start(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
+	port = uart_port_lock(state, flags);
 	__uart_start(tty);
-	spin_unlock_irqrestore(&port->lock, flags);
+	uart_port_unlock(port, flags);
 }
 
 static void
@@ -496,7 +526,7 @@ static void uart_change_speed(struct tty_struct *tty, struct uart_state *state,
 static int uart_put_char(struct tty_struct *tty, unsigned char c)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	struct circ_buf *circ;
 	unsigned long flags;
 	int ret = 0;
@@ -505,13 +535,13 @@ static int uart_put_char(struct tty_struct *tty, unsigned char c)
 	if (!circ->buf)
 		return 0;
 
-	spin_lock_irqsave(&port->lock, flags);
-	if (uart_circ_chars_free(circ) != 0) {
+	port = uart_port_lock(state, flags);
+	if (port && uart_circ_chars_free(circ) != 0) {
 		circ->buf[circ->head] = c;
 		circ->head = (circ->head + 1) & (UART_XMIT_SIZE - 1);
 		ret = 1;
 	}
-	spin_unlock_irqrestore(&port->lock, flags);
+	uart_port_unlock(port, flags);
 	return ret;
 }
 
@@ -538,14 +568,12 @@ static int uart_write(struct tty_struct *tty,
 		return -EL3HLT;
 	}
 
-	port = state->uart_port;
 	circ = &state->xmit;
-
 	if (!circ->buf)
 		return 0;
 
-	spin_lock_irqsave(&port->lock, flags);
-	while (1) {
+	port = uart_port_lock(state, flags);
+	while (port) {
 		c = CIRC_SPACE_TO_END(circ->head, circ->tail, UART_XMIT_SIZE);
 		if (count < c)
 			c = count;
@@ -559,32 +587,33 @@ static int uart_write(struct tty_struct *tty,
 	}
 
 	__uart_start(tty);
-	spin_unlock_irqrestore(&port->lock, flags);
-
+	uart_port_unlock(port, flags);
 	return ret;
 }
 
 static int uart_write_room(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
+	struct uart_port *port;
 	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&state->uart_port->lock, flags);
+	port = uart_port_lock(state, flags);
 	ret = uart_circ_chars_free(&state->xmit);
-	spin_unlock_irqrestore(&state->uart_port->lock, flags);
+	uart_port_unlock(port, flags);
 	return ret;
 }
 
 static int uart_chars_in_buffer(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
+	struct uart_port *port;
 	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&state->uart_port->lock, flags);
+	port = uart_port_lock(state, flags);
 	ret = uart_circ_chars_pending(&state->xmit);
-	spin_unlock_irqrestore(&state->uart_port->lock, flags);
+	uart_port_unlock(port, flags);
 	return ret;
 }
 
@@ -603,14 +632,15 @@ static void uart_flush_buffer(struct tty_struct *tty)
 		return;
 	}
 
-	port = state->uart_port;
 	pr_debug("uart_flush_buffer(%d) called\n", tty->index);
 
-	spin_lock_irqsave(&port->lock, flags);
+	port = uart_port_lock(state, flags);
+	if (!port)
+		return;
 	uart_circ_clear(&state->xmit);
 	if (port->ops->flush_buffer)
 		port->ops->flush_buffer(port);
-	spin_unlock_irqrestore(&port->lock, flags);
+	uart_port_unlock(port, flags);
 	tty_wakeup(tty);
 }
 
@@ -621,9 +651,13 @@ static void uart_flush_buffer(struct tty_struct *tty)
 static void uart_send_xchar(struct tty_struct *tty, char ch)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	unsigned long flags;
 
+	port = uart_port_ref(state);
+	if (!port)
+		return;
+
 	if (port->ops->send_xchar)
 		port->ops->send_xchar(port, ch);
 	else {
@@ -633,14 +667,19 @@ static void uart_send_xchar(struct tty_struct *tty, char ch)
 			port->ops->start_tx(port);
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
+	uart_port_deref(port);
 }
 
 static void uart_throttle(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	upstat_t mask = 0;
 
+	port = uart_port_ref(state);
+	if (!port)
+		return;
+
 	if (I_IXOFF(tty))
 		mask |= UPSTAT_AUTOXOFF;
 	if (C_CRTSCTS(tty))
@@ -656,14 +695,20 @@ static void uart_throttle(struct tty_struct *tty)
 
 	if (mask & UPSTAT_AUTOXOFF)
 		uart_send_xchar(tty, STOP_CHAR(tty));
+
+	uart_port_deref(port);
 }
 
 static void uart_unthrottle(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
-	struct uart_port *port = state->uart_port;
+	struct uart_port *port;
 	upstat_t mask = 0;
 
+	port = uart_port_ref(state);
+	if (!port)
+		return;
+
 	if (I_IXOFF(tty))
 		mask |= UPSTAT_AUTOXOFF;
 	if (C_CRTSCTS(tty))
@@ -679,6 +724,8 @@ static void uart_unthrottle(struct tty_struct *tty)
 
 	if (mask & UPSTAT_AUTOXOFF)
 		uart_send_xchar(tty, START_CHAR(tty));
+
+	uart_port_deref(port);
 }
 
 static int uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
@@ -1116,10 +1163,9 @@ static void uart_enable_ms(struct uart_port *uport)
  * FIXME: This wants extracting into a common all driver implementation
  * of TIOCMWAIT using tty_port.
  */
-static int
-uart_wait_modem_status(struct uart_state *state, unsigned long arg)
+static int uart_wait_modem_status(struct uart_state *state, unsigned long arg)
 {
-	struct uart_port *uport = state->uart_port;
+	struct uart_port *uport;
 	struct tty_port *port = &state->port;
 	DECLARE_WAITQUEUE(wait, current);
 	struct uart_icount cprev, cnow;
@@ -1128,6 +1174,9 @@ uart_wait_modem_status(struct uart_state *state, unsigned long arg)
 	/*
 	 * note the counters on entry
 	 */
+	uport = uart_port_ref(state);
+	if (!uport)
+		return -EIO;
 	spin_lock_irq(&uport->lock);
 	memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
 	uart_enable_ms(uport);
@@ -1161,6 +1210,7 @@ uart_wait_modem_status(struct uart_state *state, unsigned long arg)
 	}
 	__set_current_state(TASK_RUNNING);
 	remove_wait_queue(&port->delta_msr_wait, &wait);
+	uart_port_deref(uport);
 
 	return ret;
 }
@@ -1176,11 +1226,15 @@ static int uart_get_icount(struct tty_struct *tty,
 {
 	struct uart_state *state = tty->driver_data;
 	struct uart_icount cnow;
-	struct uart_port *uport = state->uart_port;
+	struct uart_port *uport;
 
+	uport = uart_port_ref(state);
+	if (!uport)
+		return -EIO;
 	spin_lock_irq(&uport->lock);
 	memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
 	spin_unlock_irq(&uport->lock);
+	uart_port_deref(uport);
 
 	icount->cts         = cnow.cts;
 	icount->dsr         = cnow.dsr;
@@ -1481,11 +1535,14 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
 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 *port;
 	unsigned long char_time, expire;
 
-	if (port->type == PORT_UNKNOWN || port->fifosize == 0)
+	port = uart_port_ref(state);
+	if (!port || port->type == PORT_UNKNOWN || port->fifosize == 0) {
+		uart_port_deref(port);
 		return;
+	}
 
 	/*
 	 * Set the check interval to be 1/5 of the estimated time to
@@ -1531,6 +1588,7 @@ static void uart_wait_until_sent(struct tty_struct *tty, int timeout)
 		if (time_after(jiffies, expire))
 			break;
 	}
+	uart_port_deref(port);
 }
 
 /*
@@ -1591,12 +1649,23 @@ static void uart_port_shutdown(struct tty_port *port)
 static int uart_carrier_raised(struct tty_port *port)
 {
 	struct uart_state *state = container_of(port, struct uart_state, port);
-	struct uart_port *uport = state->uart_port;
+	struct uart_port *uport;
 	int mctrl;
+
+	uport = uart_port_ref(state);
+	/*
+	 * Should never observe uport == NULL since checks for hangup should
+	 * abort the tty_port_block_til_ready() loop before checking for carrier
+	 * raised -- but report carrier raised if it does anyway so open will
+	 * continue and not sleep
+	 */
+	if (WARN_ON(!uport))
+		return 1;
 	spin_lock_irq(&uport->lock);
 	uart_enable_ms(uport);
 	mctrl = uport->ops->get_mctrl(uport);
 	spin_unlock_irq(&uport->lock);
+	uart_port_deref(uport);
 	if (mctrl & TIOCM_CAR)
 		return 1;
 	return 0;
@@ -1605,12 +1674,18 @@ static int uart_carrier_raised(struct tty_port *port)
 static void uart_dtr_rts(struct tty_port *port, int onoff)
 {
 	struct uart_state *state = container_of(port, struct uart_state, port);
-	struct uart_port *uport = state->uart_port;
+	struct uart_port *uport;
+
+	uport = uart_port_ref(state);
+	if (!uport)
+		return;
 
 	if (onoff)
 		uart_set_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
 	else
 		uart_clear_mctrl(uport, TIOCM_DTR | TIOCM_RTS);
+
+	uart_port_deref(uport);
 }
 
 /*
@@ -2320,12 +2395,15 @@ static int uart_poll_get_char(struct tty_driver *driver, int line)
 	struct uart_driver *drv = driver->driver_state;
 	struct uart_state *state = drv->state + line;
 	struct uart_port *port;
+	int ret = -1;
 
-	if (!state || !state->uart_port)
-		return -1;
-
-	port = state->uart_port;
-	return port->ops->poll_get_char(port);
+	if (state) {
+		port = uart_port_ref(state);
+		if (port)
+			ret = port->ops->poll_get_char(port);
+		uart_port_deref(port);
+	}
+	return ret;
 }
 
 static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
@@ -2334,14 +2412,17 @@ static void uart_poll_put_char(struct tty_driver *driver, int line, char ch)
 	struct uart_state *state = drv->state + line;
 	struct uart_port *port;
 
-	if (!state || !state->uart_port)
+	if (!state)
 		return;
 
-	port = state->uart_port;
+	port = uart_port_ref(state);
+	if (!port)
+		return;
 
 	if (ch == '\n')
 		port->ops->poll_put_char(port, '\r');
 	port->ops->poll_put_char(port, ch);
+	uart_port_deref(port);
 }
 #endif
 
@@ -2688,6 +2769,8 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
 	}
 
 	/* Link the port to the driver state table and vice versa */
+	atomic_set(&state->refcount, 1);
+	init_waitqueue_head(&state->remove_wait);
 	state->uart_port = uport;
 	uport->state = state;
 
@@ -2816,6 +2899,8 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 	uport->type = PORT_UNKNOWN;
 
 	mutex_lock(&port->mutex);
+	WARN_ON(atomic_dec_return(&state->refcount) < 0);
+	wait_event(state->remove_wait, !atomic_read(&state->refcount));
 	state->uart_port = NULL;
 	mutex_unlock(&port->mutex);
 out:

include/linux/serial_core.h

@@ -281,6 +281,8 @@ struct uart_state {
 	enum uart_pm_state	pm_state;
 	struct circ_buf		xmit;
 
+	atomic_t		refcount;
+	wait_queue_head_t	remove_wait;
 	struct uart_port	*uart_port;
 };