svn commit: r291114 - head/lib/libc/gen

[Ravi Pokala]

Author: rpokala
Date: Fri Nov 20 22:36:41 2015
New Revision: 291114
URL: https://svnweb.freebsd.org/changeset/base/291114

Log:
  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.
  
  Submitted by:	Ramachandra Topannavar <rtopannavar at panasas.com>
  Reviewed by:	jilles
  Approved by:	jhb (mentor)
  MFC after:	2 weeks
  Sponsored by:	Panasas, Inc.
  Differential Revision:	https://reviews.freebsd.org/D4126
  
  M    lib/libc/gen/popen.c

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

Modified: head/lib/libc/gen/popen.c
==============================================================================
--- head/lib/libc/gen/popen.c	Fri Nov 20 21:56:20 2015	(r291113)
+++ head/lib/libc/gen/popen.c	Fri Nov 20 22:36:41 2015	(r291114)
@@ -72,6 +72,7 @@ popen(const char *command, const char *t
 	struct pid *cur;
 	FILE *iop;
 	int pdes[2], pid, twoway, cloexec;
+	int pdes_unused_in_parent;
 	char *argv[4];
 	struct pid *p;
 
@@ -98,6 +99,20 @@ popen(const char *command, const char *t
 		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;
@@ -107,9 +122,14 @@ popen(const char *command, const char *t
 	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. */
@@ -145,14 +165,8 @@ popen(const char *command, const char *t
 	}
 	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;