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 0/8] Fix unsafe uart port access
Date: Sat, 12 Dec 2015 16:36:24 -0800 [thread overview]
Message-ID: <1449966992-4033-1-git-send-email-peter@hurleysoftware.com> (raw)
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
next reply other threads:[~2015-12-13 0:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-13 0:36 Peter Hurley [this message]
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 ` [PATCH 6/8] serial: core: Prevent unsafe uart port access, part 1 Peter Hurley
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-1-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).