Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Fix OpenSSH pty regression on close

OpenSSH expects the (non-blocking) read() of pty master to return
EAGAIN only if it has received all of the slave-side output after
it has received SIGCHLD. This used to work on pre-3.12 kernels.

This fix effectively forces non-blocking read() and poll() to
block for parallel i/o to complete for all ttys. It also unwinds
these changes:

1) f8747d4a466ab2cafe56112c51b3379f9fdb7a12
tty: Fix pty master read() after slave closes

2) 52bce7f8d4fc633c9a9d0646eef58ba6ae9a3b73
pty, n_tty: Simplify input processing on final close

3) 1a48632ffed61352a7810ce089dc5a8bcd505a60
pty: Fix input race when closing

Inspired by analysis and patch from Marc Aurele La France <tsi@tuyoix.net>

Reported-by: Volth <openssh@volth.com>
Reported-by: Marc Aurele La France <tsi@tuyoix.net>
BugLink: https://bugzilla.mindrot.org/show_bug.cgi?id=52
BugLink: https://bugzilla.mindrot.org/show_bug.cgi?id=2492
Signed-off-by: Brian Bloniarz <brian.bloniarz@gmail.com>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Brian Bloniarz and committed by
Greg Kroah-Hartman
0f40fbbc d11df618

+44 -75
-3
Documentation/serial/tty.txt
··· 210 210 211 211 TTY_OTHER_CLOSED Device is a pty and the other side has closed. 212 212 213 - TTY_OTHER_DONE Device is a pty and the other side has closed and 214 - all pending input processing has been completed. 215 - 216 213 TTY_NO_WRITE_SPLIT Prevent driver from splitting up writes into 217 214 smaller chunks. 218 215
+2 -2
drivers/tty/n_hdlc.c
··· 599 599 add_wait_queue(&tty->read_wait, &wait); 600 600 601 601 for (;;) { 602 - if (test_bit(TTY_OTHER_DONE, &tty->flags)) { 602 + if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) { 603 603 ret = -EIO; 604 604 break; 605 605 } ··· 827 827 /* set bits for operations that won't block */ 828 828 if (n_hdlc->rx_buf_list.head) 829 829 mask |= POLLIN | POLLRDNORM; /* readable */ 830 - if (test_bit(TTY_OTHER_DONE, &tty->flags)) 830 + if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) 831 831 mask |= POLLHUP; 832 832 if (tty_hung_up_p(filp)) 833 833 mask |= POLLHUP;
+34 -38
drivers/tty/n_tty.c
··· 1917 1917 return ldata->commit_head - ldata->read_tail >= amt; 1918 1918 } 1919 1919 1920 - static inline int check_other_done(struct tty_struct *tty) 1921 - { 1922 - int done = test_bit(TTY_OTHER_DONE, &tty->flags); 1923 - if (done) { 1924 - /* paired with cmpxchg() in check_other_closed(); ensures 1925 - * read buffer head index is not stale 1926 - */ 1927 - smp_mb__after_atomic(); 1928 - } 1929 - return done; 1930 - } 1931 - 1932 1920 /** 1933 1921 * copy_from_read_buf - copy read data directly 1934 1922 * @tty: terminal device ··· 2112 2124 struct n_tty_data *ldata = tty->disc_data; 2113 2125 unsigned char __user *b = buf; 2114 2126 DEFINE_WAIT_FUNC(wait, woken_wake_function); 2115 - int c, done; 2127 + int c; 2116 2128 int minimum, time; 2117 2129 ssize_t retval = 0; 2118 2130 long timeout; ··· 2171 2183 break; 2172 2184 } 2173 2185 2174 - done = check_other_done(tty); 2175 - 2176 2186 if (!input_available_p(tty, 0)) { 2177 - if (done) { 2178 - retval = -EIO; 2179 - break; 2180 - } 2181 - if (tty_hung_up_p(file)) 2182 - break; 2183 - if (!timeout) 2184 - break; 2185 - if (file->f_flags & O_NONBLOCK) { 2186 - retval = -EAGAIN; 2187 - break; 2188 - } 2189 - if (signal_pending(current)) { 2190 - retval = -ERESTARTSYS; 2191 - break; 2192 - } 2193 2187 up_read(&tty->termios_rwsem); 2194 - 2195 - timeout = wait_woken(&wait, TASK_INTERRUPTIBLE, 2196 - timeout); 2197 - 2188 + tty_buffer_flush_work(tty->port); 2198 2189 down_read(&tty->termios_rwsem); 2199 - continue; 2190 + if (!input_available_p(tty, 0)) { 2191 + if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) { 2192 + retval = -EIO; 2193 + break; 2194 + } 2195 + if (tty_hung_up_p(file)) 2196 + break; 2197 + if (!timeout) 2198 + break; 2199 + if (file->f_flags & O_NONBLOCK) { 2200 + retval = -EAGAIN; 2201 + break; 2202 + } 2203 + if (signal_pending(current)) { 2204 + retval = -ERESTARTSYS; 2205 + break; 2206 + } 2207 + up_read(&tty->termios_rwsem); 2208 + 2209 + timeout = wait_woken(&wait, TASK_INTERRUPTIBLE, 2210 + timeout); 2211 + 2212 + down_read(&tty->termios_rwsem); 2213 + continue; 2214 + } 2200 2215 } 2201 2216 2202 2217 if (ldata->icanon && !L_EXTPROC(tty)) { ··· 2377 2386 2378 2387 poll_wait(file, &tty->read_wait, wait); 2379 2388 poll_wait(file, &tty->write_wait, wait); 2380 - if (check_other_done(tty)) 2381 - mask |= POLLHUP; 2382 2389 if (input_available_p(tty, 1)) 2383 2390 mask |= POLLIN | POLLRDNORM; 2391 + else { 2392 + tty_buffer_flush_work(tty->port); 2393 + if (input_available_p(tty, 1)) 2394 + mask |= POLLIN | POLLRDNORM; 2395 + } 2384 2396 if (tty->packet && tty->link->ctrl_status) 2385 2397 mask |= POLLPRI | POLLIN | POLLRDNORM; 2398 + if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) 2399 + mask |= POLLHUP; 2386 2400 if (tty_hung_up_p(file)) 2387 2401 mask |= POLLHUP; 2388 2402 if (tty->ops->write && !tty_is_writelocked(tty) &&
+1 -3
drivers/tty/pty.c
··· 59 59 if (!tty->link) 60 60 return; 61 61 set_bit(TTY_OTHER_CLOSED, &tty->link->flags); 62 - tty_flip_buffer_push(tty->link->port); 62 + wake_up_interruptible(&tty->link->read_wait); 63 63 wake_up_interruptible(&tty->link->write_wait); 64 64 if (tty->driver->subtype == PTY_TYPE_MASTER) { 65 65 set_bit(TTY_OTHER_CLOSED, &tty->flags); ··· 247 247 goto out; 248 248 249 249 clear_bit(TTY_IO_ERROR, &tty->flags); 250 - /* TTY_OTHER_CLOSED must be cleared before TTY_OTHER_DONE */ 251 250 clear_bit(TTY_OTHER_CLOSED, &tty->link->flags); 252 - clear_bit(TTY_OTHER_DONE, &tty->link->flags); 253 251 set_bit(TTY_THROTTLED, &tty->flags); 254 252 return 0; 255 253
+6 -28
drivers/tty/tty_buffer.c
··· 37 37 38 38 #define TTY_BUFFER_PAGE (((PAGE_SIZE - sizeof(struct tty_buffer)) / 2) & ~0xFF) 39 39 40 - /* 41 - * If all tty flip buffers have been processed by flush_to_ldisc() or 42 - * dropped by tty_buffer_flush(), check if the linked pty has been closed. 43 - * If so, wake the reader/poll to process 44 - */ 45 - static inline void check_other_closed(struct tty_struct *tty) 46 - { 47 - unsigned long flags, old; 48 - 49 - /* transition from TTY_OTHER_CLOSED => TTY_OTHER_DONE must be atomic */ 50 - for (flags = ACCESS_ONCE(tty->flags); 51 - test_bit(TTY_OTHER_CLOSED, &flags); 52 - ) { 53 - old = flags; 54 - __set_bit(TTY_OTHER_DONE, &flags); 55 - flags = cmpxchg(&tty->flags, old, flags); 56 - if (old == flags) { 57 - wake_up_interruptible(&tty->read_wait); 58 - break; 59 - } 60 - } 61 - } 62 - 63 40 /** 64 41 * tty_buffer_lock_exclusive - gain exclusive access to buffer 65 42 * tty_buffer_unlock_exclusive - release exclusive access ··· 230 253 231 254 if (ld && ld->ops->flush_buffer) 232 255 ld->ops->flush_buffer(tty); 233 - 234 - check_other_closed(tty); 235 256 236 257 atomic_dec(&buf->priority); 237 258 mutex_unlock(&buf->lock); ··· 497 522 */ 498 523 count = smp_load_acquire(&head->commit) - head->read; 499 524 if (!count) { 500 - if (next == NULL) { 501 - check_other_closed(tty); 525 + if (next == NULL) 502 526 break; 503 - } 504 527 buf->head = next; 505 528 tty_buffer_free(port, head); 506 529 continue; ··· 586 613 bool tty_buffer_cancel_work(struct tty_port *port) 587 614 { 588 615 return cancel_work_sync(&port->buf.work); 616 + } 617 + 618 + void tty_buffer_flush_work(struct tty_port *port) 619 + { 620 + flush_work(&port->buf.work); 589 621 }
+1 -1
include/linux/tty.h
··· 351 351 #define TTY_OTHER_CLOSED 2 /* Other side (if any) has closed */ 352 352 #define TTY_EXCLUSIVE 3 /* Exclusive open mode */ 353 353 #define TTY_DO_WRITE_WAKEUP 5 /* Call write_wakeup after queuing new */ 354 - #define TTY_OTHER_DONE 6 /* Closed pty has completed input processing */ 355 354 #define TTY_LDISC_OPEN 11 /* Line discipline is open */ 356 355 #define TTY_PTY_LOCK 16 /* pty private */ 357 356 #define TTY_NO_WRITE_SPLIT 17 /* Preserve write boundaries to driver */ ··· 479 480 extern void tty_buffer_set_lock_subclass(struct tty_port *port); 480 481 extern bool tty_buffer_restart_work(struct tty_port *port); 481 482 extern bool tty_buffer_cancel_work(struct tty_port *port); 483 + extern void tty_buffer_flush_work(struct tty_port *port); 482 484 extern speed_t tty_termios_baud_rate(struct ktermios *termios); 483 485 extern speed_t tty_termios_input_baud_rate(struct ktermios *termios); 484 486 extern void tty_termios_encode_baud_rate(struct ktermios *termios,