linux-kernel - [PATCH 3.16 038/305] Fix OpenSSH pty regression on close

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1471110171.826409436@decadent.org.uk>
Date:	Sat, 13 Aug 2016 18:42:51 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:	akpm@...ux-foundation.org,
	"Brian Bloniarz" <brian.bloniarz@...il.com>,
	"Peter Hurley" <peter@...leysoftware.com>,
	"Volth" <openssh@...th.com>,
	"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
	"Marc Aurele La France" <tsi@...oix.net>
Subject: [PATCH 3.16 038/305] Fix OpenSSH pty regression on close

3.16.37-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Brian Bloniarz <brian.bloniarz@...il.com>

commit 0f40fbbcc34e093255a2b2d70b6b0fb48c3f39aa upstream.

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@...oix.net>

Reported-by: Volth <openssh@...th.com>
Reported-by: Marc Aurele La France <tsi@...oix.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@...il.com>
Reviewed-by: Peter Hurley <peter@...leysoftware.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
[bwh: Backported to 3.16:
 - No need to unwind commits 2 and 3
 - Keep using tty_flush_to_ldisc() rather than adding tty_buffer_flush_work()]]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -2251,15 +2251,14 @@ static ssize_t n_tty_read(struct tty_str
 			ldata->minimum_to_wake = (minimum - (b - buf));
 
 		if (!input_available_p(tty, 0)) {
-			if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
-				up_read(&tty->termios_rwsem);
-				tty_flush_to_ldisc(tty);
-				down_read(&tty->termios_rwsem);
-				if (!input_available_p(tty, 0)) {
+			up_read(&tty->termios_rwsem);
+			tty_flush_to_ldisc(tty);
+			down_read(&tty->termios_rwsem);
+			if (!input_available_p(tty, 0)) {
+				if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
 					retval = -EIO;
 					break;
 				}
-			} else {
 				if (tty_hung_up_p(file))
 					break;
 				if (!timeout)
@@ -2467,6 +2466,11 @@ static unsigned int n_tty_poll(struct tt
 	poll_wait(file, &tty->write_wait, wait);
 	if (input_available_p(tty, 1))
 		mask |= POLLIN | POLLRDNORM;
+	else {
+		tty_flush_to_ldisc(tty);
+		if (input_available_p(tty, 1))
+			mask |= POLLIN | POLLRDNORM;
+	}
 	if (tty->packet && tty->link->ctrl_status)
 		mask |= POLLPRI | POLLIN | POLLRDNORM;
 	if (test_bit(TTY_OTHER_CLOSED, &tty->flags))