|
|
721969b |
From: Peter Hurley <peter@hurleysoftware.com>
|
|
|
721969b |
Date: Mon, 13 Apr 2015 13:24:34 -0400
|
|
|
721969b |
Subject: [PATCH] pty: Fix input race when closing
|
|
|
721969b |
|
|
|
721969b |
A read() from a pty master may mistakenly indicate EOF (errno == -EIO)
|
|
|
721969b |
after the pty slave has closed, even though input data remains to be read.
|
|
|
721969b |
For example,
|
|
|
721969b |
|
|
|
721969b |
pty slave | input worker | pty master
|
|
|
721969b |
| |
|
|
|
721969b |
| | n_tty_read()
|
|
|
721969b |
pty_write() | | input avail? no
|
|
|
721969b |
add data | | sleep
|
|
|
721969b |
schedule worker --->| | .
|
|
|
721969b |
|---> flush_to_ldisc() | .
|
|
|
721969b |
pty_close() | fill read buffer | .
|
|
|
721969b |
wait for worker | wakeup reader --->| .
|
|
|
721969b |
| read buffer full? |---> input avail ? yes
|
|
|
721969b |
|<--- yes - exit worker | copy 4096 bytes to user
|
|
|
721969b |
TTY_OTHER_CLOSED <---| |<--- kick worker
|
|
|
721969b |
| |
|
|
|
721969b |
|
|
|
721969b |
**** New read() before worker starts ****
|
|
|
721969b |
|
|
|
721969b |
| | n_tty_read()
|
|
|
721969b |
| | input avail? no
|
|
|
721969b |
| | TTY_OTHER_CLOSED? yes
|
|
|
721969b |
| | return -EIO
|
|
|
721969b |
|
|
|
721969b |
Several conditions are required to trigger this race:
|
|
|
721969b |
1. the ldisc read buffer must become full so the input worker exits
|
|
|
721969b |
2. the read() count parameter must be >= 4096 so the ldisc read buffer
|
|
|
721969b |
is empty
|
|
|
721969b |
3. the subsequent read() occurs before the kicked worker has processed
|
|
|
721969b |
more input
|
|
|
721969b |
|
|
|
721969b |
However, the underlying cause of the race is that data is pipelined, while
|
|
|
721969b |
tty state is not; ie., data already written by the pty slave end is not
|
|
|
721969b |
yet visible to the pty master end, but state changes by the pty slave end
|
|
|
721969b |
are visible to the pty master end immediately.
|
|
|
721969b |
|
|
|
721969b |
Pipeline the TTY_OTHER_CLOSED state through input worker to the reader.
|
|
|
721969b |
1. Introduce TTY_OTHER_DONE which is set by the input worker when
|
|
|
721969b |
TTY_OTHER_CLOSED is set and either the input buffers are flushed or
|
|
|
721969b |
input processing has completed. Readers/polls are woken when
|
|
|
721969b |
TTY_OTHER_DONE is set.
|
|
|
721969b |
2. Reader/poll checks TTY_OTHER_DONE instead of TTY_OTHER_CLOSED.
|
|
|
721969b |
3. A new input worker is started from pty_close() after setting
|
|
|
721969b |
TTY_OTHER_CLOSED, which ensures the TTY_OTHER_DONE state will be
|
|
|
721969b |
set if the last input worker is already finished (or just about to
|
|
|
721969b |
exit).
|
|
|
721969b |
|
|
|
721969b |
Remove tty_flush_to_ldisc(); no in-tree callers.
|
|
|
721969b |
|
|
|
721969b |
Fixes: 52bce7f8d4fc ("pty, n_tty: Simplify input processing on final close")
|
|
|
721969b |
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96311
|
|
|
721969b |
BugLink: http://bugs.launchpad.net/bugs/1429756
|
|
|
721969b |
Cc: <stable@vger.kernel.org> # 3.19+
|
|
|
721969b |
Reported-by: Andy Whitcroft <apw@canonical.com>
|
|
|
721969b |
Reported-by: H.J. Lu <hjl.tools@gmail.com>
|
|
|
721969b |
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
|
|
|
721969b |
---
|
|
|
721969b |
Documentation/serial/tty.txt | 3 +++
|
|
|
721969b |
drivers/tty/n_hdlc.c | 4 ++--
|
|
|
721969b |
drivers/tty/n_tty.c | 22 ++++++++++++++++++----
|
|
|
721969b |
drivers/tty/pty.c | 5 +++--
|
|
|
721969b |
drivers/tty/tty_buffer.c | 41 +++++++++++++++++++++++++++--------------
|
|
|
721969b |
include/linux/tty.h | 2 +-
|
|
|
721969b |
6 files changed, 54 insertions(+), 23 deletions(-)
|
|
|
721969b |
|
|
|
721969b |
diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
|
|
|
721969b |
index 1e52d67d0abf..dbe6623fed1c 100644
|
|
|
721969b |
--- a/Documentation/serial/tty.txt
|
|
|
721969b |
+++ b/Documentation/serial/tty.txt
|
|
|
721969b |
@@ -198,6 +198,9 @@ TTY_IO_ERROR If set, causes all subsequent userspace read/write
|
|
|
721969b |
|
|
|
721969b |
TTY_OTHER_CLOSED Device is a pty and the other side has closed.
|
|
|
721969b |
|
|
|
721969b |
+TTY_OTHER_DONE Device is a pty and the other side has closed and
|
|
|
721969b |
+ all pending input processing has been completed.
|
|
|
721969b |
+
|
|
|
721969b |
TTY_NO_WRITE_SPLIT Prevent driver from splitting up writes into
|
|
|
721969b |
smaller chunks.
|
|
|
721969b |
|
|
|
721969b |
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
|
|
|
721969b |
index 644ddb841d9f..bbc4ce66c2c1 100644
|
|
|
721969b |
--- a/drivers/tty/n_hdlc.c
|
|
|
721969b |
+++ b/drivers/tty/n_hdlc.c
|
|
|
721969b |
@@ -600,7 +600,7 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
|
|
|
721969b |
add_wait_queue(&tty->read_wait, &wait);
|
|
|
721969b |
|
|
|
721969b |
for (;;) {
|
|
|
721969b |
- if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
|
|
|
721969b |
+ if (test_bit(TTY_OTHER_DONE, &tty->flags)) {
|
|
|
721969b |
ret = -EIO;
|
|
|
721969b |
break;
|
|
|
721969b |
}
|
|
|
721969b |
@@ -828,7 +828,7 @@ static unsigned int n_hdlc_tty_poll(struct tty_struct *tty, struct file *filp,
|
|
|
721969b |
/* set bits for operations that won't block */
|
|
|
721969b |
if (n_hdlc->rx_buf_list.head)
|
|
|
721969b |
mask |= POLLIN | POLLRDNORM; /* readable */
|
|
|
721969b |
- if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
|
|
|
721969b |
+ if (test_bit(TTY_OTHER_DONE, &tty->flags))
|
|
|
721969b |
mask |= POLLHUP;
|
|
|
721969b |
if (tty_hung_up_p(filp))
|
|
|
721969b |
mask |= POLLHUP;
|
|
|
721969b |
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
|
|
|
721969b |
index cf6e0f2e1331..cc57a3a6b02b 100644
|
|
|
721969b |
--- a/drivers/tty/n_tty.c
|
|
|
721969b |
+++ b/drivers/tty/n_tty.c
|
|
|
721969b |
@@ -1949,6 +1949,18 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
|
|
|
721969b |
return ldata->commit_head - ldata->read_tail >= amt;
|
|
|
721969b |
}
|
|
|
721969b |
|
|
|
721969b |
+static inline int check_other_done(struct tty_struct *tty)
|
|
|
721969b |
+{
|
|
|
721969b |
+ int done = test_bit(TTY_OTHER_DONE, &tty->flags);
|
|
|
721969b |
+ if (done) {
|
|
|
721969b |
+ /* paired with cmpxchg() in check_other_closed(); ensures
|
|
|
721969b |
+ * read buffer head index is not stale
|
|
|
721969b |
+ */
|
|
|
721969b |
+ smp_mb__after_atomic();
|
|
|
721969b |
+ }
|
|
|
721969b |
+ return done;
|
|
|
721969b |
+}
|
|
|
721969b |
+
|
|
|
721969b |
/**
|
|
|
721969b |
* copy_from_read_buf - copy read data directly
|
|
|
721969b |
* @tty: terminal device
|
|
|
721969b |
@@ -2167,7 +2179,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
|
|
|
721969b |
struct n_tty_data *ldata = tty->disc_data;
|
|
|
721969b |
unsigned char __user *b = buf;
|
|
|
721969b |
DEFINE_WAIT_FUNC(wait, woken_wake_function);
|
|
|
721969b |
- int c;
|
|
|
721969b |
+ int c, done;
|
|
|
721969b |
int minimum, time;
|
|
|
721969b |
ssize_t retval = 0;
|
|
|
721969b |
long timeout;
|
|
|
721969b |
@@ -2235,8 +2247,10 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
|
|
|
721969b |
((minimum - (b - buf)) >= 1))
|
|
|
721969b |
ldata->minimum_to_wake = (minimum - (b - buf));
|
|
|
721969b |
|
|
|
721969b |
+ done = check_other_done(tty);
|
|
|
721969b |
+
|
|
|
721969b |
if (!input_available_p(tty, 0)) {
|
|
|
721969b |
- if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
|
|
|
721969b |
+ if (done) {
|
|
|
721969b |
retval = -EIO;
|
|
|
721969b |
break;
|
|
|
721969b |
}
|
|
|
721969b |
@@ -2443,12 +2457,12 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
|
|
|
721969b |
|
|
|
721969b |
poll_wait(file, &tty->read_wait, wait);
|
|
|
721969b |
poll_wait(file, &tty->write_wait, wait);
|
|
|
721969b |
+ if (check_other_done(tty))
|
|
|
721969b |
+ mask |= POLLHUP;
|
|
|
721969b |
if (input_available_p(tty, 1))
|
|
|
721969b |
mask |= POLLIN | POLLRDNORM;
|
|
|
721969b |
if (tty->packet && tty->link->ctrl_status)
|
|
|
721969b |
mask |= POLLPRI | POLLIN | POLLRDNORM;
|
|
|
721969b |
- if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
|
|
|
721969b |
- mask |= POLLHUP;
|
|
|
721969b |
if (tty_hung_up_p(file))
|
|
|
721969b |
mask |= POLLHUP;
|
|
|
721969b |
if (!(mask & (POLLHUP | POLLIN | POLLRDNORM))) {
|
|
|
721969b |
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
|
|
|
721969b |
index e72ee629cead..4d5e8409769c 100644
|
|
|
721969b |
--- a/drivers/tty/pty.c
|
|
|
721969b |
+++ b/drivers/tty/pty.c
|
|
|
721969b |
@@ -53,9 +53,8 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
|
|
|
721969b |
/* Review - krefs on tty_link ?? */
|
|
|
721969b |
if (!tty->link)
|
|
|
721969b |
return;
|
|
|
721969b |
- tty_flush_to_ldisc(tty->link);
|
|
|
721969b |
set_bit(TTY_OTHER_CLOSED, &tty->link->flags);
|
|
|
721969b |
- wake_up_interruptible(&tty->link->read_wait);
|
|
|
721969b |
+ tty_flip_buffer_push(tty->link->port);
|
|
|
721969b |
wake_up_interruptible(&tty->link->write_wait);
|
|
|
721969b |
if (tty->driver->subtype == PTY_TYPE_MASTER) {
|
|
|
721969b |
set_bit(TTY_OTHER_CLOSED, &tty->flags);
|
|
|
721969b |
@@ -243,7 +242,9 @@ static int pty_open(struct tty_struct *tty, struct file *filp)
|
|
|
721969b |
goto out;
|
|
|
721969b |
|
|
|
721969b |
clear_bit(TTY_IO_ERROR, &tty->flags);
|
|
|
721969b |
+ /* TTY_OTHER_CLOSED must be cleared before TTY_OTHER_DONE */
|
|
|
721969b |
clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
|
|
|
721969b |
+ clear_bit(TTY_OTHER_DONE, &tty->link->flags);
|
|
|
721969b |
set_bit(TTY_THROTTLED, &tty->flags);
|
|
|
721969b |
return 0;
|
|
|
721969b |
|
|
|
721969b |
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
|
|
|
721969b |
index 75661641f5fe..2f78b77f0f81 100644
|
|
|
721969b |
--- a/drivers/tty/tty_buffer.c
|
|
|
721969b |
+++ b/drivers/tty/tty_buffer.c
|
|
|
721969b |
@@ -37,6 +37,28 @@
|
|
|
721969b |
|
|
|
721969b |
#define TTY_BUFFER_PAGE (((PAGE_SIZE - sizeof(struct tty_buffer)) / 2) & ~0xFF)
|
|
|
721969b |
|
|
|
721969b |
+/*
|
|
|
721969b |
+ * If all tty flip buffers have been processed by flush_to_ldisc() or
|
|
|
721969b |
+ * dropped by tty_buffer_flush(), check if the linked pty has been closed.
|
|
|
721969b |
+ * If so, wake the reader/poll to process
|
|
|
721969b |
+ */
|
|
|
721969b |
+static inline void check_other_closed(struct tty_struct *tty)
|
|
|
721969b |
+{
|
|
|
721969b |
+ unsigned long flags, old;
|
|
|
721969b |
+
|
|
|
721969b |
+ /* transition from TTY_OTHER_CLOSED => TTY_OTHER_DONE must be atomic */
|
|
|
721969b |
+ for (flags = ACCESS_ONCE(tty->flags);
|
|
|
721969b |
+ test_bit(TTY_OTHER_CLOSED, &flags);
|
|
|
721969b |
+ ) {
|
|
|
721969b |
+ old = flags;
|
|
|
721969b |
+ __set_bit(TTY_OTHER_DONE, &flags);
|
|
|
721969b |
+ flags = cmpxchg(&tty->flags, old, flags);
|
|
|
721969b |
+ if (old == flags) {
|
|
|
721969b |
+ wake_up_interruptible(&tty->read_wait);
|
|
|
721969b |
+ break;
|
|
|
721969b |
+ }
|
|
|
721969b |
+ }
|
|
|
721969b |
+}
|
|
|
721969b |
|
|
|
721969b |
/**
|
|
|
721969b |
* tty_buffer_lock_exclusive - gain exclusive access to buffer
|
|
|
721969b |
@@ -229,6 +251,8 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
|
|
|
721969b |
if (ld && ld->ops->flush_buffer)
|
|
|
721969b |
ld->ops->flush_buffer(tty);
|
|
|
721969b |
|
|
|
721969b |
+ check_other_closed(tty);
|
|
|
721969b |
+
|
|
|
721969b |
atomic_dec(&buf->priority);
|
|
|
721969b |
mutex_unlock(&buf->lock);
|
|
|
721969b |
}
|
|
|
721969b |
@@ -471,8 +495,10 @@ static void flush_to_ldisc(struct work_struct *work)
|
|
|
721969b |
smp_rmb();
|
|
|
721969b |
count = head->commit - head->read;
|
|
|
721969b |
if (!count) {
|
|
|
721969b |
- if (next == NULL)
|
|
|
721969b |
+ if (next == NULL) {
|
|
|
721969b |
+ check_other_closed(tty);
|
|
|
721969b |
break;
|
|
|
721969b |
+ }
|
|
|
721969b |
buf->head = next;
|
|
|
721969b |
tty_buffer_free(port, head);
|
|
|
721969b |
continue;
|
|
|
721969b |
@@ -489,19 +515,6 @@ static void flush_to_ldisc(struct work_struct *work)
|
|
|
721969b |
}
|
|
|
721969b |
|
|
|
721969b |
/**
|
|
|
721969b |
- * tty_flush_to_ldisc
|
|
|
721969b |
- * @tty: tty to push
|
|
|
721969b |
- *
|
|
|
721969b |
- * Push the terminal flip buffers to the line discipline.
|
|
|
721969b |
- *
|
|
|
721969b |
- * Must not be called from IRQ context.
|
|
|
721969b |
- */
|
|
|
721969b |
-void tty_flush_to_ldisc(struct tty_struct *tty)
|
|
|
721969b |
-{
|
|
|
721969b |
- flush_work(&tty->port->buf.work);
|
|
|
721969b |
-}
|
|
|
721969b |
-
|
|
|
721969b |
-/**
|
|
|
721969b |
* tty_flip_buffer_push - terminal
|
|
|
721969b |
* @port: tty port to push
|
|
|
721969b |
*
|
|
|
721969b |
diff --git a/include/linux/tty.h b/include/linux/tty.h
|
|
|
721969b |
index 358a337af598..790752ac074a 100644
|
|
|
721969b |
--- a/include/linux/tty.h
|
|
|
721969b |
+++ b/include/linux/tty.h
|
|
|
721969b |
@@ -339,6 +339,7 @@ struct tty_file_private {
|
|
|
721969b |
#define TTY_EXCLUSIVE 3 /* Exclusive open mode */
|
|
|
721969b |
#define TTY_DEBUG 4 /* Debugging */
|
|
|
721969b |
#define TTY_DO_WRITE_WAKEUP 5 /* Call write_wakeup after queuing new */
|
|
|
721969b |
+#define TTY_OTHER_DONE 6 /* Closed pty has completed input processing */
|
|
|
721969b |
#define TTY_LDISC_OPEN 11 /* Line discipline is open */
|
|
|
721969b |
#define TTY_PTY_LOCK 16 /* pty private */
|
|
|
721969b |
#define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver */
|
|
|
721969b |
@@ -462,7 +463,6 @@ extern int tty_hung_up_p(struct file *filp);
|
|
|
721969b |
extern void do_SAK(struct tty_struct *tty);
|
|
|
721969b |
extern void __do_SAK(struct tty_struct *tty);
|
|
|
721969b |
extern void no_tty(void);
|
|
|
721969b |
-extern void tty_flush_to_ldisc(struct tty_struct *tty);
|
|
|
721969b |
extern void tty_buffer_free_all(struct tty_port *port);
|
|
|
721969b |
extern void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld);
|
|
|
721969b |
extern void tty_buffer_init(struct tty_port *port);
|