MFC r183565:

  Small cleanups to openpty().

  - Pass O_NOCTTY to posix_openpt(2). This makes the implementation work
    consistently on implementations that make the PTY the controlling TTY
    by default.

  - Call unlockpt() before opening the slave device. POSIX mentions that
    de slave device should only be opened after grantpt() and unlockpt()
    have been called.

  - Replace some redundant code by a label.

As a safety net, add a call to revoke() to unlockpt(). All applications
out there use openpty(), explicitly call revoke() or implement their own
PTY allocation routines. Adding the call to unlockpt() won't hurt, but
will prevent foot-shooting.

Reviewed by:	jhb, kib
Approved by:	re

3 files changed, 25 insertions, 32 deletions

diff --git a/lib/libc/stdlib/grantpt.3 b/lib/libc/stdlib/grantpt.3
index b4ad8c4edf41..c411b8b1b55f 100644
--- a/lib/libc/stdlib/grantpt.3
+++ b/lib/libc/stdlib/grantpt.3
@@ -212,11 +212,6 @@ and
 functions appeared in
 .Fx 5.0 .
 .Sh NOTES
-The purpose of the
-.Fn unlockpt
-function has no meaning in
-.Fx .
-.Pp
 The flag
 .Dv O_NOCTTY
 is included for compatibility; in
diff --git a/lib/libc/stdlib/grantpt.c b/lib/libc/stdlib/grantpt.c
index 0aa458f5da43..a0e119d4942c 100644
--- a/lib/libc/stdlib/grantpt.c
+++ b/lib/libc/stdlib/grantpt.c
@@ -237,14 +237,20 @@ invalid:
 int
 unlockpt(int fildes)
 {
+	const char *slave;
 
 	/*
-	 * Unlocking a master/slave pseudo-terminal pair has no meaning in a
-	 * non-streams PTY environment.  However, we do ensure fildes is a
-	 * valid master pseudo-terminal device.
+	 * Even though unlocking a PTY has no meaning in a non-streams
+	 * PTY environment, make this function call revoke() to ensure
+	 * the PTY slave device is not being evesdropped.
 	 */
-	if (ptsname(fildes) == NULL)
+	if ((slave = ptsname(fildes)) == NULL)
 		return (-1);
 
+	if (revoke(slave) == -1) {
+		errno = EINVAL;
+		return (-1);
+	}
+
 	return (0);
 }
diff --git a/lib/libutil/pty.c b/lib/libutil/pty.c
index 4e16b001616e..77d8d4b95d09 100644
--- a/lib/libutil/pty.c
+++ b/lib/libutil/pty.c
@@ -60,37 +60,26 @@ openpty(int *amaster, int *aslave, char *name, struct termios *termp,
 	const char *slavename;
 	int master, slave;
 
-	master = posix_openpt(O_RDWR);
+	master = posix_openpt(O_RDWR|O_NOCTTY);
 	if (master == -1)
 		return (-1);
 
-	if (grantpt(master) == -1) {
-		close(master);
-		return (-1);
-	}
+	if (grantpt(master) == -1)
+		goto bad;
+
+	if (unlockpt(master) == -1)
+		goto bad;
 
 	slavename = ptsname(master);
-	if (slavename == NULL) {
-		close(master);
-		return (-1);
-	}
+	if (slavename == NULL)
+		goto bad;
 
-	if (revoke(slavename) == -1) {
-		close(master);
-		return (-1);
-	}
+	if (revoke(slavename) == -1)
+		goto bad;
 
 	slave = open(slavename, O_RDWR);
-	if (slave == -1) {
-		close(master);
-		return (-1);
-	}
-
-	if (unlockpt(master) == -1) {
-		close(master);
-		close(slave);
-		return (-1);
-	}
+	if (slave == -1)
+		goto bad;
 
 	*amaster = master;
 	*aslave = slave;
@@ -103,6 +92,9 @@ openpty(int *amaster, int *aslave, char *name, struct termios *termp,
 		ioctl(slave, TIOCSWINSZ, (char *)winp);
 
 	return (0);
+
+bad:	close(master);
+	return (-1);
 }
 
 int