[lkml]   [2016]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 0/8] Fix unsafe uart port access
On 12/12/2015 04:36 PM, Peter Hurley wrote:
> 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

Hi Greg,

I split the first three patches above and submitted those
in the "Misc serial cleanups" v2 series.

For the remaining patches below, I need to rework how to
guarantee the lifespan of uart port during throttle/unthrottle,
and then rebase the following patches.

Peter Hurley

> 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(-)

 \ /
  Last update: 2016-01-11 07:21    [W:0.085 / U:36.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site