diff options
author | Peter Hurley <peter@hurleysoftware.com> | 2016-01-10 22:41:06 -0800 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2016-01-27 15:01:44 -0800 |
commit | 892d1fa7eaaed9d3c04954cb140c34ebc3393932 (patch) | |
tree | c85eb2b4043f4dcac9a57f610dd1aee7c9be1070 /drivers/tty | |
parent | 7896f30d6fc602f02198999acca4840620288990 (diff) | |
download | linux-892d1fa7eaaed9d3c04954cb140c34ebc3393932.tar.bz2 |
tty: Destroy ldisc instance on hangup
Currently, when the tty is hungup, the ldisc is re-instanced; ie., the
current instance is destroyed and a new instance is created. The purpose
of this design was to guarantee a valid, open ldisc for the lifetime of
the tty.
However, now that tty buffers are owned by and have lifetime equivalent
to the tty_port (since v3.10), any data received immediately after the
ldisc is re-instanced may cause continued driver i/o operations
concurrently with the driver's hangup() operation. For drivers that
shutdown h/w on hangup, this is unexpected and usually bad. For example,
the serial core may free the xmit buffer page concurrently with an
in-progress write() operation (triggered by echo).
With the existing stable and robust ldisc reference handling, the
cleaned-up tty_reopen(), the straggling unsafe ldisc use cleaned up, and
the preparation to properly handle a NULL tty->ldisc, the ldisc instance
can be destroyed and only re-instanced when the tty is re-opened.
If the tty was opened as /dev/console or /dev/tty0, the original behavior
of re-instancing the ldisc is retained (the 'reinit' parameter to
tty_ldisc_hangup() is true). This is required since those file descriptors
are never hungup.
This patch has neglible impact on userspace; the tty file_operations ptr
is changed to point to the hungup file operations _before_ the ldisc
instance is destroyed, so only racing file operations might now retrieve
a NULL ldisc reference (which is simply handled as if the hungup file
operation had been called instead -- see "tty: Prepare for destroying
line discipline on hangup").
This resolves a long-standing FIXME and several crash reports.
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/tty')
-rw-r--r-- | drivers/tty/tty_io.c | 12 | ||||
-rw-r--r-- | drivers/tty/tty_ldisc.c | 40 |
2 files changed, 23 insertions, 29 deletions
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 7f556e3c1515..d3ecbb513fa1 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -727,7 +727,7 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session) while (refs--) tty_kref_put(tty); - tty_ldisc_hangup(tty); + tty_ldisc_hangup(tty, cons_filp != NULL); spin_lock_irq(&tty->ctrl_lock); clear_bit(TTY_THROTTLED, &tty->flags); @@ -752,10 +752,9 @@ static void __tty_hangup(struct tty_struct *tty, int exit_session) } else if (tty->ops->hangup) tty->ops->hangup(tty); /* - * We don't want to have driver/ldisc interactions beyond - * the ones we did here. The driver layer expects no - * calls after ->hangup() from the ldisc side. However we - * can't yet guarantee all that. + * We don't want to have driver/ldisc interactions beyond the ones + * we did here. The driver layer expects no calls after ->hangup() + * from the ldisc side, which is now guaranteed. */ set_bit(TTY_HUPPED, &tty->flags); tty_unlock(tty); @@ -1475,7 +1474,8 @@ static int tty_reopen(struct tty_struct *tty) tty->count++; - WARN_ON(!tty->ldisc); + if (!tty->ldisc) + return tty_ldisc_reinit(tty, tty->termios.c_line); return 0; } diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c index e18c8e864110..c6f970d63060 100644 --- a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@ -257,6 +257,9 @@ const struct file_operations tty_ldiscs_proc_fops = { * reference to it. If the line discipline is in flux then * wait patiently until it changes. * + * Returns: NULL if the tty has been hungup and not re-opened with + * a new file descriptor, otherwise valid ldisc reference + * * Note: Must not be called from an IRQ/timer context. The caller * must also be careful not to hold other locks that will deadlock * against a discipline change, such as an existing ldisc reference @@ -642,14 +645,15 @@ static void tty_reset_termios(struct tty_struct *tty) * @disc: line discipline to reinitialize * * Completely reinitialize the line discipline state, by closing the - * current instance and opening a new instance. If an error occurs opening - * the new non-N_TTY instance, the instance is dropped and tty->ldisc reset - * to NULL. The caller can then retry with N_TTY instead. + * current instance, if there is one, and opening a new instance. If + * an error occurs opening the new non-N_TTY instance, the instance + * is dropped and tty->ldisc reset to NULL. The caller can then retry + * with N_TTY instead. * * Returns 0 if successful, otherwise error code < 0 */ -static int tty_ldisc_reinit(struct tty_struct *tty, int disc) +int tty_ldisc_reinit(struct tty_struct *tty, int disc) { struct tty_ldisc *ld; int retval; @@ -693,11 +697,9 @@ static int tty_ldisc_reinit(struct tty_struct *tty, int disc) * tty itself so we must be careful about locking rules. */ -void tty_ldisc_hangup(struct tty_struct *tty) +void tty_ldisc_hangup(struct tty_struct *tty, bool reinit) { struct tty_ldisc *ld; - int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS; - int err = 0; tty_ldisc_debug(tty, "%p: hangup\n", tty->ldisc); @@ -725,25 +727,17 @@ void tty_ldisc_hangup(struct tty_struct *tty) */ tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT); - if (tty->ldisc) { + if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) + tty_reset_termios(tty); - /* At this point we have a halted ldisc; we want to close it and - reopen a new ldisc. We could defer the reopen to the next - open but it means auditing a lot of other paths so this is - a FIXME */ - if (reset == 0) - err = tty_ldisc_reinit(tty, tty->termios.c_line); - - /* If the re-open fails or we reset then go to N_TTY. The - N_TTY open cannot fail */ - if (reset || err < 0) - tty_ldisc_reinit(tty, N_TTY); + if (tty->ldisc) { + if (reinit) { + if (tty_ldisc_reinit(tty, tty->termios.c_line) < 0) + tty_ldisc_reinit(tty, N_TTY); + } else + tty_ldisc_kill(tty); } tty_ldisc_unlock(tty); - if (reset) - tty_reset_termios(tty); - - tty_ldisc_debug(tty, "%p: re-opened\n", tty->ldisc); } /** |