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.
This commit is contained in:
Eric Blake 2011-10-21 11:09:23 -06:00
parent 2c27dfaeb1
commit 69d044c034
4 changed files with 19 additions and 34 deletions

View File

@ -232,18 +232,14 @@ static int daemonForkIntoBackground(const char *argv0)
default: default:
{ {
int got, exitstatus = 0;
int ret; int ret;
char status; char status;
VIR_FORCE_CLOSE(statuspipe[1]); VIR_FORCE_CLOSE(statuspipe[1]);
/* We wait to make sure the first child forked successfully */ /* We wait to make sure the first child forked successfully */
if ((got = waitpid(pid, &exitstatus, 0)) < 0 || if (virPidWait(pid, NULL) < 0)
got != pid ||
exitstatus != 0) {
return -1; return -1;
}
/* Now block until the second child initializes successfully */ /* Now block until the second child initializes successfully */
again: again:

View File

@ -42,6 +42,7 @@
#include "intprops.h" #include "intprops.h"
#include "conf.h" #include "conf.h"
#include "rpc/virnettlscontext.h" #include "rpc/virnettlscontext.h"
#include "command.h"
#ifndef WITH_DRIVER_MODULES #ifndef WITH_DRIVER_MODULES
# ifdef WITH_TEST # ifdef WITH_TEST
@ -108,34 +109,22 @@ static int initialized = 0;
#if defined(POLKIT_AUTH) #if defined(POLKIT_AUTH)
static int virConnectAuthGainPolkit(const char *privilege) { static int virConnectAuthGainPolkit(const char *privilege) {
const char *const args[] = { virCommandPtr cmd;
POLKIT_AUTH, "--obtain", privilege, NULL int status;
}; int ret = -1;
int childpid, status, ret;
/* Root has all rights */
if (getuid() == 0) if (getuid() == 0)
return 0; return 0;
if ((childpid = fork()) < 0) cmd = virCommandNewArgList(POLKIT_AUTH, "--obtain", privilege, NULL);
return -1; if (virCommandRun(cmd, &status) < 0 ||
status > 1)
goto cleanup;
if (!childpid) { ret = 0;
execvp(args[0], (char **)args); cleanup:
_exit(-1); virCommandFree(cmd);
} return ret;
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;
} }
#endif #endif

View File

@ -1229,7 +1229,6 @@ int lxcContainerAvailable(int features)
int cpid; int cpid;
char *childStack; char *childStack;
char *stack; char *stack;
int childStatus;
if (features & LXC_CONTAINER_FEATURE_USER) if (features & LXC_CONTAINER_FEATURE_USER)
flags |= CLONE_NEWUSER; flags |= CLONE_NEWUSER;
@ -1251,8 +1250,8 @@ int lxcContainerAvailable(int features)
VIR_DEBUG("clone call returned %s, container support is not enabled", VIR_DEBUG("clone call returned %s, container support is not enabled",
virStrerror(errno, ebuf, sizeof ebuf)); virStrerror(errno, ebuf, sizeof ebuf));
return -1; return -1;
} else { } else if (virPidWait(cpid, NULL) < 0) {
waitpid(cpid, &childStatus, 0); return -1;
} }
VIR_DEBUG("Mounted all filesystems"); VIR_DEBUG("Mounted all filesystems");

View File

@ -33,6 +33,7 @@
#include "virterror_internal.h" #include "virterror_internal.h"
#include "buf.h" #include "buf.h"
#include "logging.h" #include "logging.h"
#include "command.h"
#if TEST_OOM_TRACE #if TEST_OOM_TRACE
# include <execinfo.h> # include <execinfo.h>
@ -309,7 +310,8 @@ virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen)
VIR_FORCE_CLOSE(pipefd[1]); VIR_FORCE_CLOSE(pipefd[1]);
len = virFileReadLimFD(pipefd[0], maxlen, buf); len = virFileReadLimFD(pipefd[0], maxlen, buf);
VIR_FORCE_CLOSE(pipefd[0]); VIR_FORCE_CLOSE(pipefd[0]);
waitpid(pid, NULL, 0); if (virPidWait(pid, NULL) < 0)
return -1;
return len; return len;
} }
@ -674,8 +676,7 @@ int virtTestMain(int argc,
} else { } else {
int i, status; int i, status;
for (i = 0 ; i < mp ; i++) { for (i = 0 ; i < mp ; i++) {
waitpid(workers[i], &status, 0); if (virPidWait(workers[i], NULL) < 0)
if (WEXITSTATUS(status) != EXIT_SUCCESS)
ret = EXIT_FAILURE; ret = EXIT_FAILURE;
} }
VIR_FREE(workers); VIR_FREE(workers);