From 69d044c034dcc2b9af137a6d36f2a54d8944b8fc Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Fri, 21 Oct 2011 11:09:23 -0600
Subject: [PATCH] waitpid: improve safety

Based on a report by Coverity.  waitpid() can leak resources if it
fails with EINTR, so it should never be used without checking return
status.  But we already have a helper function that does that, so
use it in more places.

* src/lxc/lxc_container.c (lxcContainerAvailable): Use safer
virWaitPid.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput, virtTestMain):
Likewise.
* src/libvirt.c (virConnectAuthGainPolkit): Simplify with virCommand.
---
 daemon/libvirtd.c       |  6 +-----
 src/libvirt.c           | 35 ++++++++++++-----------------------
 src/lxc/lxc_container.c |  5 ++---
 tests/testutils.c       |  7 ++++---
 4 files changed, 19 insertions(+), 34 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 2f0e1be7df..5e1fc965e5 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -232,18 +232,14 @@ static int daemonForkIntoBackground(const char *argv0)
 
     default:
         {
-            int got, exitstatus = 0;
             int ret;
             char status;
 
             VIR_FORCE_CLOSE(statuspipe[1]);
 
             /* We wait to make sure the first child forked successfully */
-            if ((got = waitpid(pid, &exitstatus, 0)) < 0 ||
-                got != pid ||
-                exitstatus != 0) {
+            if (virPidWait(pid, NULL) < 0)
                 return -1;
-            }
 
             /* Now block until the second child initializes successfully */
         again:
diff --git a/src/libvirt.c b/src/libvirt.c
index 0b975daecc..a6bcee69b4 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -42,6 +42,7 @@
 #include "intprops.h"
 #include "conf.h"
 #include "rpc/virnettlscontext.h"
+#include "command.h"
 
 #ifndef WITH_DRIVER_MODULES
 # ifdef WITH_TEST
@@ -108,34 +109,22 @@ static int initialized = 0;
 
 #if defined(POLKIT_AUTH)
 static int virConnectAuthGainPolkit(const char *privilege) {
-    const char *const args[] = {
-        POLKIT_AUTH, "--obtain", privilege, NULL
-    };
-    int childpid, status, ret;
+    virCommandPtr cmd;
+    int status;
+    int ret = -1;
 
-    /* Root has all rights */
     if (getuid() == 0)
         return 0;
 
-    if ((childpid = fork()) < 0)
-        return -1;
+    cmd = virCommandNewArgList(POLKIT_AUTH, "--obtain", privilege, NULL);
+    if (virCommandRun(cmd, &status) < 0 ||
+        status > 1)
+        goto cleanup;
 
-    if (!childpid) {
-        execvp(args[0], (char **)args);
-        _exit(-1);
-    }
-
-    while ((ret = waitpid(childpid, &status, 0) == -1) && errno == EINTR);
-    if (ret == -1) {
-        return -1;
-    }
-
-    if (!WIFEXITED(status) ||
-        (WEXITSTATUS(status) != 0 && WEXITSTATUS(status) != 1)) {
-        return -1;
-    }
-
-    return 0;
+    ret = 0;
+cleanup:
+    virCommandFree(cmd);
+    return ret;
 }
 #endif
 
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index e9891f7255..06ccf7e2db 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1229,7 +1229,6 @@ int lxcContainerAvailable(int features)
     int cpid;
     char *childStack;
     char *stack;
-    int childStatus;
 
     if (features & LXC_CONTAINER_FEATURE_USER)
         flags |= CLONE_NEWUSER;
@@ -1251,8 +1250,8 @@ int lxcContainerAvailable(int features)
         VIR_DEBUG("clone call returned %s, container support is not enabled",
               virStrerror(errno, ebuf, sizeof ebuf));
         return -1;
-    } else {
-        waitpid(cpid, &childStatus, 0);
+    } else if (virPidWait(cpid, NULL) < 0) {
+        return -1;
     }
 
     VIR_DEBUG("Mounted all filesystems");
diff --git a/tests/testutils.c b/tests/testutils.c
index 00a8d8f8d0..acdfdc1b40 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -33,6 +33,7 @@
 #include "virterror_internal.h"
 #include "buf.h"
 #include "logging.h"
+#include "command.h"
 
 #if TEST_OOM_TRACE
 # include <execinfo.h>
@@ -309,7 +310,8 @@ virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen)
         VIR_FORCE_CLOSE(pipefd[1]);
         len = virFileReadLimFD(pipefd[0], maxlen, buf);
         VIR_FORCE_CLOSE(pipefd[0]);
-        waitpid(pid, NULL, 0);
+        if (virPidWait(pid, NULL) < 0)
+            return -1;
 
         return len;
     }
@@ -674,8 +676,7 @@ int virtTestMain(int argc,
             } else {
                 int i, status;
                 for (i = 0 ; i < mp ; i++) {
-                    waitpid(workers[i], &status, 0);
-                    if (WEXITSTATUS(status) != EXIT_SUCCESS)
+                    if (virPidWait(workers[i], NULL) < 0)
                         ret = EXIT_FAILURE;
                 }
                 VIR_FREE(workers);