linux-kernel - [PATCH -next v2 04/26] tty: Clarify re-open behavior of master ptys

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: <1415207589-15967-5-git-send-email-peter@hurleysoftware.com>
Date:	Wed,  5 Nov 2014 12:12:47 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Jiri Slaby <jslaby@...e.cz>,
	One Thousand Gnomes <gnomes@...rguk.ukuu.org.uk>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	Peter Hurley <peter@...leysoftware.com>
Subject: [PATCH -next v2 04/26] tty: Clarify re-open behavior of master ptys

Re-opening master ptys is not allowed. Once opened and for the remaining
lifetime of the master pty, its tty count is 1. If its tty count has
dropped to 0, then the master pty was closed and TTY_CLOSING was set,
and destruction may begin imminently.

Besides the normal case of a legacy BSD pty master being re-opened
(which always returns -EIO), this code is only reachable in 2 degenerate
cases:
1. The pty master is the controlling terminal (this is possible through
   the TIOCSCTTY ioctl). pty masters are not designed to be controlling
   terminals and it's an oversight that tiocsctty() ever let that happen.
   The attempted open of /dev/tty will always fail. No known program does
   this.
2. The legacy BSD pty slave was opened first. The slave open will fail
   in pty_open() and tty_release() will commence. But before tty_release()
   claims the tty_mutex, there is a very small window where a parallel
   master open might succeed. In a test of racing legacy BSD slave and
   master parallel opens, where:
      slave open attempts:  10000   success:4527  failure:5473
      master open attempts: 11728   success:5789  failure:5939
   only 8 master open attempts would have succeeded reaching this code and
   successfully opened the master pty. This case is not possible with
   SysV ptys.

Always return -EIO if a master pty is re-opened or the slave is opened
first and the master opened in parallel (for legacy BSD ptys).

Furthermore, now that changing the slave's count is not required,
the tty_lock is sufficient for preventing concurrent changes to the
tty being re-opened (or failing re-opening).

Reviewed-by: Alan Cox <alan@...ux.intel.com>
Signed-off-by: Peter Hurley <peter@...leysoftware.com>
---
 drivers/tty/tty_io.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index bb4d2f2..6dc3fa0 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1444,9 +1444,9 @@ void tty_driver_remove_tty(struct tty_driver *driver, struct tty_struct *tty)
  * 	@tty	- the tty to open
  *
  *	Return 0 on success, -errno on error.
+ *	Re-opens on master ptys are not allowed and return -EIO.
  *
- *	Locking: tty_mutex must be held from the time the tty was found
- *		 till this open completes.
+ *	Locking: Caller must hold tty_lock
  */
 static int tty_reopen(struct tty_struct *tty)
 {
@@ -1456,16 +1456,9 @@ static int tty_reopen(struct tty_struct *tty)
 		return -EIO;
 
 	if (driver->type == TTY_DRIVER_TYPE_PTY &&
-	    driver->subtype == PTY_TYPE_MASTER) {
-		/*
-		 * special case for PTY masters: only one open permitted,
-		 * and the slave side open count is incremented as well.
-		 */
-		if (tty->count)
-			return -EIO;
+	    driver->subtype == PTY_TYPE_MASTER)
+		return -EIO;
 
-		tty->link->count++;
-	}
 	tty->count++;
 
 	WARN_ON(!tty->ldisc);
-- 
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/