Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 20 Oct 2013 20:50:17 +0000 (UTC)
From:      Jilles Tjoelker <jilles@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r256802 - head/lib/libc/gen
Message-ID:  <201310202050.r9KKoIVG062745@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jilles
Date: Sun Oct 20 20:50:17 2013
New Revision: 256802
URL: http://svnweb.freebsd.org/changeset/base/256802

Log:
  popen(): Try to prevent inappropriate fd passing even if 'e' is not used.
  
  Even though not all race conditions can be fixed if the 'e' option is not
  used, still fix some race conditions using pipe2():
  
  * Prevent both ends of the pipe from leaking to a concurrent popen().
  
  * Prevent the child process's end of the pipe from leaking to any concurrent
    fork and exec.
  
  This change also simplifies the code.

Modified:
  head/lib/libc/gen/popen.c

Modified: head/lib/libc/gen/popen.c
==============================================================================
--- head/lib/libc/gen/popen.c	Sun Oct 20 20:41:38 2013	(r256801)
+++ head/lib/libc/gen/popen.c	Sun Oct 20 20:50:17 2013	(r256802)
@@ -90,7 +90,7 @@ popen(command, type)
 		    (type[1] && (type[1] != 'e' || type[2])))
 			return (NULL);
 	}
-	if ((cloexec ? pipe2(pdes, O_CLOEXEC) : pipe(pdes)) < 0)
+	if (pipe2(pdes, O_CLOEXEC) < 0)
 		return (NULL);
 
 	if ((cur = malloc(sizeof(struct pid))) == NULL) {
@@ -123,29 +123,20 @@ popen(command, type)
 			 * the compiler is free to corrupt all the local
 			 * variables.
 			 */
-			if (!cloexec)
-				(void)_close(pdes[0]);
 			if (pdes[1] != STDOUT_FILENO) {
 				(void)_dup2(pdes[1], STDOUT_FILENO);
-				if (!cloexec)
-					(void)_close(pdes[1]);
 				if (twoway)
 					(void)_dup2(STDOUT_FILENO, 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
 				(void)_fcntl(pdes[1], F_SETFD, 0);
 		} else {
 			if (pdes[0] != STDIN_FILENO) {
 				(void)_dup2(pdes[0], STDIN_FILENO);
-				if (!cloexec)
-					(void)_close(pdes[0]);
-			} else if (cloexec)
+			} else
 				(void)_fcntl(pdes[0], F_SETFD, 0);
-			if (!cloexec)
-				(void)_close(pdes[1]);
 		}
 		SLIST_FOREACH(p, &pidlist, next)
 			(void)_close(fileno(p->fp));
@@ -171,6 +162,14 @@ popen(command, type)
 	SLIST_INSERT_HEAD(&pidlist, cur, next);
 	THREAD_UNLOCK();
 
+	/*
+	 * To guard against undesired fd passing with concurrent calls,
+	 * only clear the close-on-exec flag after linking the file into
+	 * the list which will cause an explicit close.
+	 */
+	if (!cloexec)
+		(void)_fcntl(*type == 'r' ? pdes[0] : pdes[1], F_SETFD, 0);
+
 	return (iop);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310202050.r9KKoIVG062745>