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
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 <[email protected]>
Cc: Daeseok Youn <[email protected]>
Cc: Mark Hounschell <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
Cc: Aloisio Almeida Jr <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
---
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
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: <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
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: <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
---
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
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: <[email protected]>
Signed-off-by: Peter Hurley <[email protected]>
---
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
tty_ldisc_get() returns ERR_PTR() values if unsuccessful, not NULL;
fix function header documentation.
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
The line discipline id is stored in the tty's termios; document the
implicit initial value of N_TTY.
Signed-off-by: Peter Hurley <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> Cc: Johan Hedberg <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel