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 <[email protected]>
  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;
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to