From owner-svn-src-all@FreeBSD.ORG Sun Oct 20 20:50:18 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 5370383F; Sun, 20 Oct 2013 20:50:18 +0000 (UTC) (envelope-from jilles@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 25C882EDE; Sun, 20 Oct 2013 20:50:18 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id r9KKoIsk062746; Sun, 20 Oct 2013 20:50:18 GMT (envelope-from jilles@svn.freebsd.org) Received: (from jilles@localhost) by svn.freebsd.org (8.14.7/8.14.5/Submit) id r9KKoIVG062745; Sun, 20 Oct 2013 20:50:18 GMT (envelope-from jilles@svn.freebsd.org) Message-Id: <201310202050.r9KKoIVG062745@svn.freebsd.org> From: Jilles Tjoelker Date: Sun, 20 Oct 2013 20:50:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r256802 - head/lib/libc/gen X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 20 Oct 2013 20:50:18 -0000 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); }