From e9dec7758d3372e48746d32ba8c485cf89d83802 Mon Sep 17 00:00:00 2001
From: Jilles Tjoelker <jilles@FreeBSD.org>
Date: Mon, 20 May 2013 17:31:18 +0000
Subject: [PATCH] popen(): Add 'e' mode character to set close-on-exec on the
 new fd.

If 'e' is used, the kernel must support the recently added pipe2() system
call.

The use of pipe2() with O_CLOEXEC also fixes race conditions between
concurrent popen() calls from different threads, even if the close-on-exec
flag on the fd of the returned FILE is later cleared (because popen() closes
all file descriptors from earlier popen() calls in the child process).
Therefore, this approach should be used in all cases when pipe2() can be
assumed present.

The old version of popen() rejects "re" and "we" but treats "r+e" like "r+".
---
 lib/libc/gen/popen.3                       |  7 ++++-
 lib/libc/gen/popen.c                       | 30 +++++++++++++++-------
 tools/regression/lib/libc/gen/test-popen.c |  8 +++---
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lib/libc/gen/popen.3 b/lib/libc/gen/popen.3
index 90297e46f728..386a0bdf9857 100644
--- a/lib/libc/gen/popen.3
+++ b/lib/libc/gen/popen.3
@@ -28,7 +28,7 @@
 .\"     @(#)popen.3	8.2 (Berkeley) 5/3/95
 .\" $FreeBSD$
 .\"
-.Dd May 3, 1995
+.Dd May 20, 2013
 .Dt POPEN 3
 .Os
 .Sh NAME
@@ -79,6 +79,11 @@ for writing, or
 .Ql r+
 for reading and writing.
 .Pp
+A letter
+.Ql e
+may be appended to that to request that the underlying file descriptor
+be set close-on-exec.
+.Pp
 The
 .Fa command
 argument is a pointer to a null-terminated string
diff --git a/lib/libc/gen/popen.c b/lib/libc/gen/popen.c
index b123234837ee..b0597c80783f 100644
--- a/lib/libc/gen/popen.c
+++ b/lib/libc/gen/popen.c
@@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$");
 
 #include <signal.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -71,10 +72,11 @@ popen(command, type)
 {
 	struct pid *cur;
 	FILE *iop;
-	int pdes[2], pid, twoway;
+	int pdes[2], pid, twoway, cloexec;
 	char *argv[4];
 	struct pid *p;
 
+	cloexec = strchr(type, 'e') != NULL;
 	/*
 	 * Lite2 introduced two-way popen() pipes using _socketpair().
 	 * FreeBSD's pipe() is bidirectional, so we use that.
@@ -84,10 +86,11 @@ popen(command, type)
 		type = "r+";
 	} else  {
 		twoway = 0;
-		if ((*type != 'r' && *type != 'w') || type[1])
+		if ((*type != 'r' && *type != 'w') ||
+		    (type[1] && (type[1] != 'e' || type[2])))
 			return (NULL);
 	}
-	if (pipe(pdes) < 0)
+	if ((cloexec ? pipe2(pdes, O_CLOEXEC) : pipe(pdes)) < 0)
 		return (NULL);
 
 	if ((cur = malloc(sizeof(struct pid))) == NULL) {
@@ -120,20 +123,29 @@ popen(command, type)
 			 * the compiler is free to corrupt all the local
 			 * variables.
 			 */
-			(void)_close(pdes[0]);
+			if (!cloexec)
+				(void)_close(pdes[0]);
 			if (pdes[1] != STDOUT_FILENO) {
 				(void)_dup2(pdes[1], STDOUT_FILENO);
-				(void)_close(pdes[1]);
+				if (!cloexec)
+					(void)_close(pdes[1]);
 				if (twoway)
 					(void)_dup2(STDOUT_FILENO, STDIN_FILENO);
-			} else if (twoway && (pdes[1] != STDIN_FILENO))
+			} else if (twoway && (pdes[1] != STDIN_FILENO)) {
 				(void)_dup2(pdes[1], STDIN_FILENO);
+				if (cloexec)
+					(void)_fcntl(pdes[1], F_SETFD, 0);
+			} else if (cloexec)
+				(void)_fcntl(pdes[1], F_SETFD, 0);
 		} else {
 			if (pdes[0] != STDIN_FILENO) {
 				(void)_dup2(pdes[0], STDIN_FILENO);
-				(void)_close(pdes[0]);
-			}
-			(void)_close(pdes[1]);
+				if (!cloexec)
+					(void)_close(pdes[0]);
+			} else if (cloexec)
+				(void)_fcntl(pdes[0], F_SETFD, 0);
+			if (!cloexec)
+				(void)_close(pdes[1]);
 		}
 		SLIST_FOREACH(p, &pidlist, next)
 			(void)_close(fileno(p->fp));
diff --git a/tools/regression/lib/libc/gen/test-popen.c b/tools/regression/lib/libc/gen/test-popen.c
index b1ae295aa125..bf301d2f970c 100644
--- a/tools/regression/lib/libc/gen/test-popen.c
+++ b/tools/regression/lib/libc/gen/test-popen.c
@@ -70,10 +70,10 @@ main(int argc, char *argv[])
 	FILE *fp, *fp2;
 	int i, j, status;
 	const char *mode;
-	const char *allmodes[] = { "r", "w", "r+" };
-	const char *rmodes[] = { "r", "r+" };
-	const char *wmodes[] = { "w", "r+" };
-	const char *rwmodes[] = { "r+" };
+	const char *allmodes[] = { "r", "w", "r+", "re", "we", "r+e", "re+" };
+	const char *rmodes[] = { "r", "r+", "re", "r+e", "re+" };
+	const char *wmodes[] = { "w", "r+", "we", "r+e", "re+" };
+	const char *rwmodes[] = { "r+", "r+e", "re+" };
 	char buf[80];
 	struct sigaction act, oact;