Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751750AbbLMAhD (ORCPT ); Sat, 12 Dec 2015 19:37:03 -0500 Received: from mail-pf0-f177.google.com ([209.85.192.177]:33385 "EHLO mail-pf0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751499AbbLMAgx (ORCPT ); Sat, 12 Dec 2015 19:36:53 -0500 From: Peter Hurley To: Greg Kroah-Hartman Cc: Jiri Slaby , linux-kernel@vger.kernel.org, Peter Hurley Subject: [PATCH 7/8] serial: core: Prevent unsafe uart port access, part 2 Date: Sat, 12 Dec 2015 16:36:31 -0800 Message-Id: <1449966992-4033-8-git-send-email-peter@hurleysoftware.com> X-Mailer: git-send-email 2.6.3 In-Reply-To: <1449966992-4033-1-git-send-email-peter@hurleysoftware.com> References: <1449966992-4033-1-git-send-email-peter@hurleysoftware.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8288 Lines: 234 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 --- 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/