[PATCH 0/8] Fix unsafe uart port access

* [PATCH 0/8] Fix unsafe uart port access
@ 2015-12-13  0:36 Peter Hurley
  2015-12-13  0:36 ` [PATCH 1/8] serial: core: Fold __uart_put_char() into caller Peter Hurley
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Peter Hurley @ 2015-12-13  0:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

Hi Greg,

The serial core has always intended to allow uart drivers to detach and
unload, even while ttys are open and running. Since the serial core is
the actual tty driver, it acts as a proxy for the uart driver, which
allows the uart driver to remove the port (which hangs up the tty) and
then unload.

However, all of this is broken.

Firstly, there is no mechanism for blocking the uart port removal until
current operations are complete. The tty core does not, and never has,
serialized ioctl() or any other tty operation other than open/close
with hangup.

Secondly, uart_register_driver() directly binds data from the uart driver
with the tty core, rather than providing proxy copies _without taking
module references to the uart driver_. This produces some spectacular
results if the tty is in-use when the uart driver unloads.

Thirdly, uart_unregister_driver() immediately destroys the tty port
despite that it may be in use, and will continue to be for the
lifetime of the tty, which is unbounded (userspace may retain open
ttys forever).

This series addresses the first problem of ensuring safe uart port
dereferencing with concurrent uart port removal. Two distinct
mechanisms are used to provide the necessary safe dereference: the
tty_port mutex and RCU.

By serializing the uart port detach (ie, reset to NULL) with the
tty_port mutex, existing sections of the serial core that already
hold the port mutex are guaranteed the uart port detach will not
be concurrent, and so can simply test for NULL value before first
dereference.

Most of the existing serial core that holds the port mutex is
ioctl-related and so can return -EIO as if the tty has been hungup
(which it has been).

Other portions of the serial core that sleep (eg. uart_wait_until_sent())
also now claim the port mutex to prevent concurrent uart port detach,
and thus protect the reference. The port mutex is dropped for the
actual sleep, retaken on wakeup and the uart port is re-checked
for NULL.

For portions of the serial core that don't sleep, RCU is used to
defer actual uart port teardown after detach (with synchronize_rcu())
until the current holders of rcu_read_lock() are complete.

The xmit buffer operations that don't modify the buffer indexes --
uart_write_room() and uart_chars_in_buffer() -- just report the state
of the xmit buffer anyway if the uart port has been removed, despite
that the xmit buffer is no longer lockable (the lock is in the
uart_port that is now gone).

Xmit buffer operations that do modify the buffer indexes are aborted.

Extra care is taken with the close/hangup/shutdown paths since this
must continue to perform other work even if the uart port has been
removed (for example, if the uart port was /dev/console and so will
only be closed when the file descriptor is released).

I do have a plan for the second and third problems but this series
does not implement those.

Regards,

Peter Hurley (8):
  serial: core: Fold __uart_put_char() into caller
  serial: core: Fold do_uart_get_info() into caller
  serial: core: Use tty->index for port # in debug messages
  serial: core: Take port mutex for send_xchar() tty method
  serial: core: Expand port mutex section in uart_poll_init()
  serial: core: Prevent unsafe uart port access, part 1
  serial: core: Prevent unsafe uart port access, part 2
  serial: core: Prevent unsafe uart port access, part 3

 drivers/tty/serial/8250/8250_port.c |   6 +-
 drivers/tty/serial/serial_core.c    | 547 +++++++++++++++++++++++-------------
 2 files changed, 355 insertions(+), 198 deletions(-)

-- 
2.6.3

^ permalink raw reply	[flat|nested] 11+ messages in thread