Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 9 Jan 2016 05:05:15 +0000 (UTC)
From:      Ravi Pokala <rpokala@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r293463 - stable/10/lib/libc/gen
Message-ID:  <201601090505.u0955FYL030225@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rpokala
Date: Sat Jan  9 05:05:15 2016
New Revision: 293463
URL: https://svnweb.freebsd.org/changeset/base/293463

Log:
  MFC r291114: popen() requires check for fdopen() failure
  
  Move fdopen() up near other resource allocation like malloc(); do proper
  deallocation on failure later on in the function.
  
  Approved by:	jhb
  Sponsored by:	Panasas, Inc.

Modified:
  stable/10/lib/libc/gen/popen.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/lib/libc/gen/popen.c
==============================================================================
--- stable/10/lib/libc/gen/popen.c	Sat Jan  9 05:02:35 2016	(r293462)
+++ stable/10/lib/libc/gen/popen.c	Sat Jan  9 05:05:15 2016	(r293463)
@@ -73,6 +73,7 @@ popen(command, type)
 	struct pid *cur;
 	FILE *iop;
 	int pdes[2], pid, twoway, cloexec;
+	int pdes_unused_in_parent;
 	char *argv[4];
 	struct pid *p;
 
@@ -99,6 +100,20 @@ popen(command, type)
 		return (NULL);
 	}
 
+	if (*type == 'r') {
+		iop = fdopen(pdes[0], type);
+		pdes_unused_in_parent = pdes[1];
+	} else {
+		iop = fdopen(pdes[1], type);
+		pdes_unused_in_parent = pdes[0];
+	}
+	if (iop == NULL) {
+		(void)_close(pdes[0]);
+		(void)_close(pdes[1]);
+		free(cur);
+		return (NULL);
+	}
+
 	argv[0] = "sh";
 	argv[1] = "-c";
 	argv[2] = (char *)command;
@@ -108,9 +123,14 @@ popen(command, type)
 	switch (pid = vfork()) {
 	case -1:			/* Error. */
 		THREAD_UNLOCK();
-		(void)_close(pdes[0]);
-		(void)_close(pdes[1]);
+		/*
+		 * The _close() closes the unused end of pdes[], while
+		 * the fclose() closes the used end of pdes[], *and* cleans
+		 * up iop.
+		 */
+		(void)_close(pdes_unused_in_parent);
 		free(cur);
+		(void)fclose(iop);
 		return (NULL);
 		/* NOTREACHED */
 	case 0:				/* Child. */
@@ -155,14 +175,8 @@ popen(command, type)
 	}
 	THREAD_UNLOCK();
 
-	/* Parent; assume fdopen can't fail. */
-	if (*type == 'r') {
-		iop = fdopen(pdes[0], type);
-		(void)_close(pdes[1]);
-	} else {
-		iop = fdopen(pdes[1], type);
-		(void)_close(pdes[0]);
-	}
+	/* Parent. */
+	(void)_close(pdes_unused_in_parent);
 
 	/* Link into list of file descriptors. */
 	cur->fp = iop;



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