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 v2 17/19] tty: Destroy ldisc instance on hangup
Date: Sat, 9 Jan 2016 20:41:08 -0800 [thread overview]
Message-ID: <1452400870-6005-18-git-send-email-peter@hurleysoftware.com> (raw)
In-Reply-To: <1452400870-6005-1-git-send-email-peter@hurleysoftware.com>
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>
---
drivers/tty/tty_io.c | 12 ++++++------
drivers/tty/tty_ldisc.c | 40 +++++++++++++++++-----------------------
include/linux/tty.h | 3 ++-
3 files changed, 25 insertions(+), 30 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f6f559c..fea8318 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -728,7 +728,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 != 0);
spin_lock_irq(&tty->ctrl_lock);
clear_bit(TTY_THROTTLED, &tty->flags);
@@ -753,10 +753,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);
@@ -1480,7 +1479,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 527fc5b..199e4b4 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
@@ -644,14 +647,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;
@@ -695,11 +699,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);
@@ -727,25 +729,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);
}
/**
diff --git a/include/linux/tty.h b/include/linux/tty.h
index ee73220..56d1133 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -493,7 +493,8 @@ extern int tty_set_termios(struct tty_struct *tty, struct ktermios *kt);
extern struct tty_ldisc *tty_ldisc_ref(struct tty_struct *);
extern void tty_ldisc_deref(struct tty_ldisc *);
extern struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *);
-extern void tty_ldisc_hangup(struct tty_struct *tty);
+extern void tty_ldisc_hangup(struct tty_struct *tty, bool reset);
+extern int tty_ldisc_reinit(struct tty_struct *tty, int disc);
extern const struct file_operations tty_ldiscs_proc_fops;
extern void tty_wakeup(struct tty_struct *tty);
--
2.7.0
next prev parent reply other threads:[~2016-01-10 4:42 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
2015-11-27 21:38 ` [PATCH 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
2015-11-27 21:38 ` [PATCH 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
2015-11-27 21:39 ` [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
2015-12-02 7:47 ` Marcel Holtmann
2015-11-27 21:39 ` [PATCH 04/19] NFC: nci: " Peter Hurley
2015-11-27 21:39 ` [PATCH 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
2015-11-27 21:39 ` [PATCH 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
2015-11-27 21:39 ` [PATCH 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
2015-11-27 21:39 ` [PATCH 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
2015-11-27 21:39 ` [PATCH 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
2015-11-27 21:39 ` [PATCH 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
2015-11-27 21:39 ` [PATCH 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
2015-11-27 21:39 ` [PATCH 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
2015-11-27 21:39 ` [PATCH 13/19] tty: Handle NULL tty->ldisc Peter Hurley
2015-11-27 21:39 ` [PATCH 14/19] tty: Move tty_ldisc_kill() Peter Hurley
2015-11-27 21:39 ` [PATCH 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
2015-11-27 21:39 ` [PATCH 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
2015-11-27 21:39 ` [PATCH 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
2015-11-27 21:39 ` [PATCH 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
2015-11-27 21:39 ` [PATCH 19/19] tty: Touch up style issues in ldisc core Peter Hurley
2016-01-10 4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
2016-01-10 4:40 ` [PATCH v2 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
2016-01-10 4:40 ` [PATCH v2 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
2016-01-10 4:40 ` [PATCH v2 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
2016-01-10 4:40 ` [PATCH v2 04/19] NFC: nci: " Peter Hurley
2016-01-10 4:40 ` [PATCH v2 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
2016-01-10 4:40 ` [PATCH v2 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
2016-01-10 5:24 ` Peter Hurley
2016-01-10 4:40 ` [PATCH v2 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
2016-01-10 5:26 ` Peter Hurley
2016-01-10 4:40 ` [PATCH v2 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
2016-01-10 4:41 ` [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
2016-01-10 23:16 ` Ben Hutchings
2016-01-11 0:25 ` Peter Hurley
2016-01-11 5:40 ` Peter Hurley
2016-01-11 10:37 ` Ben Hutchings
2016-01-10 4:41 ` [PATCH v2 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
2016-01-10 4:41 ` [PATCH v2 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
2016-01-10 4:41 ` [PATCH v2 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
2016-01-10 4:41 ` [PATCH v2 13/19] tty: Handle NULL tty->ldisc Peter Hurley
2016-01-10 4:41 ` [PATCH v2 14/19] tty: Move tty_ldisc_kill() Peter Hurley
2016-01-10 4:41 ` [PATCH v2 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
2016-01-10 4:41 ` [PATCH v2 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
2016-01-10 4:41 ` Peter Hurley [this message]
2016-01-10 6:24 ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup kbuild test robot
2016-01-10 6:24 ` [PATCH] tty: fix badzero.cocci warnings kbuild test robot
2016-01-10 7:02 ` Peter Hurley
2016-01-10 4:41 ` [PATCH v2 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
2016-01-10 4:41 ` [PATCH v2 19/19] tty: Avoid unnecessary temporaries for tty->ldisc Peter Hurley
2016-01-11 6:40 ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
2016-01-11 6:40 ` [PATCH v3 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
2016-01-11 6:40 ` [PATCH v3 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
2016-01-11 14:12 ` One Thousand Gnomes
2016-01-11 6:40 ` [PATCH v3 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
2016-01-11 6:40 ` [PATCH v3 04/19] NFC: nci: " Peter Hurley
2016-01-11 6:40 ` [PATCH v3 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
2016-01-11 6:40 ` [PATCH v3 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
2016-01-11 6:40 ` [PATCH v3 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
2016-01-11 6:40 ` [PATCH v3 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
2016-01-11 6:40 ` [PATCH v3 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
2016-01-11 6:40 ` [PATCH v3 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
2016-01-11 6:41 ` [PATCH v3 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
2016-01-11 6:41 ` [PATCH v3 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
2016-01-11 6:41 ` [PATCH v3 13/19] tty: Handle NULL tty->ldisc Peter Hurley
2016-01-11 6:41 ` [PATCH v3 14/19] tty: Move tty_ldisc_kill() Peter Hurley
2016-01-11 6:41 ` [PATCH v3 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
2016-01-11 6:41 ` [PATCH v3 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
2016-01-11 6:41 ` [PATCH v3 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
2016-01-11 6:41 ` [PATCH v3 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
2016-01-11 6:41 ` [PATCH v3 19/19] tty: Avoid unnecessary temporaries for tty->ldisc 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=1452400870-6005-18-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).