From: Peter Hurley <peter@hurleysoftware.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>,
linux-kernel@vger.kernel.org,
Peter Hurley <peter@hurleysoftware.com>
Subject: [PATCH 6/8] serial: core: Prevent unsafe uart port access, part 1
Date: Sat, 12 Dec 2015 16:36:30 -0800 [thread overview]
Message-ID: <1449966992-4033-7-git-send-email-peter@hurleysoftware.com> (raw)
In-Reply-To: <1449966992-4033-1-git-send-email-peter@hurleysoftware.com>
uart_remove_one_port() may race with every serial core operation
requiring a valid dereference of state->uart_port. In particular,
uart_remove_one_port() may unlink the uart port concurrently with
any serial core operation that may dereference same.
Ensure safe dereference for those operations that already claim
the port->mutex, and extend that guarantee for trivial cases,
such as the ioctl handlers.
For ioctls, return -EIO as if the port has been hung up (since it
has).
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/tty/serial/serial_core.c | 115 ++++++++++++++++++++++++++-------------
1 file changed, 78 insertions(+), 37 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 16c4c48..2806f52 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -681,10 +681,11 @@ static void uart_unthrottle(struct tty_struct *tty)
uart_set_mctrl(port, TIOCM_RTS);
}
-static void uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
+static int uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
{
struct uart_state *state = container_of(port, struct uart_state, port);
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
+ int ret = -ENODEV;
memset(retinfo, 0, sizeof(*retinfo));
@@ -693,6 +694,10 @@ static void uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
* occur as we go
*/
mutex_lock(&port->mutex);
+ uport = state->uart_port;
+ if (!uport)
+ goto out;
+
retinfo->type = uport->type;
retinfo->line = uport->line;
retinfo->port = uport->iobase;
@@ -711,7 +716,11 @@ static void uart_get_info(struct tty_port *port, struct serial_struct *retinfo)
retinfo->io_type = uport->iotype;
retinfo->iomem_reg_shift = uport->regshift;
retinfo->iomem_base = (void *)(unsigned long)uport->mapbase;
+
+ ret = 0;
+out:
mutex_unlock(&port->mutex);
+ return ret;
}
static int uart_get_info_user(struct tty_port *port,
@@ -719,7 +728,8 @@ static int uart_get_info_user(struct tty_port *port,
{
struct serial_struct tmp;
- uart_get_info(port, &tmp);
+ if (uart_get_info(port, &tmp) < 0)
+ return -EIO;
if (copy_to_user(retinfo, &tmp, sizeof(*retinfo)))
return -EFAULT;
@@ -737,6 +747,9 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
upf_t old_flags, new_flags;
int retval = 0;
+ if (!uport)
+ return -EIO;
+
new_port = new_info->port;
if (HIGH_BITS_OFFSET)
new_port += (unsigned long) new_info->port_high << HIGH_BITS_OFFSET;
@@ -946,8 +959,6 @@ static int uart_set_info_user(struct tty_struct *tty, struct uart_state *state,
* @tty: tty associated with the UART
* @state: UART being queried
* @value: returned modem value
- *
- * Note: uart_ioctl protects us against hangups.
*/
static int uart_get_lsr_info(struct tty_struct *tty,
struct uart_state *state, unsigned int __user *value)
@@ -975,18 +986,22 @@ static int uart_tiocmget(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
int result = -EIO;
mutex_lock(&port->mutex);
+ uport = state->uart_port;
+ if (!uport)
+ goto out;
+
if (!(tty->flags & (1 << TTY_IO_ERROR))) {
result = uport->mctrl;
spin_lock_irq(&uport->lock);
result |= uport->ops->get_mctrl(uport);
spin_unlock_irq(&uport->lock);
}
+out:
mutex_unlock(&port->mutex);
-
return result;
}
@@ -994,15 +1009,20 @@ static int
uart_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear)
{
struct uart_state *state = tty->driver_data;
- struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
+ struct uart_port *uport;
int ret = -EIO;
mutex_lock(&port->mutex);
+ uport = state->uart_port;
+ if (!uport)
+ goto out;
+
if (!(tty->flags & (1 << TTY_IO_ERROR))) {
uart_update_mctrl(uport, set, clear);
ret = 0;
}
+out:
mutex_unlock(&port->mutex);
return ret;
}
@@ -1011,21 +1031,26 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
{
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
+ int ret = -EIO;
mutex_lock(&port->mutex);
+ uport = state->uart_port;
+ if (!uport)
+ goto out;
if (uport->type != PORT_UNKNOWN)
uport->ops->break_ctl(uport, break_state);
-
+ ret = 0;
+out:
mutex_unlock(&port->mutex);
- return 0;
+ return ret;
}
static int uart_do_autoconfig(struct tty_struct *tty,struct uart_state *state)
{
- struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
+ struct uart_port *uport;
int flags, ret;
if (!capable(CAP_SYS_ADMIN))
@@ -1039,6 +1064,12 @@ static int uart_do_autoconfig(struct tty_struct *tty,struct uart_state *state)
if (mutex_lock_interruptible(&port->mutex))
return -ERESTARTSYS;
+ uport = state->uart_port;
+ if (!uport) {
+ ret = -EIO;
+ goto out;
+ }
+
ret = -EBUSY;
if (tty_port_users(port) == 1) {
uart_shutdown(tty, state);
@@ -1062,6 +1093,7 @@ static int uart_do_autoconfig(struct tty_struct *tty,struct uart_state *state)
ret = uart_startup(tty, state, 1);
}
+out:
mutex_unlock(&port->mutex);
return ret;
}
@@ -1210,11 +1242,11 @@ static int uart_set_rs485_config(struct uart_port *port,
* Called via sys_ioctl. We can use spin_lock_irq() here.
*/
static int
-uart_ioctl(struct tty_struct *tty, unsigned int cmd,
- unsigned long arg)
+uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
{
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
+ struct uart_port *uport;
void __user *uarg = (void __user *)arg;
int ret = -ENOIOCTLCMD;
@@ -1266,8 +1298,9 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd,
goto out;
mutex_lock(&port->mutex);
+ uport = state->uart_port;
- if (tty->flags & (1 << TTY_IO_ERROR)) {
+ if (!uport || test_bit(TTY_IO_ERROR, &tty->flags)) {
ret = -EIO;
goto out_up;
}
@@ -1283,19 +1316,17 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd,
break;
case TIOCGRS485:
- ret = uart_get_rs485_config(state->uart_port, uarg);
+ ret = uart_get_rs485_config(uport, uarg);
break;
case TIOCSRS485:
- ret = uart_set_rs485_config(state->uart_port, uarg);
+ ret = uart_set_rs485_config(uport, uarg);
break;
- default: {
- struct uart_port *uport = state->uart_port;
+ default:
if (uport->ops->ioctl)
ret = uport->ops->ioctl(uport, cmd, arg);
break;
}
- }
out_up:
mutex_unlock(&port->mutex);
out:
@@ -1305,24 +1336,29 @@ out:
static void uart_set_ldisc(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
- if (uport->ops->set_ldisc) {
- mutex_lock(&state->port.mutex);
+ mutex_lock(&state->port.mutex);
+ uport = state->uart_port;
+ if (uport && uport->ops->set_ldisc)
uport->ops->set_ldisc(uport, &tty->termios);
- mutex_unlock(&state->port.mutex);
- }
+ mutex_unlock(&state->port.mutex);
}
static void uart_set_termios(struct tty_struct *tty,
struct ktermios *old_termios)
{
struct uart_state *state = tty->driver_data;
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
unsigned int cflag = tty->termios.c_cflag;
unsigned int iflag_mask = IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK;
bool sw_changed = false;
+ mutex_lock(&state->port.mutex);
+ uport = state->uart_port;
+ if (!uport)
+ goto out;
+
/*
* Drivers doing software flow control also need to know
* about changes to these input settings.
@@ -1345,12 +1381,10 @@ static void uart_set_termios(struct tty_struct *tty,
tty->termios.c_ispeed == old_termios->c_ispeed &&
((tty->termios.c_iflag ^ old_termios->c_iflag) & iflag_mask) == 0 &&
!sw_changed) {
- return;
+ goto out;
}
- mutex_lock(&state->port.mutex);
uart_change_speed(tty, state, old_termios);
- mutex_unlock(&state->port.mutex);
/* reload cflag from termios; port driver may have overriden flags */
cflag = tty->termios.c_cflag;
@@ -1364,6 +1398,8 @@ static void uart_set_termios(struct tty_struct *tty,
mask |= TIOCM_RTS;
uart_set_mctrl(uport, mask);
}
+out:
+ mutex_unlock(&state->port.mutex);
}
/*
@@ -1659,13 +1695,15 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
struct uart_state *state = drv->state + i;
struct tty_port *port = &state->port;
enum uart_pm_state pm_state;
- struct uart_port *uport = state->uart_port;
+ struct uart_port *uport;
char stat_buf[32];
unsigned int status;
int mmio;
+ mutex_lock(&port->mutex);
+ uport = state->uart_port;
if (!uport)
- return;
+ goto out;
mmio = uport->iotype >= UPIO_MEM;
seq_printf(m, "%d: uart:%s %s%08llX irq:%d",
@@ -1677,11 +1715,10 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
if (uport->type == PORT_UNKNOWN) {
seq_putc(m, '\n');
- return;
+ goto out;
}
if (capable(CAP_SYS_ADMIN)) {
- mutex_lock(&port->mutex);
pm_state = state->pm_state;
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, UART_PM_STATE_ON);
@@ -1690,7 +1727,6 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
spin_unlock_irq(&uport->lock);
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, pm_state);
- mutex_unlock(&port->mutex);
seq_printf(m, " tx:%d rx:%d",
uport->icount.tx, uport->icount.rx);
@@ -1732,6 +1768,9 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
seq_putc(m, '\n');
#undef STATBIT
#undef INFOBIT
+out:
+ mutex_unlock(&port->mutex);
+ return;
}
static int uart_proc_show(struct seq_file *m, void *v)
@@ -1995,11 +2034,11 @@ EXPORT_SYMBOL_GPL(uart_set_options);
static void uart_change_pm(struct uart_state *state,
enum uart_pm_state pm_state)
{
- struct uart_port *port = state->uart_port;
+ struct uart_port *uport = state->uart_port;
if (state->pm_state != pm_state) {
- if (port->ops->pm)
- port->ops->pm(port, pm_state, state->pm_state);
+ if (uport && uport->ops->pm)
+ uport->ops->pm(uport, pm_state, state->pm_state);
state->pm_state = pm_state;
}
}
@@ -2803,7 +2842,9 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
*/
uport->type = PORT_UNKNOWN;
+ mutex_lock(&port->mutex);
state->uart_port = NULL;
+ mutex_unlock(&port->mutex);
out:
mutex_unlock(&port_mutex);
--
2.6.3
next prev parent reply other threads:[~2015-12-13 0:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-13 0:36 [PATCH 0/8] Fix unsafe uart port access Peter Hurley
2015-12-13 0:36 ` [PATCH 1/8] serial: core: Fold __uart_put_char() into caller Peter Hurley
2015-12-13 0:36 ` [PATCH 2/8] serial: core: Fold do_uart_get_info() " Peter Hurley
2015-12-13 0:36 ` [PATCH 3/8] serial: core: Use tty->index for port # in debug messages Peter Hurley
2015-12-13 0:36 ` [PATCH 4/8] serial: core: Take port mutex for send_xchar() tty method Peter Hurley
2016-01-11 5:23 ` Peter Hurley
2015-12-13 0:36 ` [PATCH 5/8] serial: core: Expand port mutex section in uart_poll_init() Peter Hurley
2015-12-13 0:36 ` Peter Hurley [this message]
2015-12-13 0:36 ` [PATCH 7/8] serial: core: Prevent unsafe uart port access, part 2 Peter Hurley
2015-12-13 0:36 ` [PATCH 8/8] serial: core: Prevent unsafe uart port access, part 3 Peter Hurley
2016-01-11 5:29 ` [PATCH 0/8] Fix unsafe uart port access Peter Hurley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1449966992-4033-7-git-send-email-peter@hurleysoftware.com \
--to=peter@hurleysoftware.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.