linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/19] Fix driver crashes on hangup
@ 2015-11-27 21:38 Peter Hurley
  2015-11-27 21:38 ` [PATCH 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
                   ` (19 more replies)
  0 siblings, 20 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

Hi Greg,

This series fixes the underlying design problem that leads to driver crashes
during hangup (eg., Andi Kleen's report https://lkml.org/lkml/2015/11/9/786).

Quoting from patch 17/19:

    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.

With this patch series, the tty core now guarantees no further driver/ldisc
interactions after hangup.

Patch 1-4 remove direct tty->ldisc access outside the tty core.
Patch 5 removes the defunct chars_in_buffer() ldisc method (which has been
        deprecated since 3.12)
Patch 6 & 7 fix unsafe ldisc uses which coincidentally have been discovered
        to cause crashes (https://lkml.org/lkml/2015/11/26/173 and
	https://lkml.org/lkml/2015/11/26/253). These have been tagged for
	-stable.
Patch 8-16 are preparations; documenting existing functions and refactoring.
        Patch 12 adds handling for the possibility of NULL ldisc references
	after tty_ldisc_ref_wait(); that commit log details the logic of
	why/how that works.
Patch 17 implements the fix: the ldisc instance is killed and left dead.
        At tty_reopen() if the tty->ldisc is NULL, a new ldisc is instanced.
Patch 18-19 are minor add-ons.


REQUIRES: tty: Simplify tty_set_ldisc() exit handling
	  "tty core printk cleanup" 14-patch series

Regards,

Peter Hurley (19):
  staging: digi: Replace open-coded tty_wakeup()
  serial: 68328: Remove bogus ldisc reset
  bluetooth: hci_ldisc: Remove dead code
  NFC: nci: Remove dead code
  tty: Remove chars_in_buffer() line discipline method
  tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  n_tty: Fix unsafe reference to "other" ldisc
  tty: Reset c_line from driver's init_termios
  staging/speakup: Use tty_ldisc_ref() for paste kworker
  tty: Fix comments for tty_ldisc_get()
  tty: Fix comments for tty_ldisc_release()
  tty: Prepare for destroying line discipline on hangup
  tty: Handle NULL tty->ldisc
  tty: Move tty_ldisc_kill()
  tty: Use 'disc' for line discipline index name
  tty: Refactor tty_ldisc_reinit() for reuse
  tty: Destroy ldisc instance on hangup
  tty: Document c_line == N_TTY initial condition
  tty: Touch up style issues in ldisc core

 Documentation/serial/tty.txt        |   3 -
 drivers/bluetooth/hci_ldisc.c       |   8 +-
 drivers/staging/dgap/dgap.c         |  28 ++----
 drivers/staging/dgnc/dgnc_tty.c     |  18 +---
 drivers/staging/speakup/selection.c |   4 +-
 drivers/tty/amiserial.c             |   6 +-
 drivers/tty/cyclades.c              |   8 +-
 drivers/tty/n_gsm.c                 |  16 ----
 drivers/tty/n_tty.c                 |  30 +------
 drivers/tty/rocket.c                |   6 +-
 drivers/tty/serial/68328serial.c    |  12 +--
 drivers/tty/serial/crisv10.c        |  12 ++-
 drivers/tty/tty_io.c                |  57 ++++++++++--
 drivers/tty/tty_ldisc.c             | 175 +++++++++++++++++++-----------------
 drivers/tty/vt/selection.c          |   2 +
 include/linux/tty.h                 |   5 +-
 include/linux/tty_ldisc.h           |   7 --
 net/nfc/nci/uart.c                  |   9 +-
 18 files changed, 176 insertions(+), 230 deletions(-)

-- 
2.6.3


^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH 01/19] staging: digi: Replace open-coded tty_wakeup()
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
@ 2015-11-27 21:38 ` Peter Hurley
  2015-11-27 21:38 ` [PATCH 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley, Lidza Louina,
	Daeseok Youn, Mark Hounschell

The open-coded tty_wakeup()s are attempts to workaround fixed bugs
in the line discipline write_wakeup() method. Replace with tty_wakeup().

Cc: Lidza Louina <lidza.louina@gmail.com>
Cc: Daeseok Youn <daeseok.youn@gmail.com>
Cc: Mark Hounschell <markh@compro.net>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/dgap/dgap.c     | 28 ++++++----------------------
 drivers/staging/dgnc/dgnc_tty.c | 18 ++----------------
 2 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index da59c5b..eb35b62 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1665,9 +1665,7 @@ static void dgap_input(struct channel_t *ch)
 }
 
 static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
-			      struct un_t *un, u32 mask,
-			      unsigned long *irq_flags1,
-			      unsigned long *irq_flags2)
+			      struct un_t *un, u32 mask)
 {
 	if (!(un->un_flags & mask))
 		return;
@@ -1677,17 +1675,7 @@ static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
 	if (!(un->un_flags & UN_ISOPEN))
 		return;
 
-	if ((un->un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-	    un->un_tty->ldisc->ops->write_wakeup) {
-		spin_unlock_irqrestore(&ch->ch_lock, *irq_flags2);
-		spin_unlock_irqrestore(&bd->bd_lock, *irq_flags1);
-
-		(un->un_tty->ldisc->ops->write_wakeup)(un->un_tty);
-
-		spin_lock_irqsave(&bd->bd_lock, *irq_flags1);
-		spin_lock_irqsave(&ch->ch_lock, *irq_flags2);
-	}
-	wake_up_interruptible(&un->un_tty->write_wait);
+	tty_wakeup(un->un_tty);
 	wake_up_interruptible(&un->un_flags_wait);
 }
 
@@ -1952,10 +1940,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit low.
 		 */
 		if (reason & IFTLW) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW);
 			if (ch->ch_flags & CH_WLOW) {
 				ch->ch_flags &= ~CH_WLOW;
 				wake_up_interruptible(&ch->ch_flags_wait);
@@ -1966,10 +1952,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit empty.
 		 */
 		if (reason & IFTEM) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY);
 			if (ch->ch_flags & CH_WEMPTY) {
 				ch->ch_flags &= ~CH_WEMPTY;
 				wake_up_interruptible(&ch->ch_flags_wait);
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 955a924..7a786b5 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -933,14 +933,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_tun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_tun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_tun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_tun.un_tty->ldisc->ops->write_wakeup(ch->ch_tun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_tun.un_tty->write_wait);
+		tty_wakeup(ch->ch_tun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
@@ -975,14 +968,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_pun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_pun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_pun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_pun.un_tty->ldisc->ops->write_wakeup(ch->ch_pun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_pun.un_tty->write_wait);
+		tty_wakeup(ch->ch_pun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 02/19] serial: 68328: Remove bogus ldisc reset
  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 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:38 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

As the #warning indicates, the open-coded ldisc reset was always not ok.
Not only is this code long dead, but now it would have no effect as
the ldisc is destroyed when this driver's close() method returns; remove.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/68328serial.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
index 9a6aaf0..a190a37 100644
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -1053,17 +1053,7 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
 		
 	tty_ldisc_flush(tty);
 	tty_port_tty_set(&info->tport, NULL);
-#warning "This is not and has never been valid so fix it"	
-#if 0
-	if (tty->ldisc.num != ldiscs[N_TTY].num) {
-		if (tty->ldisc.close)
-			(tty->ldisc.close)(tty);
-		tty->ldisc = ldiscs[N_TTY];
-		tty->termios.c_line = N_TTY;
-		if (tty->ldisc.open)
-			(tty->ldisc.open)(tty);
-	}
-#endif	
+
 	if (port->blocked_open) {
 		if (port->close_delay)
 			msleep_interruptible(jiffies_to_msecs(port->close_delay));
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code
  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 ` Peter Hurley
  2015-12-02  7:47   ` Marcel Holtmann
  2015-11-27 21:39 ` [PATCH 04/19] NFC: nci: " Peter Hurley
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley,
	Marcel Holtmann, Johan Hedberg, linux-bluetooth

The N_HCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: <linux-bluetooth@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/bluetooth/hci_ldisc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 96bcec5..c303f87 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -461,13 +461,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	INIT_WORK(&hu->init_ready, hci_uart_init_work);
 	INIT_WORK(&hu->write_work, hci_uart_write_work);
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	   open path is before the ldisc is referencable */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 04/19] NFC: nci: Remove dead code
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (2 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley,
	Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	David S. Miller, linux-wireless

The N_NCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Cc: Lauro Ramos Venancio <lauro.venancio@openbossa.org>
Cc: Aloisio Almeida Jr <aloisio.almeida@openbossa.org>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <linux-wireless@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/nfc/nci/uart.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index 21d8875..c468eab 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -171,14 +171,7 @@ static int nci_uart_tty_open(struct tty_struct *tty)
 	tty->disc_data = NULL;
 	tty->receive_room = 65536;
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	 * open path is before the ldisc is referencable.
-	 */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 05/19] tty: Remove chars_in_buffer() line discipline method
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (3 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 04/19] NFC: nci: " Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

The chars_in_buffer() line discipline method serves no functional
purpose, other than as a (dubious) debugging aid for mostly bit-rotting
drivers. Despite being documented as an optional method, every caller
is unconditionally executed (although conditionally compiled).
Furthermore, direct tty->ldisc access without an ldisc ref is unsafe.
Lastly, N_TTY's chars_in_buffer() has warned of removal since 3.12.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 Documentation/serial/tty.txt |  3 ---
 drivers/tty/amiserial.c      |  6 ++----
 drivers/tty/cyclades.c       |  8 ++++----
 drivers/tty/n_gsm.c          | 16 ----------------
 drivers/tty/n_tty.c          | 23 -----------------------
 drivers/tty/rocket.c         |  6 ++----
 drivers/tty/serial/crisv10.c | 12 +++++-------
 include/linux/tty_ldisc.h    |  7 -------
 8 files changed, 13 insertions(+), 68 deletions(-)

diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
index bc3842d..798cba8 100644
--- a/Documentation/serial/tty.txt
+++ b/Documentation/serial/tty.txt
@@ -72,9 +72,6 @@ flush_buffer()	-	(optional) May be called at any point between
 			open and close, and instructs the line discipline
 			to empty its input buffer.
 
-chars_in_buffer() -	(optional) Report the number of bytes in the input
-			buffer.
-
 set_termios()	-	(optional) Called on termios structure changes.
 			The caller passes the old termios data and the
 			current data is in the tty. Called under the
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index bffc0a4..b9efc3b 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -965,8 +965,7 @@ static void rs_throttle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_throttle"))
@@ -988,8 +987,7 @@ static void rs_unthrottle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_unthrottle"))
diff --git a/drivers/tty/cyclades.c b/drivers/tty/cyclades.c
index d4a1331..ee94ece 100644
--- a/drivers/tty/cyclades.c
+++ b/drivers/tty/cyclades.c
@@ -2852,8 +2852,8 @@ static void cy_throttle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:throttle %s: %ld...ttyC%d\n", tty_name(tty),
-			tty->ldisc.chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:throttle %s ...ttyC%d\n", tty_name(tty),
+			 info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_throttle"))
@@ -2891,8 +2891,8 @@ static void cy_unthrottle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:unthrottle %s: %ld...ttyC%d\n",
-		tty_name(tty), tty_chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:unthrottle %s ...ttyC%d\n",
+		tty_name(tty), info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_unthrottle"))
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c3fe026..e3cc277 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2304,21 +2304,6 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 }
 
 /**
- *	gsmld_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: gsm lock
- */
-
-static ssize_t gsmld_chars_in_buffer(struct tty_struct *tty)
-{
-	return 0;
-}
-
-/**
  *	gsmld_flush_buffer	-	clean input queue
  *	@tty:	terminal device
  *
@@ -2830,7 +2815,6 @@ static struct tty_ldisc_ops tty_ldisc_packet = {
 	.open            = gsmld_open,
 	.close           = gsmld_close,
 	.flush_buffer    = gsmld_flush_buffer,
-	.chars_in_buffer = gsmld_chars_in_buffer,
 	.read            = gsmld_read,
 	.write           = gsmld_write,
 	.ioctl           = gsmld_ioctl,
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 9b0b610..2dddc72 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -385,28 +385,6 @@ static void n_tty_flush_buffer(struct tty_struct *tty)
 }
 
 /**
- *	n_tty_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: exclusive termios_rwsem
- */
-
-static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
-{
-	ssize_t n;
-
-	WARN_ONCE(1, "%s is deprecated and scheduled for removal.", __func__);
-
-	down_write(&tty->termios_rwsem);
-	n = chars_in_buffer(tty);
-	up_write(&tty->termios_rwsem);
-	return n;
-}
-
-/**
  *	is_utf8_continuation	-	utf8 multibyte check
  *	@c: byte to check
  *
@@ -2534,7 +2512,6 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.open            = n_tty_open,
 	.close           = n_tty_close,
 	.flush_buffer    = n_tty_flush_buffer,
-	.chars_in_buffer = n_tty_chars_in_buffer,
 	.read            = n_tty_read,
 	.write           = n_tty_write,
 	.ioctl           = n_tty_ioctl,
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index 5905200..b575f05 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -1355,8 +1355,7 @@ static void rp_throttle(struct tty_struct *tty)
 	struct r_port *info = tty->driver_data;
 
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "throttle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "throttle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_throttle"))
@@ -1372,8 +1371,7 @@ static void rp_unthrottle(struct tty_struct *tty)
 {
 	struct r_port *info = tty->driver_data;
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "unthrottle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "unthrottle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_unthrottle"))
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index 0100410..5f2bd78 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -2968,7 +2968,7 @@ static int rs_raw_write(struct tty_struct *tty,
 
 	local_save_flags(flags);
 	DFLOW(DEBUG_LOG(info->line, "write count %i ", count));
-	DFLOW(DEBUG_LOG(info->line, "ldisc %i\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line, "ldisc\n"));
 
 
 	/* The local_irq_disable/restore_flags pairs below are needed
@@ -3161,10 +3161,9 @@ rs_throttle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_throttle %lu\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_throttle\n"));
 
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
@@ -3181,10 +3180,9 @@ rs_unthrottle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc %d\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc\n"));
 	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle flip.count: %i\n", tty->flip.count));
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 0f3b19f..db0abe56 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -25,12 +25,6 @@
  *	buffers of any input characters it may have queued to be
  *	delivered to the user mode process.
  *
- * ssize_t (*chars_in_buffer)(struct tty_struct *tty);
- *
- *	This function returns the number of input characters the line
- *	discipline may have queued up to be delivered to the user mode
- *	process.
- *
  * ssize_t (*read)(struct tty_struct * tty, struct file * file,
  *		   unsigned char * buf, size_t nr);
  *
@@ -196,7 +190,6 @@ struct tty_ldisc_ops {
 	int	(*open)(struct tty_struct *);
 	void	(*close)(struct tty_struct *);
 	void	(*flush_buffer)(struct tty_struct *tty);
-	ssize_t	(*chars_in_buffer)(struct tty_struct *tty);
 	ssize_t	(*read)(struct tty_struct *tty, struct file *file,
 			unsigned char __user *buf, size_t nr);
 	ssize_t	(*write)(struct tty_struct *tty, struct file *file,
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (4 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley, stable

ioctl(TIOCGETD) retrieves the line discipline id directly from the
ldisc because the line discipline id (c_line) in termios is untrustworthy;
userspace may have set termios via ioctl(TCSETS*) without actually
changing the line discipline via ioctl(TIOCSETD).

However, directly accessing the current ldisc via tty->ldisc is
unsafe; the ldisc ptr dereferenced may be stale if the line discipline
is changing via ioctl(TIOCSETD) or hangup.

Wait for the line discipline reference (just like read() or write())
to retrieve the "current" line discipline id.

Cc: <stable@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 892c923..56d3a6b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2649,6 +2649,28 @@ static int tiocsetd(struct tty_struct *tty, int __user *p)
 }
 
 /**
+ *	tiocgetd	-	get line discipline
+ *	@tty: tty device
+ *	@p: pointer to user data
+ *
+ *	Retrieves the line discipline id directly from the ldisc.
+ *
+ *	Locking: waits for ldisc reference (in case the line discipline
+ *		is changing or the tty is being hungup)
+ */
+
+static int tiocgetd(struct tty_struct *tty, int __user *p)
+{
+	struct tty_ldisc *ld;
+	int ret;
+
+	ld = tty_ldisc_ref_wait(tty);
+	ret = put_user(ld->ops->num, p);
+	tty_ldisc_deref(ld);
+	return ret;
+}
+
+/**
  *	send_break	-	performed time break
  *	@tty: device to break on
  *	@duration: timeout in mS
@@ -2874,7 +2896,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case TIOCGSID:
 		return tiocgsid(tty, real_tty, p);
 	case TIOCGETD:
-		return put_user(tty->ldisc->ops->num, (int __user *)p);
+		return tiocgetd(tty, p);
 	case TIOCSETD:
 		return tiocsetd(tty, p);
 	case TIOCVHANGUP:
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 07/19] n_tty: Fix unsafe reference to "other" ldisc
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (5 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley, stable

Although n_tty_check_unthrottle() has a valid ldisc reference (since
the tty core gets the ldisc ref in tty_read() before calling the line
discipline read() method), it does not have a valid ldisc reference to
the "other" pty of a pty pair. Since getting an ldisc reference for
tty->link essentially open-codes tty_wakeup(), just replace with the
equivalent tty_wakeup().

Cc: <stable@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 2dddc72..703b219 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -270,16 +270,13 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 
 static void n_tty_check_unthrottle(struct tty_struct *tty)
 {
-	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
-	    tty->link->ldisc->ops->write_wakeup == n_tty_write_wakeup) {
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
 		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
 			return;
 		if (!tty->count)
 			return;
 		n_tty_kick_worker(tty);
-		n_tty_write_wakeup(tty->link);
-		if (waitqueue_active(&tty->link->write_wait))
-			wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
+		tty_wakeup(tty->link);
 		return;
 	}
 
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 08/19] tty: Reset c_line from driver's init_termios
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (6 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

After the ldisc is released, but before the tty is destroyed, the termios
is saved (in tty_free_termios()); this termios is restored if a new
tty is created on next open(). However, the line discipline is always
reset, which is not obvious in the current method. Instead, reset
as part of the restore.

Restore the original line discipline, which may not have been N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    | 5 +++--
 drivers/tty/tty_ldisc.c | 3 ---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 56d3a6b..0871d1e 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1388,9 +1388,10 @@ int tty_init_termios(struct tty_struct *tty)
 	else {
 		/* Check for lazy saved data */
 		tp = tty->driver->termios[idx];
-		if (tp != NULL)
+		if (tp != NULL) {
 			tty->termios = *tp;
-		else
+			tty->termios.c_line  = tty->driver->init_termios.c_line;
+		} else
 			tty->termios = tty->driver->init_termios;
 	}
 	/* Compatibility until drivers always set this */
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index c97f8e9..a9b2fd7 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -765,9 +765,6 @@ static void tty_ldisc_kill(struct tty_struct *tty)
 	tty_ldisc_put(tty->ldisc);
 	/* Force an oops if we mess this up */
 	tty->ldisc = NULL;
-
-	/* Ensure the next open requests the N_TTY ldisc */
-	tty_set_termios_ldisc(tty, N_TTY);
 }
 
 /**
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (7 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley, stable

As the function documentation for tty_ldisc_ref_wait() notes, it is
only callable from a tty file_operations routine; otherwise there
is no guarantee the ref won't be NULL.

The key difference with the VT's paste_selection() is that is an ioctl,
where __speakup_paste_selection() is completely asynch kworker, kicked
off from interrupt context.

Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
       tty (ab)usage to match vt")
Cc: <stable@vger.kernel.org>

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/speakup/selection.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
index aa5ab6c..86c0b9a 100644
--- a/drivers/staging/speakup/selection.c
+++ b/drivers/staging/speakup/selection.c
@@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
 	struct tty_ldisc *ld;
 	DECLARE_WAITQUEUE(wait, current);
 
-	ld = tty_ldisc_ref_wait(tty);
+	ld = tty_ldisc_ref(tty);
+	if (!ld)
+		return;
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 10/19] tty: Fix comments for tty_ldisc_get()
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (8 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

tty_ldisc_get() returns ERR_PTR() values if unsuccessful, not NULL;
fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index a9b2fd7..33764ea 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -140,9 +140,16 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
  *	@disc: ldisc number
  *
  *	Takes a reference to a line discipline. Deals with refcounts and
- *	module locking counts. Returns NULL if the discipline is not available.
- *	Returns a pointer to the discipline and bumps the ref count if it is
- *	available
+ *	module locking counts.
+ *
+ *	Returns: -EINVAL if the discipline index is not [N_TTY..NR_LDISCS] or
+ *			 if the discipline is not registered
+ *		 -EAGAIN if request_module() failed to load or register the
+ *			 the discipline
+ *		 -ENOMEM if allocation failure
+ *
+ *		 Otherwise, returns a pointer to the discipline and bumps the
+ *		 ref count
  *
  *	Locking:
  *		takes tty_ldiscs_lock to guard against ldisc races
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 11/19] tty: Fix comments for tty_ldisc_release()
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (9 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

tty_ldisc_kill() sets tty->ldisc to NULL; _not_ to N_TTY with a valid
but unopened ldisc. Fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 33764ea..e769f5a 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -779,8 +779,7 @@ static void tty_ldisc_kill(struct tty_struct *tty)
  *	@tty: tty being shut down (or one end of pty pair)
  *
  *	Called during the final close of a tty or a pty pair in order to shut
- *	down the line discpline layer. On exit, each ldisc assigned is N_TTY and
- *	each ldisc has not been opened.
+ *	down the line discpline layer. On exit, each tty's ldisc is NULL.
  */
 
 void tty_ldisc_release(struct tty_struct *tty)
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 12/19] tty: Prepare for destroying line discipline on hangup
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (10 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 13/19] tty: Handle NULL tty->ldisc Peter Hurley
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

tty file_operations (read/write/ioctl) wait for the ldisc reference
indefinitely (until ldisc lifetime events, such as hangup or TIOCSETD,
finish). Since hangup now destroys the ldisc and does not instance
another copy, file_operations must now be prepared to receive a NULL
ldisc reference from tty_ldisc_ref_wait():

CPU 0                                   CPU 1
-----                                   -----
(*f_op->read)() => tty_read()
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()
ld = tty_ldisc_ref_wait()
/* ld == NULL */

Instead, the action taken now is to return the same value as if the
tty had been hungup a moment earlier:

CPU 0                                   CPU 1
-----                                   -----
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
(*f_op->read)() => hung_up_tty_read()
return 0;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c       | 14 ++++++++++++++
 drivers/tty/tty_ldisc.c    |  4 ++--
 drivers/tty/vt/selection.c |  2 ++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0871d1e..89822c4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1069,6 +1069,8 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
 	/* We want to wait for the line discipline to sort out in this
 	   situation */
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_read(file, buf, count, ppos);
 	if (ld->ops->read)
 		i = ld->ops->read(tty, file, buf, count);
 	else
@@ -1243,6 +1245,8 @@ static ssize_t tty_write(struct file *file, const char __user *buf,
 	if (tty->ops->write_room == NULL)
 		tty_err(tty, "missing write_room method\n");
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_write(file, buf, count, ppos);
 	if (!ld->ops->write)
 		ret = -EIO;
 	else
@@ -2185,6 +2189,8 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 		return 0;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_poll(filp, wait);
 	if (ld->ops->poll)
 		ret = ld->ops->poll(tty, filp, wait);
 	tty_ldisc_deref(ld);
@@ -2274,6 +2280,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 		return -EFAULT;
 	tty_audit_tiocsti(tty, ch);
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ld->ops->receive_buf(tty, &ch, &mbz, 1);
 	tty_ldisc_deref(ld);
 	return 0;
@@ -2666,6 +2674,8 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
 	int ret;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ret = put_user(ld->ops->num, p);
 	tty_ldisc_deref(ld);
 	return ret;
@@ -2963,6 +2973,8 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			return retval;
 	}
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_ioctl(file, cmd, arg);
 	retval = -EINVAL;
 	if (ld->ops->ioctl) {
 		retval = ld->ops->ioctl(tty, file, cmd, arg);
@@ -2991,6 +3003,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 	}
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_compat_ioctl(file, cmd, arg);
 	if (ld->ops->compat_ioctl)
 		retval = ld->ops->compat_ioctl(tty, file, cmd, arg);
 	else
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index e769f5a..e9a19c9 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -262,8 +262,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
  *	against a discipline change, such as an existing ldisc reference
  *	(which we check for)
  *
- *	Note: only callable from a file_operations routine (which
- *	guarantees tty->ldisc != NULL when the lock is acquired).
+ *	Note: a file_operations routine (read/poll/write) should use this
+ *	function to wait for any ldisc lifetime events to finish.
  */
 
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 381a2b1..4dd9dd2 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -347,6 +347,8 @@ int paste_selection(struct tty_struct *tty)
 	console_unlock();
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;	/* ldisc was hung up */
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 13/19] tty: Handle NULL tty->ldisc
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (11 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 14/19] tty: Move tty_ldisc_kill() Peter Hurley
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

In preparation of destroying line discipline on hangup, fix
ldisc core operations to properly handle when the tty's ldisc is
NULL.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index e9a19c9..680c049 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -269,7 +269,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
 {
 	ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
-	WARN_ON(!tty->ldisc);
+	if (!tty->ldisc)
+		ldsem_up_read(&tty->ldisc_sem);
 	return tty->ldisc;
 }
 EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait);
@@ -482,7 +483,7 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
 		if (ret)
 			clear_bit(TTY_LDISC_OPEN, &tty->flags);
 
-		tty_ldisc_debug(tty, "%p: opened\n", tty->ldisc);
+		tty_ldisc_debug(tty, "%p: opened\n", ld);
 		return ret;
 	}
 	return 0;
@@ -503,7 +504,7 @@ static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld)
 	clear_bit(TTY_LDISC_OPEN, &tty->flags);
 	if (ld->ops->close)
 		ld->ops->close(tty);
-	tty_ldisc_debug(tty, "%p: closed\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: closed\n", ld);
 }
 
 /**
@@ -566,6 +567,11 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	if (retval)
 		goto err;
 
+	if (!tty->ldisc) {
+		retval = -EIO;
+		goto out;
+	}
+
 	/* Check the no-op case */
 	if (tty->ldisc->ops->num == ldisc)
 		goto out;
@@ -681,7 +687,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 	int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS;
 	int err = 0;
 
-	tty_ldisc_debug(tty, "%p: closing\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: hangup\n", tty->ldisc);
 
 	ld = tty_ldisc_ref(tty);
 	if (ld != NULL) {
@@ -765,6 +771,8 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 
 static void tty_ldisc_kill(struct tty_struct *tty)
 {
+	if (!tty->ldisc)
+		return;
 	/*
 	 * Now kill off the ldisc
 	 */
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 14/19] tty: Move tty_ldisc_kill()
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (12 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 13/19] tty: Handle NULL tty->ldisc Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

In preparation for destroying the line discipline instance on hangup,
move tty_ldisc_kill() to eliminate needless forward declarations.
No functional change.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 680c049..748d9a9 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -623,6 +623,25 @@ err:
 }
 
 /**
+ *	tty_ldisc_kill	-	teardown ldisc
+ *	@tty: tty being released
+ *
+ *	Perform final close of the ldisc and reset tty->ldisc
+ */
+static void tty_ldisc_kill(struct tty_struct *tty)
+{
+	if (!tty->ldisc)
+		return;
+	/*
+	 * Now kill off the ldisc
+	 */
+	tty_ldisc_close(tty, tty->ldisc);
+	tty_ldisc_put(tty->ldisc);
+	/* Force an oops if we mess this up */
+	tty->ldisc = NULL;
+}
+
+/**
  *	tty_reset_termios	-	reset terminal state
  *	@tty: tty to reset
  *
@@ -769,19 +788,6 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 	return 0;
 }
 
-static void tty_ldisc_kill(struct tty_struct *tty)
-{
-	if (!tty->ldisc)
-		return;
-	/*
-	 * Now kill off the ldisc
-	 */
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/* Force an oops if we mess this up */
-	tty->ldisc = NULL;
-}
-
 /**
  *	tty_ldisc_release		-	release line discipline
  *	@tty: tty being shut down (or one end of pty pair)
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 15/19] tty: Use 'disc' for line discipline index name
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (13 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 14/19] tty: Move tty_ldisc_kill() Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

tty->ldisc is a ptr to struct tty_ldisc, but unfortunately 'ldisc' is
also used as a parameter or local name to refer to the line discipline
index value (ie, N_TTY, N_GSM, etc.); instead prefer the name used
by the line discipline registration/ref counting functions.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    |  6 +++---
 drivers/tty/tty_ldisc.c | 22 +++++++++++-----------
 include/linux/tty.h     |  2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 89822c4..f6f559c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2646,13 +2646,13 @@ static int tiocgsid(struct tty_struct *tty, struct tty_struct *real_tty, pid_t _
 
 static int tiocsetd(struct tty_struct *tty, int __user *p)
 {
-	int ldisc;
+	int disc;
 	int ret;
 
-	if (get_user(ldisc, p))
+	if (get_user(disc, p))
 		return -EFAULT;
 
-	ret = tty_set_ldisc(tty, ldisc);
+	ret = tty_set_ldisc(tty, disc);
 
 	return ret;
 }
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 748d9a9..670b0ab 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -439,7 +439,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_closing);
 /**
  *	tty_set_termios_ldisc		-	set ldisc field
  *	@tty: tty structure
- *	@num: line discipline number
+ *	@disc: line discipline number
  *
  *	This is probably overkill for real world processors but
  *	they are not on hot paths so a little discipline won't do
@@ -452,10 +452,10 @@ EXPORT_SYMBOL_GPL(tty_ldisc_closing);
  *	Locking: takes termios_rwsem
  */
 
-static void tty_set_termios_ldisc(struct tty_struct *tty, int num)
+static void tty_set_termios_ldisc(struct tty_struct *tty, int disc)
 {
 	down_write(&tty->termios_rwsem);
-	tty->termios.c_line = num;
+	tty->termios.c_line = disc;
 	up_write(&tty->termios_rwsem);
 
 	tty->disc_data = NULL;
@@ -553,12 +553,12 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
  *	the close of one side of a tty/pty pair, and eventually hangup.
  */
 
-int tty_set_ldisc(struct tty_struct *tty, int ldisc)
+int tty_set_ldisc(struct tty_struct *tty, int disc)
 {
 	int retval;
 	struct tty_ldisc *old_ldisc, *new_ldisc;
 
-	new_ldisc = tty_ldisc_get(tty, ldisc);
+	new_ldisc = tty_ldisc_get(tty, disc);
 	if (IS_ERR(new_ldisc))
 		return PTR_ERR(new_ldisc);
 
@@ -573,7 +573,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	}
 
 	/* Check the no-op case */
-	if (tty->ldisc->ops->num == ldisc)
+	if (tty->ldisc->ops->num == disc)
 		goto out;
 
 	if (test_bit(TTY_HUPPED, &tty->flags)) {
@@ -589,7 +589,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 
 	/* Now set up the new line discipline. */
 	tty->ldisc = new_ldisc;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	retval = tty_ldisc_open(tty, new_ldisc);
 	if (retval < 0) {
@@ -661,15 +661,15 @@ static void tty_reset_termios(struct tty_struct *tty)
 /**
  *	tty_ldisc_reinit	-	reinitialise the tty ldisc
  *	@tty: tty to reinit
- *	@ldisc: line discipline to reinitialize
+ *	@disc: line discipline to reinitialize
  *
  *	Switch the tty to a line discipline and leave the ldisc
  *	state closed
  */
 
-static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
+static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, ldisc);
+	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
 
 	if (IS_ERR(ld))
 		return -1;
@@ -680,7 +680,7 @@ static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
 	 *	Switch the line discipline back
 	 */
 	tty->ldisc = ld;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	return 0;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 754ed54..4989c29 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -575,7 +575,7 @@ static inline int tty_port_users(struct tty_port *port)
 
 extern int tty_register_ldisc(int disc, struct tty_ldisc_ops *new_ldisc);
 extern int tty_unregister_ldisc(int disc);
-extern int tty_set_ldisc(struct tty_struct *tty, int ldisc);
+extern int tty_set_ldisc(struct tty_struct *tty, int disc);
 extern int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty);
 extern void tty_ldisc_release(struct tty_struct *tty);
 extern void tty_ldisc_init(struct tty_struct *tty);
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 16/19] tty: Refactor tty_ldisc_reinit() for reuse
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (14 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

At tty hangup, the line discipline instance is reinitialized by
closing the current ldisc instance and opening a new instance.
This operation is complicated by error recovery: if the attempt
to reinit the current line discipline fails, the line discipline
is reset to N_TTY (which should not but can fail).

Re-purpose tty_ldisc_reinit() to return a valid, open line discipline
instance, or otherwise, an error.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 53 +++++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 670b0ab..14bb40d 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -663,26 +663,41 @@ static void tty_reset_termios(struct tty_struct *tty)
  *	@tty: tty to reinit
  *	@disc: line discipline to reinitialize
  *
- *	Switch the tty to a line discipline and leave the ldisc
- *	state closed
+ *	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.
+ *
+ *	Returns 0 if successful, otherwise error code < 0
  */
 
 static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
+	struct tty_ldisc *ld;
+	int retval;
 
-	if (IS_ERR(ld))
-		return -1;
+	ld = tty_ldisc_get(tty, disc);
+	if (IS_ERR(ld)) {
+		BUG_ON(disc == N_TTY);
+		return PTR_ERR(ld);
+	}
 
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/*
-	 *	Switch the line discipline back
-	 */
+	if (tty->ldisc) {
+		tty_ldisc_close(tty, tty->ldisc);
+		tty_ldisc_put(tty->ldisc);
+	}
+
+	/* switch the line discipline */
 	tty->ldisc = ld;
 	tty_set_termios_ldisc(tty, disc);
-
-	return 0;
+	retval = tty_ldisc_open(tty, tty->ldisc);
+	if (retval) {
+		if (!WARN_ON(disc == N_TTY)) {
+			tty_ldisc_put(tty->ldisc);
+			tty->ldisc = NULL;
+		}
+	}
+	return retval;
 }
 
 /**
@@ -738,19 +753,13 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 		   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) {
+		if (reset == 0)
+			err = tty_ldisc_reinit(tty, tty->termios.c_line);
 
-			if (!tty_ldisc_reinit(tty, tty->termios.c_line))
-				err = tty_ldisc_open(tty, tty->ldisc);
-			else
-				err = 1;
-		}
 		/* If the re-open fails or we reset then go to N_TTY. The
 		   N_TTY open cannot fail */
-		if (reset || err) {
-			BUG_ON(tty_ldisc_reinit(tty, N_TTY));
-			WARN_ON(tty_ldisc_open(tty, tty->ldisc));
-		}
+		if (reset || err < 0)
+			tty_ldisc_reinit(tty, N_TTY);
 	}
 	tty_ldisc_unlock(tty);
 	if (reset)
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 17/19] tty: Destroy ldisc instance on hangup
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (15 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2015-11-27 21:39 ` [PATCH 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

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    |  5 +++--
 drivers/tty/tty_ldisc.c | 40 +++++++++++++++++-----------------------
 include/linux/tty.h     |  3 ++-
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index f6f559c..5198163 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);
@@ -1480,7 +1480,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 14bb40d..4f292d4 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
@@ -664,14 +667,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;
@@ -715,11 +719,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);
 
@@ -747,25 +749,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 4989c29..8fd65e6 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -492,7 +492,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.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 18/19] tty: Document c_line == N_TTY initial condition
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (16 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
@ 2015-11-27 21:39 ` 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
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

The line discipline id is stored in the tty's termios; document the
implicit initial value of N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 5198163..c1867e5 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -123,7 +123,8 @@ struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
 		   ECHOCTL | ECHOKE | IEXTEN,
 	.c_cc = INIT_C_CC,
 	.c_ispeed = 38400,
-	.c_ospeed = 38400
+	.c_ospeed = 38400,
+	/* .c_line = N_TTY, */
 };
 
 EXPORT_SYMBOL(tty_std_termios);
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH 19/19] tty: Touch up style issues in ldisc core
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (17 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
@ 2015-11-27 21:39 ` Peter Hurley
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2015-11-27 21:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Andi Kleen, Peter Hurley

Combined declaration/initialization statements that perform
substantial or important operations are confusing if separated from
the main body by newline; refactor if necessary and remove the newline.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 4f292d4..3455908 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -290,7 +290,6 @@ EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait);
 struct tty_ldisc *tty_ldisc_ref(struct tty_struct *tty)
 {
 	struct tty_ldisc *ld = NULL;
-
 	if (ldsem_down_read_trylock(&tty->ldisc_sem)) {
 		ld = tty->ldisc;
 		if (!ld)
@@ -412,7 +411,6 @@ static void __lockfunc tty_ldisc_unlock_pair(struct tty_struct *tty,
 void tty_ldisc_flush(struct tty_struct *tty)
 {
 	struct tty_ldisc *ld = tty_ldisc_ref(tty);
-
 	tty_buffer_flush(tty, ld);
 	if (ld)
 		tty_ldisc_deref(ld);
@@ -430,7 +428,6 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
 void tty_ldisc_closing(struct tty_struct *tty)
 {
 	struct tty_ldisc *ld = tty_ldisc_ref(tty);
-
 	if (ld) {
 		if (ld->ops->closing)
 			ld->ops->closing(tty);
@@ -593,7 +590,6 @@ int tty_set_ldisc(struct tty_struct *tty, int disc)
 	/* Now set up the new line discipline. */
 	tty->ldisc = new_ldisc;
 	tty_set_termios_ldisc(tty, disc);
-
 	retval = tty_ldisc_open(tty, new_ldisc);
 	if (retval < 0) {
 		/* Back to the old one or N_TTY if we can't */
@@ -774,17 +770,14 @@ void tty_ldisc_hangup(struct tty_struct *tty, bool reinit)
 
 int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 {
-	struct tty_ldisc *ld = tty->ldisc;
-	int retval;
-
-	retval = tty_ldisc_open(tty, ld);
+	int retval = tty_ldisc_open(tty, tty->ldisc);
 	if (retval)
 		return retval;
 
 	if (o_tty) {
 		retval = tty_ldisc_open(o_tty, o_tty->ldisc);
 		if (retval) {
-			tty_ldisc_close(tty, ld);
+			tty_ldisc_close(tty, tty->ldisc);
 			return retval;
 		}
 	}
-- 
2.6.3


^ permalink raw reply related	[flat|nested] 71+ messages in thread
* Re: [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code
  2015-11-27 21:39 ` [PATCH 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
@ 2015-12-02  7:47   ` Marcel Holtmann
  0 siblings, 0 replies; 71+ messages in thread
From: Marcel Holtmann @ 2015-12-02  7:47 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Greg Kroah-Hartman, Jiri Slaby, LKML, Andi Kleen, Johan Hedberg,
	linux-bluetooth

Hi Peter,

> The N_HCI ldisc does not define a flush_buffer() ldisc method, so
> the check when opening the ldisc is always false.
> 
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: <linux-bluetooth@vger.kernel.org>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH v2 00/19] Fix driver crashes on hangup
  2015-11-27 21:38 [PATCH 00/19] Fix driver crashes on hangup Peter Hurley
                   ` (18 preceding siblings ...)
  2015-11-27 21:39 ` [PATCH 19/19] tty: Touch up style issues in ldisc core Peter Hurley
@ 2016-01-10  4:40 ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
                     ` (19 more replies)
  19 siblings, 20 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

Changes for v2:
	Rebased on top of current tty-next
	Reduced changes/re-titled patch 19

NB: Marcel already picked up "bluetooth: hci_ldisc: Remove dead code" for
    bluetooth-next

---
Hi Greg,

This series fixes the underlying design problem that leads to driver crashes
during hangup (eg., Andi Kleen's report https://lkml.org/lkml/2015/11/9/786).

Quoting from patch 17/19:

    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.

With this patch series, the tty core now guarantees no further driver/ldisc
interactions after hangup.

Patch 1-4 remove direct tty->ldisc access outside the tty core.
Patch 5 removes the defunct chars_in_buffer() ldisc method (which has been
        deprecated since 3.12)
Patch 6 & 7 fix unsafe ldisc uses which coincidentally have been discovered
        to cause crashes (https://lkml.org/lkml/2015/11/26/173 and
	https://lkml.org/lkml/2015/11/26/253). These have been tagged for
	-stable.
Patch 8-16 are preparations; documenting existing functions and refactoring.
        Patch 12 adds handling for the possibility of NULL ldisc references
	after tty_ldisc_ref_wait(); that commit log details the logic of
	why/how that works.
Patch 17 implements the fix: the ldisc instance is killed and left dead.
        At tty_reopen() if the tty->ldisc is NULL, a new ldisc is instanced.
Patch 18-19 are minor add-ons.

Regards,

Peter Hurley (19):
  staging: digi: Replace open-coded tty_wakeup()
  serial: 68328: Remove bogus ldisc reset
  bluetooth: hci_ldisc: Remove dead code
  NFC: nci: Remove dead code
  tty: Remove chars_in_buffer() line discipline method
  tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  n_tty: Fix unsafe reference to "other" ldisc
  tty: Reset c_line from driver's init_termios
  staging/speakup: Use tty_ldisc_ref() for paste kworker
  tty: Fix comments for tty_ldisc_get()
  tty: Fix comments for tty_ldisc_release()
  tty: Prepare for destroying line discipline on hangup
  tty: Handle NULL tty->ldisc
  tty: Move tty_ldisc_kill()
  tty: Use 'disc' for line discipline index name
  tty: Refactor tty_ldisc_reinit() for reuse
  tty: Destroy ldisc instance on hangup
  tty: Document c_line == N_TTY initial condition
  tty: Avoid unnecessary temporaries for tty->ldisc

 Documentation/serial/tty.txt        |   3 -
 drivers/bluetooth/hci_ldisc.c       |   8 +-
 drivers/staging/dgap/dgap.c         |  28 ++----
 drivers/staging/dgnc/dgnc_tty.c     |  18 +---
 drivers/staging/speakup/selection.c |   4 +-
 drivers/tty/amiserial.c             |   6 +-
 drivers/tty/cyclades.c              |   8 +-
 drivers/tty/n_gsm.c                 |  16 ----
 drivers/tty/n_tty.c                 |  30 +------
 drivers/tty/rocket.c                |   6 +-
 drivers/tty/serial/68328serial.c    |  12 +--
 drivers/tty/serial/crisv10.c        |  12 ++-
 drivers/tty/tty_io.c                |  64 +++++++++++---
 drivers/tty/tty_ldisc.c             | 171 ++++++++++++++++++++----------------
 drivers/tty/vt/selection.c          |   2 +
 include/linux/tty.h                 |   5 +-
 include/linux/tty_ldisc.h           |   7 --
 net/nfc/nci/uart.c                  |   9 +-
 18 files changed, 179 insertions(+), 230 deletions(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH v2 01/19] staging: digi: Replace open-coded tty_wakeup()
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
@ 2016-01-10  4:40   ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
                     ` (18 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The open-coded tty_wakeup()s are attempts to workaround fixed bugs
in the line discipline write_wakeup() method. Replace with tty_wakeup().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/dgap/dgap.c     | 28 ++++++----------------------
 drivers/staging/dgnc/dgnc_tty.c | 18 ++----------------
 2 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index bad3551..dfbae21 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1665,9 +1665,7 @@ static void dgap_input(struct channel_t *ch)
 }
 
 static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
-			      struct un_t *un, u32 mask,
-			      unsigned long *irq_flags1,
-			      unsigned long *irq_flags2)
+			      struct un_t *un, u32 mask)
 {
 	if (!(un->un_flags & mask))
 		return;
@@ -1677,17 +1675,7 @@ static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
 	if (!(un->un_flags & UN_ISOPEN))
 		return;
 
-	if ((un->un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-	    un->un_tty->ldisc->ops->write_wakeup) {
-		spin_unlock_irqrestore(&ch->ch_lock, *irq_flags2);
-		spin_unlock_irqrestore(&bd->bd_lock, *irq_flags1);
-
-		(un->un_tty->ldisc->ops->write_wakeup)(un->un_tty);
-
-		spin_lock_irqsave(&bd->bd_lock, *irq_flags1);
-		spin_lock_irqsave(&ch->ch_lock, *irq_flags2);
-	}
-	wake_up_interruptible(&un->un_tty->write_wait);
+	tty_wakeup(un->un_tty);
 	wake_up_interruptible(&un->un_flags_wait);
 }
 
@@ -1952,10 +1940,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit low.
 		 */
 		if (reason & IFTLW) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW);
 			if (ch->ch_flags & CH_WLOW) {
 				ch->ch_flags &= ~CH_WLOW;
 				wake_up_interruptible(&ch->ch_flags_wait);
@@ -1966,10 +1952,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit empty.
 		 */
 		if (reason & IFTEM) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY);
 			if (ch->ch_flags & CH_WEMPTY) {
 				ch->ch_flags &= ~CH_WEMPTY;
 				wake_up_interruptible(&ch->ch_flags_wait);
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 48e4b90..c152048 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -933,14 +933,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_tun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_tun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_tun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_tun.un_tty->ldisc->ops->write_wakeup(ch->ch_tun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_tun.un_tty->write_wait);
+		tty_wakeup(ch->ch_tun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
@@ -975,14 +968,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_pun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_pun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_pun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_pun.un_tty->ldisc->ops->write_wakeup(ch->ch_pun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_pun.un_tty->write_wait);
+		tty_wakeup(ch->ch_pun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 02/19] serial: 68328: Remove bogus ldisc reset
  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   ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
                     ` (17 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

As the #warning indicates, the open-coded ldisc reset was always not ok.
Not only is this code long dead, but now it would have no effect as
the ldisc is destroyed when this driver's close() method returns; remove.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/68328serial.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
index 0982c1a..90639b5 100644
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -1054,17 +1054,7 @@ static void rs_close(struct tty_struct *tty, struct file *filp)
 	tty_ldisc_flush(tty);
 	tty->closing = 0;
 	tty_port_tty_set(&info->tport, NULL);
-#warning "This is not and has never been valid so fix it"	
-#if 0
-	if (tty->ldisc.num != ldiscs[N_TTY].num) {
-		if (tty->ldisc.close)
-			(tty->ldisc.close)(tty);
-		tty->ldisc = ldiscs[N_TTY];
-		tty->termios.c_line = N_TTY;
-		if (tty->ldisc.open)
-			(tty->ldisc.open)(tty);
-	}
-#endif	
+
 	if (port->blocked_open) {
 		if (port->close_delay)
 			msleep_interruptible(jiffies_to_msecs(port->close_delay));
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 03/19] bluetooth: hci_ldisc: Remove dead code
  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   ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 04/19] NFC: nci: " Peter Hurley
                     ` (16 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The N_HCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/bluetooth/hci_ldisc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 96bcec5..c303f87 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -461,13 +461,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	INIT_WORK(&hu->init_ready, hci_uart_init_work);
 	INIT_WORK(&hu->write_work, hci_uart_write_work);
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	   open path is before the ldisc is referencable */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 04/19] NFC: nci: Remove dead code
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (2 preceding siblings ...)
  2016-01-10  4:40   ` [PATCH v2 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
@ 2016-01-10  4:40   ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
                     ` (15 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The N_NCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/nfc/nci/uart.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index 21d8875..c468eab 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -171,14 +171,7 @@ static int nci_uart_tty_open(struct tty_struct *tty)
 	tty->disc_data = NULL;
 	tty->receive_room = 65536;
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	 * open path is before the ldisc is referencable.
-	 */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 05/19] tty: Remove chars_in_buffer() line discipline method
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (3 preceding siblings ...)
  2016-01-10  4:40   ` [PATCH v2 04/19] NFC: nci: " Peter Hurley
@ 2016-01-10  4:40   ` Peter Hurley
  2016-01-10  4:40   ` [PATCH v2 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
                     ` (14 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The chars_in_buffer() line discipline method serves no functional
purpose, other than as a (dubious) debugging aid for mostly bit-rotting
drivers. Despite being documented as an optional method, every caller
is unconditionally executed (although conditionally compiled).
Furthermore, direct tty->ldisc access without an ldisc ref is unsafe.
Lastly, N_TTY's chars_in_buffer() has warned of removal since 3.12.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 Documentation/serial/tty.txt |  3 ---
 drivers/tty/amiserial.c      |  6 ++----
 drivers/tty/cyclades.c       |  8 ++++----
 drivers/tty/n_gsm.c          | 16 ----------------
 drivers/tty/n_tty.c          | 23 -----------------------
 drivers/tty/rocket.c         |  6 ++----
 drivers/tty/serial/crisv10.c | 12 +++++-------
 include/linux/tty_ldisc.h    |  7 -------
 8 files changed, 13 insertions(+), 68 deletions(-)

diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
index bc3842d..798cba8 100644
--- a/Documentation/serial/tty.txt
+++ b/Documentation/serial/tty.txt
@@ -72,9 +72,6 @@ flush_buffer()	-	(optional) May be called at any point between
 			open and close, and instructs the line discipline
 			to empty its input buffer.
 
-chars_in_buffer() -	(optional) Report the number of bytes in the input
-			buffer.
-
 set_termios()	-	(optional) Called on termios structure changes.
 			The caller passes the old termios data and the
 			current data is in the tty. Called under the
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 2caaf5a..6ba5681 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -965,8 +965,7 @@ static void rs_throttle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_throttle"))
@@ -988,8 +987,7 @@ static void rs_unthrottle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_unthrottle"))
diff --git a/drivers/tty/cyclades.c b/drivers/tty/cyclades.c
index abbed20..a48e7e6 100644
--- a/drivers/tty/cyclades.c
+++ b/drivers/tty/cyclades.c
@@ -2852,8 +2852,8 @@ static void cy_throttle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:throttle %s: %ld...ttyC%d\n", tty_name(tty),
-			tty->ldisc.chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:throttle %s ...ttyC%d\n", tty_name(tty),
+			 info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_throttle"))
@@ -2891,8 +2891,8 @@ static void cy_unthrottle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:unthrottle %s: %ld...ttyC%d\n",
-		tty_name(tty), tty_chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:unthrottle %s ...ttyC%d\n",
+		tty_name(tty), info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_unthrottle"))
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c3fe026..e3cc277 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2304,21 +2304,6 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 }
 
 /**
- *	gsmld_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: gsm lock
- */
-
-static ssize_t gsmld_chars_in_buffer(struct tty_struct *tty)
-{
-	return 0;
-}
-
-/**
  *	gsmld_flush_buffer	-	clean input queue
  *	@tty:	terminal device
  *
@@ -2830,7 +2815,6 @@ static struct tty_ldisc_ops tty_ldisc_packet = {
 	.open            = gsmld_open,
 	.close           = gsmld_close,
 	.flush_buffer    = gsmld_flush_buffer,
-	.chars_in_buffer = gsmld_chars_in_buffer,
 	.read            = gsmld_read,
 	.write           = gsmld_write,
 	.ioctl           = gsmld_ioctl,
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index d9a5fc2..e09fd58 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -384,28 +384,6 @@ static void n_tty_flush_buffer(struct tty_struct *tty)
 }
 
 /**
- *	n_tty_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: exclusive termios_rwsem
- */
-
-static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
-{
-	ssize_t n;
-
-	WARN_ONCE(1, "%s is deprecated and scheduled for removal.", __func__);
-
-	down_write(&tty->termios_rwsem);
-	n = chars_in_buffer(tty);
-	up_write(&tty->termios_rwsem);
-	return n;
-}
-
-/**
  *	is_utf8_continuation	-	utf8 multibyte check
  *	@c: byte to check
  *
@@ -2528,7 +2506,6 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.open            = n_tty_open,
 	.close           = n_tty_close,
 	.flush_buffer    = n_tty_flush_buffer,
-	.chars_in_buffer = n_tty_chars_in_buffer,
 	.read            = n_tty_read,
 	.write           = n_tty_write,
 	.ioctl           = n_tty_ioctl,
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index 802eac7..f624b93 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -1360,8 +1360,7 @@ static void rp_throttle(struct tty_struct *tty)
 	struct r_port *info = tty->driver_data;
 
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "throttle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "throttle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_throttle"))
@@ -1377,8 +1376,7 @@ static void rp_unthrottle(struct tty_struct *tty)
 {
 	struct r_port *info = tty->driver_data;
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "unthrottle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "unthrottle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_unthrottle"))
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index f13f2eb..e98aef7 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -2968,7 +2968,7 @@ static int rs_raw_write(struct tty_struct *tty,
 
 	local_save_flags(flags);
 	DFLOW(DEBUG_LOG(info->line, "write count %i ", count));
-	DFLOW(DEBUG_LOG(info->line, "ldisc %i\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line, "ldisc\n"));
 
 
 	/* The local_irq_disable/restore_flags pairs below are needed
@@ -3161,10 +3161,9 @@ rs_throttle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_throttle %lu\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_throttle\n"));
 
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
@@ -3181,10 +3180,9 @@ rs_unthrottle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc %d\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc\n"));
 	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle flip.count: %i\n", tty->flip.count));
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 00c9d68..6101ab8 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -25,12 +25,6 @@
  *	buffers of any input characters it may have queued to be
  *	delivered to the user mode process.
  *
- * ssize_t (*chars_in_buffer)(struct tty_struct *tty);
- *
- *	This function returns the number of input characters the line
- *	discipline may have queued up to be delivered to the user mode
- *	process.
- *
  * ssize_t (*read)(struct tty_struct * tty, struct file * file,
  *		   unsigned char * buf, size_t nr);
  *
@@ -188,7 +182,6 @@ struct tty_ldisc_ops {
 	int	(*open)(struct tty_struct *);
 	void	(*close)(struct tty_struct *);
 	void	(*flush_buffer)(struct tty_struct *tty);
-	ssize_t	(*chars_in_buffer)(struct tty_struct *tty);
 	ssize_t	(*read)(struct tty_struct *tty, struct file *file,
 			unsigned char __user *buf, size_t nr);
 	ssize_t	(*write)(struct tty_struct *tty, struct file *file,
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (4 preceding siblings ...)
  2016-01-10  4:40   ` [PATCH v2 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
@ 2016-01-10  4:40   ` 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
                     ` (13 subsequent siblings)
  19 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

ioctl(TIOCGETD) retrieves the line discipline id directly from the
ldisc because the line discipline id (c_line) in termios is untrustworthy;
userspace may have set termios via ioctl(TCSETS*) without actually
changing the line discipline via ioctl(TIOCSETD).

However, directly accessing the current ldisc via tty->ldisc is
unsafe; the ldisc ptr dereferenced may be stale if the line discipline
is changing via ioctl(TIOCSETD) or hangup.

Wait for the line discipline reference (just like read() or write())
to retrieve the "current" line discipline id.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 892c923..56d3a6b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2649,6 +2649,28 @@ static int tiocsetd(struct tty_struct *tty, int __user *p)
 }
 
 /**
+ *	tiocgetd	-	get line discipline
+ *	@tty: tty device
+ *	@p: pointer to user data
+ *
+ *	Retrieves the line discipline id directly from the ldisc.
+ *
+ *	Locking: waits for ldisc reference (in case the line discipline
+ *		is changing or the tty is being hungup)
+ */
+
+static int tiocgetd(struct tty_struct *tty, int __user *p)
+{
+	struct tty_ldisc *ld;
+	int ret;
+
+	ld = tty_ldisc_ref_wait(tty);
+	ret = put_user(ld->ops->num, p);
+	tty_ldisc_deref(ld);
+	return ret;
+}
+
+/**
  *	send_break	-	performed time break
  *	@tty: device to break on
  *	@duration: timeout in mS
@@ -2874,7 +2896,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case TIOCGSID:
 		return tiocgsid(tty, real_tty, p);
 	case TIOCGETD:
-		return put_user(tty->ldisc->ops->num, (int __user *)p);
+		return tiocgetd(tty, p);
 	case TIOCSETD:
 		return tiocsetd(tty, p);
 	case TIOCVHANGUP:
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 07/19] n_tty: Fix unsafe reference to "other" ldisc
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (5 preceding siblings ...)
  2016-01-10  4:40   ` [PATCH v2 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
@ 2016-01-10  4:40   ` 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
                     ` (12 subsequent siblings)
  19 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

Although n_tty_check_unthrottle() has a valid ldisc reference (since
the tty core gets the ldisc ref in tty_read() before calling the line
discipline read() method), it does not have a valid ldisc reference to
the "other" pty of a pty pair. Since getting an ldisc reference for
tty->link essentially open-codes tty_wakeup(), just replace with the
equivalent tty_wakeup().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e09fd58..90eca26 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -269,16 +269,13 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 
 static void n_tty_check_unthrottle(struct tty_struct *tty)
 {
-	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
-	    tty->link->ldisc->ops->write_wakeup == n_tty_write_wakeup) {
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
 		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
 			return;
 		if (!tty->count)
 			return;
 		n_tty_kick_worker(tty);
-		n_tty_write_wakeup(tty->link);
-		if (waitqueue_active(&tty->link->write_wait))
-			wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
+		tty_wakeup(tty->link);
 		return;
 	}
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 08/19] tty: Reset c_line from driver's init_termios
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (6 preceding siblings ...)
  2016-01-10  4:40   ` [PATCH v2 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
@ 2016-01-10  4:40   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
                     ` (11 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

After the ldisc is released, but before the tty is destroyed, the termios
is saved (in tty_free_termios()); this termios is restored if a new
tty is created on next open(). However, the line discipline is always
reset, which is not obvious in the current method. Instead, reset
as part of the restore.

Restore the original line discipline, which may not have been N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    | 5 +++--
 drivers/tty/tty_ldisc.c | 3 ---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 56d3a6b..0871d1e 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1388,9 +1388,10 @@ int tty_init_termios(struct tty_struct *tty)
 	else {
 		/* Check for lazy saved data */
 		tp = tty->driver->termios[idx];
-		if (tp != NULL)
+		if (tp != NULL) {
 			tty->termios = *tp;
-		else
+			tty->termios.c_line  = tty->driver->init_termios.c_line;
+		} else
 			tty->termios = tty->driver->init_termios;
 	}
 	/* Compatibility until drivers always set this */
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index a054d03..d5104b0 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -745,9 +745,6 @@ static void tty_ldisc_kill(struct tty_struct *tty)
 	tty_ldisc_put(tty->ldisc);
 	/* Force an oops if we mess this up */
 	tty->ldisc = NULL;
-
-	/* Ensure the next open requests the N_TTY ldisc */
-	tty_set_termios_ldisc(tty, N_TTY);
 }
 
 /**
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (7 preceding siblings ...)
  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   ` Peter Hurley
  2016-01-10 23:16     ` Ben Hutchings
  2016-01-10  4:41   ` [PATCH v2 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
                     ` (10 subsequent siblings)
  19 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley, stable

As the function documentation for tty_ldisc_ref_wait() notes, it is
only callable from a tty file_operations routine; otherwise there
is no guarantee the ref won't be NULL.

The key difference with the VT's paste_selection() is that is an ioctl,
where __speakup_paste_selection() is completely asynch kworker, kicked
off from interrupt context.

Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
       tty (ab)usage to match vt")
Cc: <stable@vger.kernel.org>

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/speakup/selection.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
index aa5ab6c..86c0b9a 100644
--- a/drivers/staging/speakup/selection.c
+++ b/drivers/staging/speakup/selection.c
@@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
 	struct tty_ldisc *ld;
 	DECLARE_WAITQUEUE(wait, current);
 
-	ld = tty_ldisc_ref_wait(tty);
+	ld = tty_ldisc_ref(tty);
+	if (!ld)
+		return;
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 10/19] tty: Fix comments for tty_ldisc_get()
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (8 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
                     ` (9 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_get() returns ERR_PTR() values if unsuccessful, not NULL;
fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index d5104b0..b3cead9 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -140,9 +140,16 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
  *	@disc: ldisc number
  *
  *	Takes a reference to a line discipline. Deals with refcounts and
- *	module locking counts. Returns NULL if the discipline is not available.
- *	Returns a pointer to the discipline and bumps the ref count if it is
- *	available
+ *	module locking counts.
+ *
+ *	Returns: -EINVAL if the discipline index is not [N_TTY..NR_LDISCS] or
+ *			 if the discipline is not registered
+ *		 -EAGAIN if request_module() failed to load or register the
+ *			 the discipline
+ *		 -ENOMEM if allocation failure
+ *
+ *		 Otherwise, returns a pointer to the discipline and bumps the
+ *		 ref count
  *
  *	Locking:
  *		takes tty_ldiscs_lock to guard against ldisc races
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 11/19] tty: Fix comments for tty_ldisc_release()
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (9 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
                     ` (8 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_kill() sets tty->ldisc to NULL; _not_ to N_TTY with a valid
but unopened ldisc. Fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index b3cead9..8582aeb 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -759,8 +759,7 @@ static void tty_ldisc_kill(struct tty_struct *tty)
  *	@tty: tty being shut down (or one end of pty pair)
  *
  *	Called during the final close of a tty or a pty pair in order to shut
- *	down the line discpline layer. On exit, each ldisc assigned is N_TTY and
- *	each ldisc has not been opened.
+ *	down the line discpline layer. On exit, each tty's ldisc is NULL.
  */
 
 void tty_ldisc_release(struct tty_struct *tty)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 12/19] tty: Prepare for destroying line discipline on hangup
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (10 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 13/19] tty: Handle NULL tty->ldisc Peter Hurley
                     ` (7 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty file_operations (read/write/ioctl) wait for the ldisc reference
indefinitely (until ldisc lifetime events, such as hangup or TIOCSETD,
finish). Since hangup now destroys the ldisc and does not instance
another copy, file_operations must now be prepared to receive a NULL
ldisc reference from tty_ldisc_ref_wait():

CPU 0                                   CPU 1
-----                                   -----
(*f_op->read)() => tty_read()
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()
ld = tty_ldisc_ref_wait()
/* ld == NULL */

Instead, the action taken now is to return the same value as if the
tty had been hungup a moment earlier:

CPU 0                                   CPU 1
-----                                   -----
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
(*f_op->read)() => hung_up_tty_read()
return 0;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c       | 14 ++++++++++++++
 drivers/tty/tty_ldisc.c    |  4 ++--
 drivers/tty/vt/selection.c |  2 ++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0871d1e..89822c4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1069,6 +1069,8 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
 	/* We want to wait for the line discipline to sort out in this
 	   situation */
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_read(file, buf, count, ppos);
 	if (ld->ops->read)
 		i = ld->ops->read(tty, file, buf, count);
 	else
@@ -1243,6 +1245,8 @@ static ssize_t tty_write(struct file *file, const char __user *buf,
 	if (tty->ops->write_room == NULL)
 		tty_err(tty, "missing write_room method\n");
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_write(file, buf, count, ppos);
 	if (!ld->ops->write)
 		ret = -EIO;
 	else
@@ -2185,6 +2189,8 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 		return 0;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_poll(filp, wait);
 	if (ld->ops->poll)
 		ret = ld->ops->poll(tty, filp, wait);
 	tty_ldisc_deref(ld);
@@ -2274,6 +2280,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 		return -EFAULT;
 	tty_audit_tiocsti(tty, ch);
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ld->ops->receive_buf(tty, &ch, &mbz, 1);
 	tty_ldisc_deref(ld);
 	return 0;
@@ -2666,6 +2674,8 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
 	int ret;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ret = put_user(ld->ops->num, p);
 	tty_ldisc_deref(ld);
 	return ret;
@@ -2963,6 +2973,8 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			return retval;
 	}
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_ioctl(file, cmd, arg);
 	retval = -EINVAL;
 	if (ld->ops->ioctl) {
 		retval = ld->ops->ioctl(tty, file, cmd, arg);
@@ -2991,6 +3003,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 	}
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_compat_ioctl(file, cmd, arg);
 	if (ld->ops->compat_ioctl)
 		retval = ld->ops->compat_ioctl(tty, file, cmd, arg);
 	else
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 8582aeb..1c9ce25 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -262,8 +262,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
  *	against a discipline change, such as an existing ldisc reference
  *	(which we check for)
  *
- *	Note: only callable from a file_operations routine (which
- *	guarantees tty->ldisc != NULL when the lock is acquired).
+ *	Note: a file_operations routine (read/poll/write) should use this
+ *	function to wait for any ldisc lifetime events to finish.
  */
 
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 381a2b1..4dd9dd2 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -347,6 +347,8 @@ int paste_selection(struct tty_struct *tty)
 	console_unlock();
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;	/* ldisc was hung up */
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 13/19] tty: Handle NULL tty->ldisc
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (11 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 14/19] tty: Move tty_ldisc_kill() Peter Hurley
                     ` (6 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

In preparation of destroying line discipline on hangup, fix
ldisc core operations to properly handle when the tty's ldisc is
NULL.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 1c9ce25..5115230 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -269,7 +269,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
 {
 	ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
-	WARN_ON(!tty->ldisc);
+	if (!tty->ldisc)
+		ldsem_up_read(&tty->ldisc_sem);
 	return tty->ldisc;
 }
 EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait);
@@ -462,7 +463,7 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
 		if (ret)
 			clear_bit(TTY_LDISC_OPEN, &tty->flags);
 
-		tty_ldisc_debug(tty, "%p: opened\n", tty->ldisc);
+		tty_ldisc_debug(tty, "%p: opened\n", ld);
 		return ret;
 	}
 	return 0;
@@ -483,7 +484,7 @@ static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld)
 	clear_bit(TTY_LDISC_OPEN, &tty->flags);
 	if (ld->ops->close)
 		ld->ops->close(tty);
-	tty_ldisc_debug(tty, "%p: closed\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: closed\n", ld);
 }
 
 /**
@@ -546,6 +547,11 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	if (retval)
 		goto err;
 
+	if (!tty->ldisc) {
+		retval = -EIO;
+		goto out;
+	}
+
 	/* Check the no-op case */
 	if (tty->ldisc->ops->num == ldisc)
 		goto out;
@@ -661,7 +667,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 	int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS;
 	int err = 0;
 
-	tty_ldisc_debug(tty, "%p: closing\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: hangup\n", tty->ldisc);
 
 	ld = tty_ldisc_ref(tty);
 	if (ld != NULL) {
@@ -745,6 +751,8 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 
 static void tty_ldisc_kill(struct tty_struct *tty)
 {
+	if (!tty->ldisc)
+		return;
 	/*
 	 * Now kill off the ldisc
 	 */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 14/19] tty: Move tty_ldisc_kill()
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (12 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 13/19] tty: Handle NULL tty->ldisc Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
                     ` (5 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

In preparation for destroying the line discipline instance on hangup,
move tty_ldisc_kill() to eliminate needless forward declarations.
No functional change.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 5115230..d91122d 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -603,6 +603,25 @@ err:
 }
 
 /**
+ *	tty_ldisc_kill	-	teardown ldisc
+ *	@tty: tty being released
+ *
+ *	Perform final close of the ldisc and reset tty->ldisc
+ */
+static void tty_ldisc_kill(struct tty_struct *tty)
+{
+	if (!tty->ldisc)
+		return;
+	/*
+	 * Now kill off the ldisc
+	 */
+	tty_ldisc_close(tty, tty->ldisc);
+	tty_ldisc_put(tty->ldisc);
+	/* Force an oops if we mess this up */
+	tty->ldisc = NULL;
+}
+
+/**
  *	tty_reset_termios	-	reset terminal state
  *	@tty: tty to reset
  *
@@ -749,19 +768,6 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 	return 0;
 }
 
-static void tty_ldisc_kill(struct tty_struct *tty)
-{
-	if (!tty->ldisc)
-		return;
-	/*
-	 * Now kill off the ldisc
-	 */
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/* Force an oops if we mess this up */
-	tty->ldisc = NULL;
-}
-
 /**
  *	tty_ldisc_release		-	release line discipline
  *	@tty: tty being shut down (or one end of pty pair)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 15/19] tty: Use 'disc' for line discipline index name
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (13 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 14/19] tty: Move tty_ldisc_kill() Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
                     ` (4 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty->ldisc is a ptr to struct tty_ldisc, but unfortunately 'ldisc' is
also used as a parameter or local name to refer to the line discipline
index value (ie, N_TTY, N_GSM, etc.); instead prefer the name used
by the line discipline registration/ref counting functions.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    |  6 +++---
 drivers/tty/tty_ldisc.c | 22 +++++++++++-----------
 include/linux/tty.h     |  2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 89822c4..f6f559c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2646,13 +2646,13 @@ static int tiocgsid(struct tty_struct *tty, struct tty_struct *real_tty, pid_t _
 
 static int tiocsetd(struct tty_struct *tty, int __user *p)
 {
-	int ldisc;
+	int disc;
 	int ret;
 
-	if (get_user(ldisc, p))
+	if (get_user(disc, p))
 		return -EFAULT;
 
-	ret = tty_set_ldisc(tty, ldisc);
+	ret = tty_set_ldisc(tty, disc);
 
 	return ret;
 }
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index d91122d..6ba6fe1 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -419,7 +419,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
 /**
  *	tty_set_termios_ldisc		-	set ldisc field
  *	@tty: tty structure
- *	@num: line discipline number
+ *	@disc: line discipline number
  *
  *	This is probably overkill for real world processors but
  *	they are not on hot paths so a little discipline won't do
@@ -432,10 +432,10 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
  *	Locking: takes termios_rwsem
  */
 
-static void tty_set_termios_ldisc(struct tty_struct *tty, int num)
+static void tty_set_termios_ldisc(struct tty_struct *tty, int disc)
 {
 	down_write(&tty->termios_rwsem);
-	tty->termios.c_line = num;
+	tty->termios.c_line = disc;
 	up_write(&tty->termios_rwsem);
 
 	tty->disc_data = NULL;
@@ -533,12 +533,12 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
  *	the close of one side of a tty/pty pair, and eventually hangup.
  */
 
-int tty_set_ldisc(struct tty_struct *tty, int ldisc)
+int tty_set_ldisc(struct tty_struct *tty, int disc)
 {
 	int retval;
 	struct tty_ldisc *old_ldisc, *new_ldisc;
 
-	new_ldisc = tty_ldisc_get(tty, ldisc);
+	new_ldisc = tty_ldisc_get(tty, disc);
 	if (IS_ERR(new_ldisc))
 		return PTR_ERR(new_ldisc);
 
@@ -553,7 +553,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	}
 
 	/* Check the no-op case */
-	if (tty->ldisc->ops->num == ldisc)
+	if (tty->ldisc->ops->num == disc)
 		goto out;
 
 	if (test_bit(TTY_HUPPED, &tty->flags)) {
@@ -569,7 +569,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 
 	/* Now set up the new line discipline. */
 	tty->ldisc = new_ldisc;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	retval = tty_ldisc_open(tty, new_ldisc);
 	if (retval < 0) {
@@ -641,15 +641,15 @@ static void tty_reset_termios(struct tty_struct *tty)
 /**
  *	tty_ldisc_reinit	-	reinitialise the tty ldisc
  *	@tty: tty to reinit
- *	@ldisc: line discipline to reinitialize
+ *	@disc: line discipline to reinitialize
  *
  *	Switch the tty to a line discipline and leave the ldisc
  *	state closed
  */
 
-static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
+static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, ldisc);
+	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
 
 	if (IS_ERR(ld))
 		return -1;
@@ -660,7 +660,7 @@ static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
 	 *	Switch the line discipline back
 	 */
 	tty->ldisc = ld;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	return 0;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2fd8708..ee73220 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -575,7 +575,7 @@ static inline int tty_port_users(struct tty_port *port)
 
 extern int tty_register_ldisc(int disc, struct tty_ldisc_ops *new_ldisc);
 extern int tty_unregister_ldisc(int disc);
-extern int tty_set_ldisc(struct tty_struct *tty, int ldisc);
+extern int tty_set_ldisc(struct tty_struct *tty, int disc);
 extern int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty);
 extern void tty_ldisc_release(struct tty_struct *tty);
 extern void tty_ldisc_init(struct tty_struct *tty);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 16/19] tty: Refactor tty_ldisc_reinit() for reuse
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (14 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-10  4:41   ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
                     ` (3 subsequent siblings)
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

At tty hangup, the line discipline instance is reinitialized by
closing the current ldisc instance and opening a new instance.
This operation is complicated by error recovery: if the attempt
to reinit the current line discipline fails, the line discipline
is reset to N_TTY (which should not but can fail).

Re-purpose tty_ldisc_reinit() to return a valid, open line discipline
instance, or otherwise, an error.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 53 +++++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 6ba6fe1..527fc5b 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -643,26 +643,41 @@ static void tty_reset_termios(struct tty_struct *tty)
  *	@tty: tty to reinit
  *	@disc: line discipline to reinitialize
  *
- *	Switch the tty to a line discipline and leave the ldisc
- *	state closed
+ *	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.
+ *
+ *	Returns 0 if successful, otherwise error code < 0
  */
 
 static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
+	struct tty_ldisc *ld;
+	int retval;
 
-	if (IS_ERR(ld))
-		return -1;
+	ld = tty_ldisc_get(tty, disc);
+	if (IS_ERR(ld)) {
+		BUG_ON(disc == N_TTY);
+		return PTR_ERR(ld);
+	}
 
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/*
-	 *	Switch the line discipline back
-	 */
+	if (tty->ldisc) {
+		tty_ldisc_close(tty, tty->ldisc);
+		tty_ldisc_put(tty->ldisc);
+	}
+
+	/* switch the line discipline */
 	tty->ldisc = ld;
 	tty_set_termios_ldisc(tty, disc);
-
-	return 0;
+	retval = tty_ldisc_open(tty, tty->ldisc);
+	if (retval) {
+		if (!WARN_ON(disc == N_TTY)) {
+			tty_ldisc_put(tty->ldisc);
+			tty->ldisc = NULL;
+		}
+	}
+	return retval;
 }
 
 /**
@@ -718,19 +733,13 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 		   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) {
+		if (reset == 0)
+			err = tty_ldisc_reinit(tty, tty->termios.c_line);
 
-			if (!tty_ldisc_reinit(tty, tty->termios.c_line))
-				err = tty_ldisc_open(tty, tty->ldisc);
-			else
-				err = 1;
-		}
 		/* If the re-open fails or we reset then go to N_TTY. The
 		   N_TTY open cannot fail */
-		if (reset || err) {
-			BUG_ON(tty_ldisc_reinit(tty, N_TTY));
-			WARN_ON(tty_ldisc_open(tty, tty->ldisc));
-		}
+		if (reset || err < 0)
+			tty_ldisc_reinit(tty, N_TTY);
 	}
 	tty_ldisc_unlock(tty);
 	if (reset)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 17/19] tty: Destroy ldisc instance on hangup
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (15 preceding siblings ...)
  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
  2016-01-10  6:24     ` kbuild test robot
  2016-01-10  6:24     ` [PATCH] tty: fix badzero.cocci warnings kbuild test robot
  2016-01-10  4:41   ` [PATCH v2 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
                     ` (2 subsequent siblings)
  19 siblings, 2 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

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

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 18/19] tty: Document c_line == N_TTY initial condition
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (16 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
@ 2016-01-10  4:41   ` 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
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The line discipline id is stored in the tty's termios; document the
implicit initial value of N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index fea8318..eda8715 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -123,7 +123,8 @@ struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
 		   ECHOCTL | ECHOKE | IEXTEN,
 	.c_cc = INIT_C_CC,
 	.c_ispeed = 38400,
-	.c_ospeed = 38400
+	.c_ospeed = 38400,
+	/* .c_line = N_TTY, */
 };
 
 EXPORT_SYMBOL(tty_std_termios);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v2 19/19] tty: Avoid unnecessary temporaries for tty->ldisc
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (17 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
@ 2016-01-10  4:41   ` Peter Hurley
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
  19 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  4:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_setup() is race-free and can reference tty->ldisc without
snapshots.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 199e4b4..b404c20 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -754,17 +754,14 @@ void tty_ldisc_hangup(struct tty_struct *tty, bool reinit)
 
 int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 {
-	struct tty_ldisc *ld = tty->ldisc;
-	int retval;
-
-	retval = tty_ldisc_open(tty, ld);
+	int retval = tty_ldisc_open(tty, tty->ldisc);
 	if (retval)
 		return retval;
 
 	if (o_tty) {
 		retval = tty_ldisc_open(o_tty, o_tty->ldisc);
 		if (retval) {
-			tty_ldisc_close(tty, ld);
+			tty_ldisc_close(tty, tty->ldisc);
 			return retval;
 		}
 	}
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  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
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  5:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel

On 01/09/2016 08:40 PM, Peter Hurley wrote:
> ioctl(TIOCGETD) retrieves the line discipline id directly from the
> ldisc because the line discipline id (c_line) in termios is untrustworthy;
> userspace may have set termios via ioctl(TCSETS*) without actually
> changing the line discipline via ioctl(TIOCSETD).
> 
> However, directly accessing the current ldisc via tty->ldisc is
> unsafe; the ldisc ptr dereferenced may be stale if the line discipline
> is changing via ioctl(TIOCSETD) or hangup.
> 
> Wait for the line discipline reference (just like read() or write())
> to retrieve the "current" line discipline id.

Hi Greg,

I forgot to re-mark this patch for stable.

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 07/19] n_tty: Fix unsafe reference to "other" ldisc
  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
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  5:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel

On 01/09/2016 08:40 PM, Peter Hurley wrote:
> Although n_tty_check_unthrottle() has a valid ldisc reference (since
> the tty core gets the ldisc ref in tty_read() before calling the line
> discipline read() method), it does not have a valid ldisc reference to
> the "other" pty of a pty pair. Since getting an ldisc reference for
> tty->link essentially open-codes tty_wakeup(), just replace with the
> equivalent tty_wakeup().

Hi Greg,

Same here (forgot to re-mark this patch for stable).

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 17/19] tty: Destroy ldisc instance on hangup
  2016-01-10  4:41   ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
@ 2016-01-10  6:24     ` kbuild test robot
  2016-01-10  6:24     ` [PATCH] tty: fix badzero.cocci warnings kbuild test robot
  1 sibling, 0 replies; 71+ messages in thread
From: kbuild test robot @ 2016-01-10  6:24 UTC (permalink / raw)
  To: Peter Hurley
  Cc: kbuild-all, Greg Kroah-Hartman, Jiri Slaby, linux-kernel, Peter Hurley

Hi Peter,

[auto build test WARNING on tty/tty-testing]
[cannot apply to v4.4-rc8 next-20160108]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Hurley/Fix-driver-crashes-on-hangup/20160110-124954
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing


coccinelle warnings: (new ones prefixed by >>)

>> drivers/tty/tty_io.c:731:36-37: WARNING comparing pointer to 0

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH] tty: fix badzero.cocci warnings
  2016-01-10  4:41   ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
  2016-01-10  6:24     ` kbuild test robot
@ 2016-01-10  6:24     ` kbuild test robot
  2016-01-10  7:02       ` Peter Hurley
  1 sibling, 1 reply; 71+ messages in thread
From: kbuild test robot @ 2016-01-10  6:24 UTC (permalink / raw)
  To: Peter Hurley
  Cc: kbuild-all, Greg Kroah-Hartman, Jiri Slaby, linux-kernel, Peter Hurley

drivers/tty/tty_io.c:731:36-37: WARNING comparing pointer to 0

 Compare pointer-typed values to NULL rather than 0

Semantic patch information:
 This makes an effort to choose between !x and x == NULL.  !x is used
 if it has previously been used with the function used to initialize x.
 This relies on type information.  More type information can be obtained
 using the option -all_includes and the option -I to specify an
 include path.

Generated by: scripts/coccinelle/null/badzero.cocci

CC: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 tty_io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -728,7 +728,7 @@ static void __tty_hangup(struct tty_stru
 	while (refs--)
 		tty_kref_put(tty);
 
-	tty_ldisc_hangup(tty, cons_filp != 0);
+	tty_ldisc_hangup(tty, cons_filp != NULL);
 
 	spin_lock_irq(&tty->ctrl_lock);
 	clear_bit(TTY_THROTTLED, &tty->flags);

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH] tty: fix badzero.cocci warnings
  2016-01-10  6:24     ` [PATCH] tty: fix badzero.cocci warnings kbuild test robot
@ 2016-01-10  7:02       ` Peter Hurley
  0 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-10  7:02 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Greg Kroah-Hartman, Jiri Slaby, linux-kernel

On 01/09/2016 10:24 PM, kbuild test robot wrote:
> drivers/tty/tty_io.c:731:36-37: WARNING comparing pointer to 0
> 
>  Compare pointer-typed values to NULL rather than 0
> 
> Semantic patch information:
>  This makes an effort to choose between !x and x == NULL.  !x is used
>  if it has previously been used with the function used to initialize x.
>  This relies on type information.  More type information can be obtained
>  using the option -all_includes and the option -I to specify an
>  include path.
> 
> Generated by: scripts/coccinelle/null/badzero.cocci

Not sure what possessed me to write 'cons_filp != 0'. Not my style, for sure.
Thanks.

Regards,
Peter Hurley

> CC: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
> 
>  tty_io.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -728,7 +728,7 @@ static void __tty_hangup(struct tty_stru
>  	while (refs--)
>  		tty_kref_put(tty);
>  
> -	tty_ldisc_hangup(tty, cons_filp != 0);
> +	tty_ldisc_hangup(tty, cons_filp != NULL);
>  
>  	spin_lock_irq(&tty->ctrl_lock);
>  	clear_bit(TTY_THROTTLED, &tty->flags);
> 

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  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
  0 siblings, 1 reply; 71+ messages in thread
From: Ben Hutchings @ 2016-01-10 23:16 UTC (permalink / raw)
  To: Peter Hurley, Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, stable

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

On Sat, 2016-01-09 at 20:41 -0800, Peter Hurley wrote:
> As the function documentation for tty_ldisc_ref_wait() notes, it is
> only callable from a tty file_operations routine; otherwise there
> is no guarantee the ref won't be NULL.
> 
> The key difference with the VT's paste_selection() is that is an ioctl,
> where __speakup_paste_selection() is completely asynch kworker, kicked
> off from interrupt context.
> 
> Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
>        tty (ab)usage to match vt")
> Cc: <stable@vger.kernel.org>
> 
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/staging/speakup/selection.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
> index aa5ab6c..86c0b9a 100644
> --- a/drivers/staging/speakup/selection.c
> +++ b/drivers/staging/speakup/selection.c
> @@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
>  	struct tty_ldisc *ld;
>  	DECLARE_WAITQUEUE(wait, current);
>  
> -	ld = tty_ldisc_ref_wait(tty);
> +	ld = tty_ldisc_ref(tty);
> +	if (!ld)
> +		return;
>  	tty_buffer_lock_exclusive(&vc->port);
>  
>  	add_wait_queue(&vc->paste_wait, &wait);

This leaks a reference to the tty.  Instead of returning directly, I
think you need to add a label and goto the tty_kref_put() at the bottom
of the function.

Ben.

-- 
Ben Hutchings
Power corrupts.  Absolute power is kind of neat.
                           - John Lehman, Secretary of the US Navy 1981-1987

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2016-01-10 23:16     ` Ben Hutchings
@ 2016-01-11  0:25       ` Peter Hurley
  2016-01-11  5:40         ` Peter Hurley
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  0:25 UTC (permalink / raw)
  To: Ben Hutchings, Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, stable

On 01/10/2016 03:16 PM, Ben Hutchings wrote:
> On Sat, 2016-01-09 at 20:41 -0800, Peter Hurley wrote:
>> As the function documentation for tty_ldisc_ref_wait() notes, it is
>> only callable from a tty file_operations routine; otherwise there
>> is no guarantee the ref won't be NULL.
>>
>> The key difference with the VT's paste_selection() is that is an ioctl,
>> where __speakup_paste_selection() is completely asynch kworker, kicked
>> off from interrupt context.
>>
>> Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
>>        tty (ab)usage to match vt")
>> Cc: <stable@vger.kernel.org>
>>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>>  drivers/staging/speakup/selection.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
>> index aa5ab6c..86c0b9a 100644
>> --- a/drivers/staging/speakup/selection.c
>> +++ b/drivers/staging/speakup/selection.c
>> @@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
>>  	struct tty_ldisc *ld;
>>  	DECLARE_WAITQUEUE(wait, current);
>>  
>> -	ld = tty_ldisc_ref_wait(tty);
>> +	ld = tty_ldisc_ref(tty);
>> +	if (!ld)
>> +		return;
>>  	tty_buffer_lock_exclusive(&vc->port);
>>  
>>  	add_wait_queue(&vc->paste_wait, &wait);
> 
> This leaks a reference to the tty.  Instead of returning directly, I
> think you need to add a label and goto the tty_kref_put() at the bottom
> of the function.

Ugh, speakup_paste_selection() is a worse hack than I thought it was.

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2016-01-11  0:25       ` Peter Hurley
@ 2016-01-11  5:40         ` Peter Hurley
  2016-01-11 10:37           ` Ben Hutchings
  0 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  5:40 UTC (permalink / raw)
  To: Ben Hutchings, Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, stable

On 01/10/2016 04:25 PM, Peter Hurley wrote:
> On 01/10/2016 03:16 PM, Ben Hutchings wrote:
>> On Sat, 2016-01-09 at 20:41 -0800, Peter Hurley wrote:
>>> As the function documentation for tty_ldisc_ref_wait() notes, it is
>>> only callable from a tty file_operations routine; otherwise there
>>> is no guarantee the ref won't be NULL.
>>>
>>> The key difference with the VT's paste_selection() is that is an ioctl,
>>> where __speakup_paste_selection() is completely asynch kworker, kicked
>>> off from interrupt context.
>>>
>>> Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
>>>        tty (ab)usage to match vt")
>>> Cc: <stable@vger.kernel.org>
>>>
>>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>>> ---
>>>  drivers/staging/speakup/selection.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
>>> index aa5ab6c..86c0b9a 100644
>>> --- a/drivers/staging/speakup/selection.c
>>> +++ b/drivers/staging/speakup/selection.c
>>> @@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
>>>  	struct tty_ldisc *ld;
>>>  	DECLARE_WAITQUEUE(wait, current);
>>>  
>>> -	ld = tty_ldisc_ref_wait(tty);
>>> +	ld = tty_ldisc_ref(tty);
>>> +	if (!ld)
>>> +		return;
>>>  	tty_buffer_lock_exclusive(&vc->port);
>>>  
>>>  	add_wait_queue(&vc->paste_wait, &wait);
>>
>> This leaks a reference to the tty.  Instead of returning directly, I
>> think you need to add a label and goto the tty_kref_put() at the bottom
>> of the function.
> 
> Ugh, speakup_paste_selection() is a worse hack than I thought it was.

What if the kworker has already been scheduled but not run? Leaky reference
anyway.

What guarantees that the kref is gettable to begin with and isn't incrementing
from 0?

This isn't how tty krefs work.

I'll fix the patch to drop the kref but this is broken anyway.

Regards,
Peter Hurley

^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH v3 00/19] Fix driver crashes on hangup
  2016-01-10  4:40 ` [PATCH v2 00/19] Fix driver crashes on hangup Peter Hurley
                     ` (18 preceding siblings ...)
  2016-01-10  4:41   ` [PATCH v2 19/19] tty: Avoid unnecessary temporaries for tty->ldisc Peter Hurley
@ 2016-01-11  6:40   ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 01/19] staging: digi: Replace open-coded tty_wakeup() Peter Hurley
                       ` (18 more replies)
  19 siblings, 19 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

Changes for v3:
	Marked "tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)"  &
	       "n_tty: Fix unsafe reference to "other" ldisc" for stable
	Addressed Ben Hutchings comment regarding speakup_paste_selection()
	Integrated Fengguang's fix for "cons_filp != 0"

Changes for v2:
	Rebased on top of current tty-next
	Reduced changes/re-titled patch 19

NB: Marcel already picked up "bluetooth: hci_ldisc: Remove dead code" for
    bluetooth-next

---
Hi Greg,

This series fixes the underlying design problem that leads to driver crashes
during hangup (eg., Andi Kleen's report https://lkml.org/lkml/2015/11/9/786).

Quoting from patch 17/19:

    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.

With this patch series, the tty core now guarantees no further driver/ldisc
interactions after hangup.

Patch 1-4 remove direct tty->ldisc access outside the tty core.
Patch 5 removes the defunct chars_in_buffer() ldisc method (which has been
        deprecated since 3.12)
Patch 6 & 7 fix unsafe ldisc uses which coincidentally have been discovered
        to cause crashes (https://lkml.org/lkml/2015/11/26/173 and
	https://lkml.org/lkml/2015/11/26/253). These have been tagged for
	-stable.
Patch 8-16 are preparations; documenting existing functions and refactoring.
        Patch 12 adds handling for the possibility of NULL ldisc references
	after tty_ldisc_ref_wait(); that commit log details the logic of
	why/how that works.
Patch 17 implements the fix: the ldisc instance is killed and left dead.
        At tty_reopen() if the tty->ldisc is NULL, a new ldisc is instanced.
Patch 18-19 are minor add-ons.

Regards,


Peter Hurley (19):
  staging: digi: Replace open-coded tty_wakeup()
  serial: 68328: Remove bogus ldisc reset
  bluetooth: hci_ldisc: Remove dead code
  NFC: nci: Remove dead code
  tty: Remove chars_in_buffer() line discipline method
  tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  n_tty: Fix unsafe reference to "other" ldisc
  tty: Reset c_line from driver's init_termios
  staging/speakup: Use tty_ldisc_ref() for paste kworker
  tty: Fix comments for tty_ldisc_get()
  tty: Fix comments for tty_ldisc_release()
  tty: Prepare for destroying line discipline on hangup
  tty: Handle NULL tty->ldisc
  tty: Move tty_ldisc_kill()
  tty: Use 'disc' for line discipline index name
  tty: Refactor tty_ldisc_reinit() for reuse
  tty: Destroy ldisc instance on hangup
  tty: Document c_line == N_TTY initial condition
  tty: Avoid unnecessary temporaries for tty->ldisc

 Documentation/serial/tty.txt        |   3 -
 drivers/bluetooth/hci_ldisc.c       |   8 +-
 drivers/staging/dgap/dgap.c         |  28 ++----
 drivers/staging/dgnc/dgnc_tty.c     |  18 +---
 drivers/staging/speakup/selection.c |   5 +-
 drivers/tty/amiserial.c             |   6 +-
 drivers/tty/cyclades.c              |   8 +-
 drivers/tty/n_gsm.c                 |  16 ----
 drivers/tty/n_tty.c                 |  30 +------
 drivers/tty/rocket.c                |   6 +-
 drivers/tty/serial/68328serial.c    |  12 +--
 drivers/tty/serial/crisv10.c        |  12 ++-
 drivers/tty/tty_io.c                |  64 +++++++++++---
 drivers/tty/tty_ldisc.c             | 171 ++++++++++++++++++++----------------
 drivers/tty/vt/selection.c          |   2 +
 include/linux/tty.h                 |   5 +-
 include/linux/tty_ldisc.h           |   7 --
 net/nfc/nci/uart.c                  |   9 +-
 18 files changed, 180 insertions(+), 230 deletions(-)

-- 
2.7.0

^ permalink raw reply	[flat|nested] 71+ messages in thread
* [PATCH v3 01/19] staging: digi: Replace open-coded tty_wakeup()
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 02/19] serial: 68328: Remove bogus ldisc reset Peter Hurley
                       ` (17 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The open-coded tty_wakeup()s are attempts to workaround fixed bugs
in the line discipline write_wakeup() method. Replace with tty_wakeup().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/dgap/dgap.c     | 28 ++++++----------------------
 drivers/staging/dgnc/dgnc_tty.c | 18 ++----------------
 2 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index bad3551..dfbae21 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -1665,9 +1665,7 @@ static void dgap_input(struct channel_t *ch)
 }
 
 static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
-			      struct un_t *un, u32 mask,
-			      unsigned long *irq_flags1,
-			      unsigned long *irq_flags2)
+			      struct un_t *un, u32 mask)
 {
 	if (!(un->un_flags & mask))
 		return;
@@ -1677,17 +1675,7 @@ static void dgap_write_wakeup(struct board_t *bd, struct channel_t *ch,
 	if (!(un->un_flags & UN_ISOPEN))
 		return;
 
-	if ((un->un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-	    un->un_tty->ldisc->ops->write_wakeup) {
-		spin_unlock_irqrestore(&ch->ch_lock, *irq_flags2);
-		spin_unlock_irqrestore(&bd->bd_lock, *irq_flags1);
-
-		(un->un_tty->ldisc->ops->write_wakeup)(un->un_tty);
-
-		spin_lock_irqsave(&bd->bd_lock, *irq_flags1);
-		spin_lock_irqsave(&ch->ch_lock, *irq_flags2);
-	}
-	wake_up_interruptible(&un->un_tty->write_wait);
+	tty_wakeup(un->un_tty);
 	wake_up_interruptible(&un->un_flags_wait);
 }
 
@@ -1952,10 +1940,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit low.
 		 */
 		if (reason & IFTLW) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_LOW);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_LOW);
 			if (ch->ch_flags & CH_WLOW) {
 				ch->ch_flags &= ~CH_WLOW;
 				wake_up_interruptible(&ch->ch_flags_wait);
@@ -1966,10 +1952,8 @@ static int dgap_event(struct board_t *bd)
 		 * Process Transmit empty.
 		 */
 		if (reason & IFTEM) {
-			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
-			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY,
-					  &lock_flags, &lock_flags2);
+			dgap_write_wakeup(bd, ch, &ch->ch_tun, UN_EMPTY);
+			dgap_write_wakeup(bd, ch, &ch->ch_pun, UN_EMPTY);
 			if (ch->ch_flags & CH_WEMPTY) {
 				ch->ch_flags &= ~CH_WEMPTY;
 				wake_up_interruptible(&ch->ch_flags_wait);
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 48e4b90..c152048 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -933,14 +933,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_tun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_tun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_tun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_tun.un_tty->ldisc->ops->write_wakeup(ch->ch_tun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_tun.un_tty->write_wait);
+		tty_wakeup(ch->ch_tun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
@@ -975,14 +968,7 @@ void dgnc_wakeup_writes(struct channel_t *ch)
 	}
 
 	if (ch->ch_pun.un_flags & UN_ISOPEN) {
-		if ((ch->ch_pun.un_tty->flags & (1 << TTY_DO_WRITE_WAKEUP)) &&
-		    ch->ch_pun.un_tty->ldisc->ops->write_wakeup) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			ch->ch_pun.un_tty->ldisc->ops->write_wakeup(ch->ch_pun.un_tty);
-			spin_lock_irqsave(&ch->ch_lock, flags);
-		}
-
-		wake_up_interruptible(&ch->ch_pun.un_tty->write_wait);
+		tty_wakeup(ch->ch_pun.un_tty);
 
 		/*
 		 * If unit is set to wait until empty, check to make sure
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 02/19] serial: 68328: Remove bogus ldisc reset
  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     ` 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
                       ` (16 subsequent siblings)
  18 siblings, 1 reply; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

As the #warning indicates, the open-coded ldisc reset was always not ok.
Not only is this code long dead, but now it would have no effect as
the ldisc is destroyed when this driver's close() method returns; remove.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/68328serial.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
index 0982c1a..90639b5 100644
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -1054,17 +1054,7 @@ static void rs_close(struct tty_struct *tty, struct file *filp)
 	tty_ldisc_flush(tty);
 	tty->closing = 0;
 	tty_port_tty_set(&info->tport, NULL);
-#warning "This is not and has never been valid so fix it"	
-#if 0
-	if (tty->ldisc.num != ldiscs[N_TTY].num) {
-		if (tty->ldisc.close)
-			(tty->ldisc.close)(tty);
-		tty->ldisc = ldiscs[N_TTY];
-		tty->termios.c_line = N_TTY;
-		if (tty->ldisc.open)
-			(tty->ldisc.open)(tty);
-	}
-#endif	
+
 	if (port->blocked_open) {
 		if (port->close_delay)
 			msleep_interruptible(jiffies_to_msecs(port->close_delay));
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 03/19] bluetooth: hci_ldisc: Remove dead code
  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  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 04/19] NFC: nci: " Peter Hurley
                       ` (15 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The N_HCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/bluetooth/hci_ldisc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 96bcec5..c303f87 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -461,13 +461,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
 	INIT_WORK(&hu->init_ready, hci_uart_init_work);
 	INIT_WORK(&hu->write_work, hci_uart_write_work);
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	   open path is before the ldisc is referencable */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 04/19] NFC: nci: Remove dead code
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (2 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 03/19] bluetooth: hci_ldisc: Remove dead code Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
                       ` (14 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The N_NCI ldisc does not define a flush_buffer() ldisc method, so
the check when opening the ldisc is always false.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 net/nfc/nci/uart.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index 21d8875..c468eab 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -171,14 +171,7 @@ static int nci_uart_tty_open(struct tty_struct *tty)
 	tty->disc_data = NULL;
 	tty->receive_room = 65536;
 
-	/* Flush any pending characters in the driver and line discipline. */
-
-	/* FIXME: why is this needed. Note don't use ldisc_ref here as the
-	 * open path is before the ldisc is referencable.
-	 */
-
-	if (tty->ldisc->ops->flush_buffer)
-		tty->ldisc->ops->flush_buffer(tty);
+	/* Flush any pending characters in the driver */
 	tty_driver_flush_buffer(tty);
 
 	return 0;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 05/19] tty: Remove chars_in_buffer() line discipline method
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (3 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 04/19] NFC: nci: " Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
                       ` (13 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The chars_in_buffer() line discipline method serves no functional
purpose, other than as a (dubious) debugging aid for mostly bit-rotting
drivers. Despite being documented as an optional method, every caller
is unconditionally executed (although conditionally compiled).
Furthermore, direct tty->ldisc access without an ldisc ref is unsafe.
Lastly, N_TTY's chars_in_buffer() has warned of removal since 3.12.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 Documentation/serial/tty.txt |  3 ---
 drivers/tty/amiserial.c      |  6 ++----
 drivers/tty/cyclades.c       |  8 ++++----
 drivers/tty/n_gsm.c          | 16 ----------------
 drivers/tty/n_tty.c          | 23 -----------------------
 drivers/tty/rocket.c         |  6 ++----
 drivers/tty/serial/crisv10.c | 12 +++++-------
 include/linux/tty_ldisc.h    |  7 -------
 8 files changed, 13 insertions(+), 68 deletions(-)

diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
index bc3842d..798cba8 100644
--- a/Documentation/serial/tty.txt
+++ b/Documentation/serial/tty.txt
@@ -72,9 +72,6 @@ flush_buffer()	-	(optional) May be called at any point between
 			open and close, and instructs the line discipline
 			to empty its input buffer.
 
-chars_in_buffer() -	(optional) Report the number of bytes in the input
-			buffer.
-
 set_termios()	-	(optional) Called on termios structure changes.
 			The caller passes the old termios data and the
 			current data is in the tty. Called under the
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 2caaf5a..6ba5681 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -965,8 +965,7 @@ static void rs_throttle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_throttle"))
@@ -988,8 +987,7 @@ static void rs_unthrottle(struct tty_struct * tty)
 	struct serial_state *info = tty->driver_data;
 	unsigned long flags;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %d....\n", tty_name(tty),
-	       tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "rs_unthrottle"))
diff --git a/drivers/tty/cyclades.c b/drivers/tty/cyclades.c
index abbed20..a48e7e6 100644
--- a/drivers/tty/cyclades.c
+++ b/drivers/tty/cyclades.c
@@ -2852,8 +2852,8 @@ static void cy_throttle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:throttle %s: %ld...ttyC%d\n", tty_name(tty),
-			tty->ldisc.chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:throttle %s ...ttyC%d\n", tty_name(tty),
+			 info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_throttle"))
@@ -2891,8 +2891,8 @@ static void cy_unthrottle(struct tty_struct *tty)
 	unsigned long flags;
 
 #ifdef CY_DEBUG_THROTTLE
-	printk(KERN_DEBUG "cyc:unthrottle %s: %ld...ttyC%d\n",
-		tty_name(tty), tty_chars_in_buffer(tty), info->line);
+	printk(KERN_DEBUG "cyc:unthrottle %s ...ttyC%d\n",
+		tty_name(tty), info->line);
 #endif
 
 	if (serial_paranoia_check(info, tty->name, "cy_unthrottle"))
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c3fe026..e3cc277 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2304,21 +2304,6 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 }
 
 /**
- *	gsmld_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: gsm lock
- */
-
-static ssize_t gsmld_chars_in_buffer(struct tty_struct *tty)
-{
-	return 0;
-}
-
-/**
  *	gsmld_flush_buffer	-	clean input queue
  *	@tty:	terminal device
  *
@@ -2830,7 +2815,6 @@ static struct tty_ldisc_ops tty_ldisc_packet = {
 	.open            = gsmld_open,
 	.close           = gsmld_close,
 	.flush_buffer    = gsmld_flush_buffer,
-	.chars_in_buffer = gsmld_chars_in_buffer,
 	.read            = gsmld_read,
 	.write           = gsmld_write,
 	.ioctl           = gsmld_ioctl,
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index d9a5fc2..e09fd58 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -384,28 +384,6 @@ static void n_tty_flush_buffer(struct tty_struct *tty)
 }
 
 /**
- *	n_tty_chars_in_buffer	-	report available bytes
- *	@tty: tty device
- *
- *	Report the number of characters buffered to be delivered to user
- *	at this instant in time.
- *
- *	Locking: exclusive termios_rwsem
- */
-
-static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
-{
-	ssize_t n;
-
-	WARN_ONCE(1, "%s is deprecated and scheduled for removal.", __func__);
-
-	down_write(&tty->termios_rwsem);
-	n = chars_in_buffer(tty);
-	up_write(&tty->termios_rwsem);
-	return n;
-}
-
-/**
  *	is_utf8_continuation	-	utf8 multibyte check
  *	@c: byte to check
  *
@@ -2528,7 +2506,6 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.open            = n_tty_open,
 	.close           = n_tty_close,
 	.flush_buffer    = n_tty_flush_buffer,
-	.chars_in_buffer = n_tty_chars_in_buffer,
 	.read            = n_tty_read,
 	.write           = n_tty_write,
 	.ioctl           = n_tty_ioctl,
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index 802eac7..f624b93 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -1360,8 +1360,7 @@ static void rp_throttle(struct tty_struct *tty)
 	struct r_port *info = tty->driver_data;
 
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "throttle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "throttle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_throttle"))
@@ -1377,8 +1376,7 @@ static void rp_unthrottle(struct tty_struct *tty)
 {
 	struct r_port *info = tty->driver_data;
 #ifdef ROCKET_DEBUG_THROTTLE
-	printk(KERN_INFO "unthrottle %s: %d....\n", tty->name,
-	       tty->ldisc.chars_in_buffer(tty));
+	printk(KERN_INFO "unthrottle %s ....\n", tty->name);
 #endif
 
 	if (rocket_paranoia_check(info, "rp_unthrottle"))
diff --git a/drivers/tty/serial/crisv10.c b/drivers/tty/serial/crisv10.c
index f13f2eb..e98aef7 100644
--- a/drivers/tty/serial/crisv10.c
+++ b/drivers/tty/serial/crisv10.c
@@ -2968,7 +2968,7 @@ static int rs_raw_write(struct tty_struct *tty,
 
 	local_save_flags(flags);
 	DFLOW(DEBUG_LOG(info->line, "write count %i ", count));
-	DFLOW(DEBUG_LOG(info->line, "ldisc %i\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line, "ldisc\n"));
 
 
 	/* The local_irq_disable/restore_flags pairs below are needed
@@ -3161,10 +3161,9 @@ rs_throttle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("throttle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("throttle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_throttle %lu\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_throttle\n"));
 
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
@@ -3181,10 +3180,9 @@ rs_unthrottle(struct tty_struct * tty)
 {
 	struct e100_serial *info = (struct e100_serial *)tty->driver_data;
 #ifdef SERIAL_DEBUG_THROTTLE
-	printk("unthrottle %s: %lu....\n", tty_name(tty),
-	       (unsigned long)tty->ldisc.chars_in_buffer(tty));
+	printk("unthrottle %s ....\n", tty_name(tty));
 #endif
-	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc %d\n", tty->ldisc.chars_in_buffer(tty)));
+	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle ldisc\n"));
 	DFLOW(DEBUG_LOG(info->line,"rs_unthrottle flip.count: %i\n", tty->flip.count));
 	/* Do RTS before XOFF since XOFF might take some time */
 	if (tty->termios.c_cflag & CRTSCTS) {
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 00c9d68..6101ab8 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -25,12 +25,6 @@
  *	buffers of any input characters it may have queued to be
  *	delivered to the user mode process.
  *
- * ssize_t (*chars_in_buffer)(struct tty_struct *tty);
- *
- *	This function returns the number of input characters the line
- *	discipline may have queued up to be delivered to the user mode
- *	process.
- *
  * ssize_t (*read)(struct tty_struct * tty, struct file * file,
  *		   unsigned char * buf, size_t nr);
  *
@@ -188,7 +182,6 @@ struct tty_ldisc_ops {
 	int	(*open)(struct tty_struct *);
 	void	(*close)(struct tty_struct *);
 	void	(*flush_buffer)(struct tty_struct *tty);
-	ssize_t	(*chars_in_buffer)(struct tty_struct *tty);
 	ssize_t	(*read)(struct tty_struct *tty, struct file *file,
 			unsigned char __user *buf, size_t nr);
 	ssize_t	(*write)(struct tty_struct *tty, struct file *file,
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD)
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (4 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 05/19] tty: Remove chars_in_buffer() line discipline method Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
                       ` (12 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley, stable

ioctl(TIOCGETD) retrieves the line discipline id directly from the
ldisc because the line discipline id (c_line) in termios is untrustworthy;
userspace may have set termios via ioctl(TCSETS*) without actually
changing the line discipline via ioctl(TIOCSETD).

However, directly accessing the current ldisc via tty->ldisc is
unsafe; the ldisc ptr dereferenced may be stale if the line discipline
is changing via ioctl(TIOCSETD) or hangup.

Wait for the line discipline reference (just like read() or write())
to retrieve the "current" line discipline id.

Cc: <stable@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 892c923..56d3a6b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2649,6 +2649,28 @@ static int tiocsetd(struct tty_struct *tty, int __user *p)
 }
 
 /**
+ *	tiocgetd	-	get line discipline
+ *	@tty: tty device
+ *	@p: pointer to user data
+ *
+ *	Retrieves the line discipline id directly from the ldisc.
+ *
+ *	Locking: waits for ldisc reference (in case the line discipline
+ *		is changing or the tty is being hungup)
+ */
+
+static int tiocgetd(struct tty_struct *tty, int __user *p)
+{
+	struct tty_ldisc *ld;
+	int ret;
+
+	ld = tty_ldisc_ref_wait(tty);
+	ret = put_user(ld->ops->num, p);
+	tty_ldisc_deref(ld);
+	return ret;
+}
+
+/**
  *	send_break	-	performed time break
  *	@tty: device to break on
  *	@duration: timeout in mS
@@ -2874,7 +2896,7 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case TIOCGSID:
 		return tiocgsid(tty, real_tty, p);
 	case TIOCGETD:
-		return put_user(tty->ldisc->ops->num, (int __user *)p);
+		return tiocgetd(tty, p);
 	case TIOCSETD:
 		return tiocsetd(tty, p);
 	case TIOCVHANGUP:
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 07/19] n_tty: Fix unsafe reference to "other" ldisc
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (5 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 06/19] tty: Fix unsafe ldisc reference via ioctl(TIOCGETD) Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 08/19] tty: Reset c_line from driver's init_termios Peter Hurley
                       ` (11 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley, stable

Although n_tty_check_unthrottle() has a valid ldisc reference (since
the tty core gets the ldisc ref in tty_read() before calling the line
discipline read() method), it does not have a valid ldisc reference to
the "other" pty of a pty pair. Since getting an ldisc reference for
tty->link essentially open-codes tty_wakeup(), just replace with the
equivalent tty_wakeup().

Cc: <stable@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index e09fd58..90eca26 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -269,16 +269,13 @@ static void n_tty_check_throttle(struct tty_struct *tty)
 
 static void n_tty_check_unthrottle(struct tty_struct *tty)
 {
-	if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
-	    tty->link->ldisc->ops->write_wakeup == n_tty_write_wakeup) {
+	if (tty->driver->type == TTY_DRIVER_TYPE_PTY) {
 		if (chars_in_buffer(tty) > TTY_THRESHOLD_UNTHROTTLE)
 			return;
 		if (!tty->count)
 			return;
 		n_tty_kick_worker(tty);
-		n_tty_write_wakeup(tty->link);
-		if (waitqueue_active(&tty->link->write_wait))
-			wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
+		tty_wakeup(tty->link);
 		return;
 	}
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 08/19] tty: Reset c_line from driver's init_termios
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (6 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 07/19] n_tty: Fix unsafe reference to "other" ldisc Peter Hurley
@ 2016-01-11  6:40     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker Peter Hurley
                       ` (10 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

After the ldisc is released, but before the tty is destroyed, the termios
is saved (in tty_free_termios()); this termios is restored if a new
tty is created on next open(). However, the line discipline is always
reset, which is not obvious in the current method. Instead, reset
as part of the restore.

Restore the original line discipline, which may not have been N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    | 5 +++--
 drivers/tty/tty_ldisc.c | 3 ---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 56d3a6b..0871d1e 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1388,9 +1388,10 @@ int tty_init_termios(struct tty_struct *tty)
 	else {
 		/* Check for lazy saved data */
 		tp = tty->driver->termios[idx];
-		if (tp != NULL)
+		if (tp != NULL) {
 			tty->termios = *tp;
-		else
+			tty->termios.c_line  = tty->driver->init_termios.c_line;
+		} else
 			tty->termios = tty->driver->init_termios;
 	}
 	/* Compatibility until drivers always set this */
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index a054d03..d5104b0 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -745,9 +745,6 @@ static void tty_ldisc_kill(struct tty_struct *tty)
 	tty_ldisc_put(tty->ldisc);
 	/* Force an oops if we mess this up */
 	tty->ldisc = NULL;
-
-	/* Ensure the next open requests the N_TTY ldisc */
-	tty_set_termios_ldisc(tty, N_TTY);
 }
 
 /**
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (7 preceding siblings ...)
  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     ` Peter Hurley
  2016-01-11  6:40     ` [PATCH v3 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
                       ` (9 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley, stable

As the function documentation for tty_ldisc_ref_wait() notes, it is
only callable from a tty file_operations routine; otherwise there
is no guarantee the ref won't be NULL.

The key difference with the VT's paste_selection() is that is an ioctl,
where __speakup_paste_selection() is completely async kworker, kicked
off from interrupt context.

Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
       tty (ab)usage to match vt")
Cc: <stable@vger.kernel.org>

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/speakup/selection.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
index aa5ab6c..41ef099 100644
--- a/drivers/staging/speakup/selection.c
+++ b/drivers/staging/speakup/selection.c
@@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
 	struct tty_ldisc *ld;
 	DECLARE_WAITQUEUE(wait, current);
 
-	ld = tty_ldisc_ref_wait(tty);
+	ld = tty_ldisc_ref(tty);
+	if (!ld)
+		goto tty_unref;
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
@@ -162,6 +164,7 @@ static void __speakup_paste_selection(struct work_struct *work)
 
 	tty_buffer_unlock_exclusive(&vc->port);
 	tty_ldisc_deref(ld);
+tty_unref:
 	tty_kref_put(tty);
 }
 
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 10/19] tty: Fix comments for tty_ldisc_get()
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (8 preceding siblings ...)
  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     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
                       ` (8 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_get() returns ERR_PTR() values if unsuccessful, not NULL;
fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index d5104b0..b3cead9 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -140,9 +140,16 @@ static void put_ldops(struct tty_ldisc_ops *ldops)
  *	@disc: ldisc number
  *
  *	Takes a reference to a line discipline. Deals with refcounts and
- *	module locking counts. Returns NULL if the discipline is not available.
- *	Returns a pointer to the discipline and bumps the ref count if it is
- *	available
+ *	module locking counts.
+ *
+ *	Returns: -EINVAL if the discipline index is not [N_TTY..NR_LDISCS] or
+ *			 if the discipline is not registered
+ *		 -EAGAIN if request_module() failed to load or register the
+ *			 the discipline
+ *		 -ENOMEM if allocation failure
+ *
+ *		 Otherwise, returns a pointer to the discipline and bumps the
+ *		 ref count
  *
  *	Locking:
  *		takes tty_ldiscs_lock to guard against ldisc races
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 11/19] tty: Fix comments for tty_ldisc_release()
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (9 preceding siblings ...)
  2016-01-11  6:40     ` [PATCH v3 10/19] tty: Fix comments for tty_ldisc_get() Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
                       ` (7 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_kill() sets tty->ldisc to NULL; _not_ to N_TTY with a valid
but unopened ldisc. Fix function header documentation.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index b3cead9..8582aeb 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -759,8 +759,7 @@ static void tty_ldisc_kill(struct tty_struct *tty)
  *	@tty: tty being shut down (or one end of pty pair)
  *
  *	Called during the final close of a tty or a pty pair in order to shut
- *	down the line discpline layer. On exit, each ldisc assigned is N_TTY and
- *	each ldisc has not been opened.
+ *	down the line discpline layer. On exit, each tty's ldisc is NULL.
  */
 
 void tty_ldisc_release(struct tty_struct *tty)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 12/19] tty: Prepare for destroying line discipline on hangup
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (10 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 11/19] tty: Fix comments for tty_ldisc_release() Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 13/19] tty: Handle NULL tty->ldisc Peter Hurley
                       ` (6 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty file_operations (read/write/ioctl) wait for the ldisc reference
indefinitely (until ldisc lifetime events, such as hangup or TIOCSETD,
finish). Since hangup now destroys the ldisc and does not instance
another copy, file_operations must now be prepared to receive a NULL
ldisc reference from tty_ldisc_ref_wait():

CPU 0                                   CPU 1
-----                                   -----
(*f_op->read)() => tty_read()
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()
ld = tty_ldisc_ref_wait()
/* ld == NULL */

Instead, the action taken now is to return the same value as if the
tty had been hungup a moment earlier:

CPU 0                                   CPU 1
-----                                   -----
                                        __tty_hangup()
                                        ...
                                        f_op = &hung_up_tty_fops;
(*f_op->read)() => hung_up_tty_read()
return 0;
                                        ...
                                        tty_ldisc_hangup()
                                           tty_ldisc_lock()
                                           tty_ldisc_kill()
                                              tty->ldisc = NULL
                                           tty_ldisc_unlock()

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c       | 14 ++++++++++++++
 drivers/tty/tty_ldisc.c    |  4 ++--
 drivers/tty/vt/selection.c |  2 ++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 0871d1e..89822c4 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1069,6 +1069,8 @@ static ssize_t tty_read(struct file *file, char __user *buf, size_t count,
 	/* We want to wait for the line discipline to sort out in this
 	   situation */
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_read(file, buf, count, ppos);
 	if (ld->ops->read)
 		i = ld->ops->read(tty, file, buf, count);
 	else
@@ -1243,6 +1245,8 @@ static ssize_t tty_write(struct file *file, const char __user *buf,
 	if (tty->ops->write_room == NULL)
 		tty_err(tty, "missing write_room method\n");
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_write(file, buf, count, ppos);
 	if (!ld->ops->write)
 		ret = -EIO;
 	else
@@ -2185,6 +2189,8 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 		return 0;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_poll(filp, wait);
 	if (ld->ops->poll)
 		ret = ld->ops->poll(tty, filp, wait);
 	tty_ldisc_deref(ld);
@@ -2274,6 +2280,8 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 		return -EFAULT;
 	tty_audit_tiocsti(tty, ch);
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ld->ops->receive_buf(tty, &ch, &mbz, 1);
 	tty_ldisc_deref(ld);
 	return 0;
@@ -2666,6 +2674,8 @@ static int tiocgetd(struct tty_struct *tty, int __user *p)
 	int ret;
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;
 	ret = put_user(ld->ops->num, p);
 	tty_ldisc_deref(ld);
 	return ret;
@@ -2963,6 +2973,8 @@ long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			return retval;
 	}
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_ioctl(file, cmd, arg);
 	retval = -EINVAL;
 	if (ld->ops->ioctl) {
 		retval = ld->ops->ioctl(tty, file, cmd, arg);
@@ -2991,6 +3003,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd,
 	}
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return hung_up_tty_compat_ioctl(file, cmd, arg);
 	if (ld->ops->compat_ioctl)
 		retval = ld->ops->compat_ioctl(tty, file, cmd, arg);
 	else
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 8582aeb..1c9ce25 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -262,8 +262,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
  *	against a discipline change, such as an existing ldisc reference
  *	(which we check for)
  *
- *	Note: only callable from a file_operations routine (which
- *	guarantees tty->ldisc != NULL when the lock is acquired).
+ *	Note: a file_operations routine (read/poll/write) should use this
+ *	function to wait for any ldisc lifetime events to finish.
  */
 
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 381a2b1..4dd9dd2 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -347,6 +347,8 @@ int paste_selection(struct tty_struct *tty)
 	console_unlock();
 
 	ld = tty_ldisc_ref_wait(tty);
+	if (!ld)
+		return -EIO;	/* ldisc was hung up */
 	tty_buffer_lock_exclusive(&vc->port);
 
 	add_wait_queue(&vc->paste_wait, &wait);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 13/19] tty: Handle NULL tty->ldisc
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (11 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 12/19] tty: Prepare for destroying line discipline on hangup Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 14/19] tty: Move tty_ldisc_kill() Peter Hurley
                       ` (5 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

In preparation of destroying line discipline on hangup, fix
ldisc core operations to properly handle when the tty's ldisc is
NULL.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 1c9ce25..5115230 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -269,7 +269,8 @@ const struct file_operations tty_ldiscs_proc_fops = {
 struct tty_ldisc *tty_ldisc_ref_wait(struct tty_struct *tty)
 {
 	ldsem_down_read(&tty->ldisc_sem, MAX_SCHEDULE_TIMEOUT);
-	WARN_ON(!tty->ldisc);
+	if (!tty->ldisc)
+		ldsem_up_read(&tty->ldisc_sem);
 	return tty->ldisc;
 }
 EXPORT_SYMBOL_GPL(tty_ldisc_ref_wait);
@@ -462,7 +463,7 @@ static int tty_ldisc_open(struct tty_struct *tty, struct tty_ldisc *ld)
 		if (ret)
 			clear_bit(TTY_LDISC_OPEN, &tty->flags);
 
-		tty_ldisc_debug(tty, "%p: opened\n", tty->ldisc);
+		tty_ldisc_debug(tty, "%p: opened\n", ld);
 		return ret;
 	}
 	return 0;
@@ -483,7 +484,7 @@ static void tty_ldisc_close(struct tty_struct *tty, struct tty_ldisc *ld)
 	clear_bit(TTY_LDISC_OPEN, &tty->flags);
 	if (ld->ops->close)
 		ld->ops->close(tty);
-	tty_ldisc_debug(tty, "%p: closed\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: closed\n", ld);
 }
 
 /**
@@ -546,6 +547,11 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	if (retval)
 		goto err;
 
+	if (!tty->ldisc) {
+		retval = -EIO;
+		goto out;
+	}
+
 	/* Check the no-op case */
 	if (tty->ldisc->ops->num == ldisc)
 		goto out;
@@ -661,7 +667,7 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 	int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS;
 	int err = 0;
 
-	tty_ldisc_debug(tty, "%p: closing\n", tty->ldisc);
+	tty_ldisc_debug(tty, "%p: hangup\n", tty->ldisc);
 
 	ld = tty_ldisc_ref(tty);
 	if (ld != NULL) {
@@ -745,6 +751,8 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 
 static void tty_ldisc_kill(struct tty_struct *tty)
 {
+	if (!tty->ldisc)
+		return;
 	/*
 	 * Now kill off the ldisc
 	 */
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 14/19] tty: Move tty_ldisc_kill()
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (12 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 13/19] tty: Handle NULL tty->ldisc Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
                       ` (4 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

In preparation for destroying the line discipline instance on hangup,
move tty_ldisc_kill() to eliminate needless forward declarations.
No functional change.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 5115230..d91122d 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -603,6 +603,25 @@ err:
 }
 
 /**
+ *	tty_ldisc_kill	-	teardown ldisc
+ *	@tty: tty being released
+ *
+ *	Perform final close of the ldisc and reset tty->ldisc
+ */
+static void tty_ldisc_kill(struct tty_struct *tty)
+{
+	if (!tty->ldisc)
+		return;
+	/*
+	 * Now kill off the ldisc
+	 */
+	tty_ldisc_close(tty, tty->ldisc);
+	tty_ldisc_put(tty->ldisc);
+	/* Force an oops if we mess this up */
+	tty->ldisc = NULL;
+}
+
+/**
  *	tty_reset_termios	-	reset terminal state
  *	@tty: tty to reset
  *
@@ -749,19 +768,6 @@ int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 	return 0;
 }
 
-static void tty_ldisc_kill(struct tty_struct *tty)
-{
-	if (!tty->ldisc)
-		return;
-	/*
-	 * Now kill off the ldisc
-	 */
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/* Force an oops if we mess this up */
-	tty->ldisc = NULL;
-}
-
 /**
  *	tty_ldisc_release		-	release line discipline
  *	@tty: tty being shut down (or one end of pty pair)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 15/19] tty: Use 'disc' for line discipline index name
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (13 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 14/19] tty: Move tty_ldisc_kill() Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
                       ` (3 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty->ldisc is a ptr to struct tty_ldisc, but unfortunately 'ldisc' is
also used as a parameter or local name to refer to the line discipline
index value (ie, N_TTY, N_GSM, etc.); instead prefer the name used
by the line discipline registration/ref counting functions.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c    |  6 +++---
 drivers/tty/tty_ldisc.c | 22 +++++++++++-----------
 include/linux/tty.h     |  2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 89822c4..f6f559c 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2646,13 +2646,13 @@ static int tiocgsid(struct tty_struct *tty, struct tty_struct *real_tty, pid_t _
 
 static int tiocsetd(struct tty_struct *tty, int __user *p)
 {
-	int ldisc;
+	int disc;
 	int ret;
 
-	if (get_user(ldisc, p))
+	if (get_user(disc, p))
 		return -EFAULT;
 
-	ret = tty_set_ldisc(tty, ldisc);
+	ret = tty_set_ldisc(tty, disc);
 
 	return ret;
 }
diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index d91122d..6ba6fe1 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -419,7 +419,7 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
 /**
  *	tty_set_termios_ldisc		-	set ldisc field
  *	@tty: tty structure
- *	@num: line discipline number
+ *	@disc: line discipline number
  *
  *	This is probably overkill for real world processors but
  *	they are not on hot paths so a little discipline won't do
@@ -432,10 +432,10 @@ EXPORT_SYMBOL_GPL(tty_ldisc_flush);
  *	Locking: takes termios_rwsem
  */
 
-static void tty_set_termios_ldisc(struct tty_struct *tty, int num)
+static void tty_set_termios_ldisc(struct tty_struct *tty, int disc)
 {
 	down_write(&tty->termios_rwsem);
-	tty->termios.c_line = num;
+	tty->termios.c_line = disc;
 	up_write(&tty->termios_rwsem);
 
 	tty->disc_data = NULL;
@@ -533,12 +533,12 @@ static void tty_ldisc_restore(struct tty_struct *tty, struct tty_ldisc *old)
  *	the close of one side of a tty/pty pair, and eventually hangup.
  */
 
-int tty_set_ldisc(struct tty_struct *tty, int ldisc)
+int tty_set_ldisc(struct tty_struct *tty, int disc)
 {
 	int retval;
 	struct tty_ldisc *old_ldisc, *new_ldisc;
 
-	new_ldisc = tty_ldisc_get(tty, ldisc);
+	new_ldisc = tty_ldisc_get(tty, disc);
 	if (IS_ERR(new_ldisc))
 		return PTR_ERR(new_ldisc);
 
@@ -553,7 +553,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	}
 
 	/* Check the no-op case */
-	if (tty->ldisc->ops->num == ldisc)
+	if (tty->ldisc->ops->num == disc)
 		goto out;
 
 	if (test_bit(TTY_HUPPED, &tty->flags)) {
@@ -569,7 +569,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 
 	/* Now set up the new line discipline. */
 	tty->ldisc = new_ldisc;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	retval = tty_ldisc_open(tty, new_ldisc);
 	if (retval < 0) {
@@ -641,15 +641,15 @@ static void tty_reset_termios(struct tty_struct *tty)
 /**
  *	tty_ldisc_reinit	-	reinitialise the tty ldisc
  *	@tty: tty to reinit
- *	@ldisc: line discipline to reinitialize
+ *	@disc: line discipline to reinitialize
  *
  *	Switch the tty to a line discipline and leave the ldisc
  *	state closed
  */
 
-static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
+static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, ldisc);
+	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
 
 	if (IS_ERR(ld))
 		return -1;
@@ -660,7 +660,7 @@ static int tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
 	 *	Switch the line discipline back
 	 */
 	tty->ldisc = ld;
-	tty_set_termios_ldisc(tty, ldisc);
+	tty_set_termios_ldisc(tty, disc);
 
 	return 0;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2fd8708..ee73220 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -575,7 +575,7 @@ static inline int tty_port_users(struct tty_port *port)
 
 extern int tty_register_ldisc(int disc, struct tty_ldisc_ops *new_ldisc);
 extern int tty_unregister_ldisc(int disc);
-extern int tty_set_ldisc(struct tty_struct *tty, int ldisc);
+extern int tty_set_ldisc(struct tty_struct *tty, int disc);
 extern int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty);
 extern void tty_ldisc_release(struct tty_struct *tty);
 extern void tty_ldisc_init(struct tty_struct *tty);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 16/19] tty: Refactor tty_ldisc_reinit() for reuse
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (14 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 15/19] tty: Use 'disc' for line discipline index name Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
                       ` (2 subsequent siblings)
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

At tty hangup, the line discipline instance is reinitialized by
closing the current ldisc instance and opening a new instance.
This operation is complicated by error recovery: if the attempt
to reinit the current line discipline fails, the line discipline
is reset to N_TTY (which should not but can fail).

Re-purpose tty_ldisc_reinit() to return a valid, open line discipline
instance, or otherwise, an error.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 53 +++++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 6ba6fe1..527fc5b 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -643,26 +643,41 @@ static void tty_reset_termios(struct tty_struct *tty)
  *	@tty: tty to reinit
  *	@disc: line discipline to reinitialize
  *
- *	Switch the tty to a line discipline and leave the ldisc
- *	state closed
+ *	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.
+ *
+ *	Returns 0 if successful, otherwise error code < 0
  */
 
 static int tty_ldisc_reinit(struct tty_struct *tty, int disc)
 {
-	struct tty_ldisc *ld = tty_ldisc_get(tty, disc);
+	struct tty_ldisc *ld;
+	int retval;
 
-	if (IS_ERR(ld))
-		return -1;
+	ld = tty_ldisc_get(tty, disc);
+	if (IS_ERR(ld)) {
+		BUG_ON(disc == N_TTY);
+		return PTR_ERR(ld);
+	}
 
-	tty_ldisc_close(tty, tty->ldisc);
-	tty_ldisc_put(tty->ldisc);
-	/*
-	 *	Switch the line discipline back
-	 */
+	if (tty->ldisc) {
+		tty_ldisc_close(tty, tty->ldisc);
+		tty_ldisc_put(tty->ldisc);
+	}
+
+	/* switch the line discipline */
 	tty->ldisc = ld;
 	tty_set_termios_ldisc(tty, disc);
-
-	return 0;
+	retval = tty_ldisc_open(tty, tty->ldisc);
+	if (retval) {
+		if (!WARN_ON(disc == N_TTY)) {
+			tty_ldisc_put(tty->ldisc);
+			tty->ldisc = NULL;
+		}
+	}
+	return retval;
 }
 
 /**
@@ -718,19 +733,13 @@ void tty_ldisc_hangup(struct tty_struct *tty)
 		   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) {
+		if (reset == 0)
+			err = tty_ldisc_reinit(tty, tty->termios.c_line);
 
-			if (!tty_ldisc_reinit(tty, tty->termios.c_line))
-				err = tty_ldisc_open(tty, tty->ldisc);
-			else
-				err = 1;
-		}
 		/* If the re-open fails or we reset then go to N_TTY. The
 		   N_TTY open cannot fail */
-		if (reset || err) {
-			BUG_ON(tty_ldisc_reinit(tty, N_TTY));
-			WARN_ON(tty_ldisc_open(tty, tty->ldisc));
-		}
+		if (reset || err < 0)
+			tty_ldisc_reinit(tty, N_TTY);
 	}
 	tty_ldisc_unlock(tty);
 	if (reset)
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 17/19] tty: Destroy ldisc instance on hangup
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (15 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 16/19] tty: Refactor tty_ldisc_reinit() for reuse Peter Hurley
@ 2016-01-11  6:41     ` 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
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

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..4ad8454 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 != NULL);
 
 	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

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 18/19] tty: Document c_line == N_TTY initial condition
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (16 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  2016-01-11  6:41     ` [PATCH v3 19/19] tty: Avoid unnecessary temporaries for tty->ldisc Peter Hurley
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

The line discipline id is stored in the tty's termios; document the
implicit initial value of N_TTY.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 4ad8454..d526d59 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -123,7 +123,8 @@ struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
 		   ECHOCTL | ECHOKE | IEXTEN,
 	.c_cc = INIT_C_CC,
 	.c_ispeed = 38400,
-	.c_ospeed = 38400
+	.c_ospeed = 38400,
+	/* .c_line = N_TTY, */
 };
 
 EXPORT_SYMBOL(tty_std_termios);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* [PATCH v3 19/19] tty: Avoid unnecessary temporaries for tty->ldisc
  2016-01-11  6:40   ` [PATCH v3 00/19] Fix driver crashes on hangup Peter Hurley
                       ` (17 preceding siblings ...)
  2016-01-11  6:41     ` [PATCH v3 18/19] tty: Document c_line == N_TTY initial condition Peter Hurley
@ 2016-01-11  6:41     ` Peter Hurley
  18 siblings, 0 replies; 71+ messages in thread
From: Peter Hurley @ 2016-01-11  6:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Peter Hurley

tty_ldisc_setup() is race-free and can reference tty->ldisc without
snapshots.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_ldisc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index 199e4b4..b404c20 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -754,17 +754,14 @@ void tty_ldisc_hangup(struct tty_struct *tty, bool reinit)
 
 int tty_ldisc_setup(struct tty_struct *tty, struct tty_struct *o_tty)
 {
-	struct tty_ldisc *ld = tty->ldisc;
-	int retval;
-
-	retval = tty_ldisc_open(tty, ld);
+	int retval = tty_ldisc_open(tty, tty->ldisc);
 	if (retval)
 		return retval;
 
 	if (o_tty) {
 		retval = tty_ldisc_open(o_tty, o_tty->ldisc);
 		if (retval) {
-			tty_ldisc_close(tty, ld);
+			tty_ldisc_close(tty, tty->ldisc);
 			return retval;
 		}
 	}
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 71+ messages in thread
* Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker
  2016-01-11  5:40         ` Peter Hurley
@ 2016-01-11 10:37           ` Ben Hutchings
  0 siblings, 0 replies; 71+ messages in thread
From: Ben Hutchings @ 2016-01-11 10:37 UTC (permalink / raw)
  To: Peter Hurley, Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, stable

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

On Sun, 2016-01-10 at 21:40 -0800, Peter Hurley wrote:
> On 01/10/2016 04:25 PM, Peter Hurley wrote:
> > On 01/10/2016 03:16 PM, Ben Hutchings wrote:
> > > On Sat, 2016-01-09 at 20:41 -0800, Peter Hurley wrote:
> > > > As the function documentation for tty_ldisc_ref_wait() notes, it is
> > > > only callable from a tty file_operations routine; otherwise there
> > > > is no guarantee the ref won't be NULL.
> > > > 
> > > > The key difference with the VT's paste_selection() is that is an ioctl,
> > > > where __speakup_paste_selection() is completely asynch kworker, kicked
> > > > off from interrupt context.
> > > > 
> > > > Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection()
> > > >        tty (ab)usage to match vt")
> > > > Cc: <stable@vger.kernel.org>
> > > > 
> > > > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> > > > ---
> > > >  drivers/staging/speakup/selection.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
> > > > index aa5ab6c..86c0b9a 100644
> > > > --- a/drivers/staging/speakup/selection.c
> > > > +++ b/drivers/staging/speakup/selection.c
> > > > @@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct work_struct *work)
> > > >  	struct tty_ldisc *ld;
> > > >  	DECLARE_WAITQUEUE(wait, current);
> > > >  
> > > > -	ld = tty_ldisc_ref_wait(tty);
> > > > +	ld = tty_ldisc_ref(tty);
> > > > +	if (!ld)
> > > > +		return;
> > > >  	tty_buffer_lock_exclusive(&vc->port);
> > > >  
> > > >  	add_wait_queue(&vc->paste_wait, &wait);
> > > 
> > > This leaks a reference to the tty.  Instead of returning directly, I
> > > think you need to add a label and goto the tty_kref_put() at the bottom
> > > of the function.
> > 
> > Ugh, speakup_paste_selection() is a worse hack than I thought it was.
> 
> What if the kworker has already been scheduled but not run? Leaky reference
> anyway.

I don't think so - the cmpxchg() should ensure that only one paste is
outstanding.

> What guarantees that the kref is gettable to begin with and isn't incrementing
> from 0?

Surely it's a bug in the caller of speakup_paste_selection() if it
doesn't hold a reference?

Ben.

> This isn't how tty krefs work.
> 
> I'll fix the patch to drop the kref but this is broken anyway.
> 
> Regards,
> Peter Hurley
> 
-- 
Ben Hutchings
Q.  Which is the greater problem in the world today, ignorance or apathy?
A.  I don't know and I couldn't care less.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 71+ messages in thread
* Re: [PATCH v3 02/19] serial: 68328: Remove bogus ldisc reset
  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
  0 siblings, 0 replies; 71+ messages in thread
From: One Thousand Gnomes @ 2016-01-11 14:12 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel

On Sun, 10 Jan 2016 22:40:51 -0800
Peter Hurley <peter@hurleysoftware.com> wrote:

> As the #warning indicates, the open-coded ldisc reset was always not ok.
> Not only is this code long dead,

The entire 683xx port is long dead. As per the previous discussions on
this the entire driver should go closely followed by the entire port.

Nobody is using 683xx parts any more, and even if they did they wouldn't
run a current Linux remotely usefully.

Alan

^ permalink raw reply	[flat|nested] 71+ messages in thread
end of thread, other threads:[~2016-01-11 14:12 UTC | newest]

Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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   ` [PATCH v2 17/19] tty: Destroy ldisc instance on hangup Peter Hurley
2016-01-10  6:24     ` 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

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