[libvirt] [PATCH] virCommandExec: Report error if execve fails

In an unlikely event of execve() failing, the virCommandExec() function does not report any error, even though checks that are at the beginning of the function are verbose when failing. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 7 +++++- tests/commandtest.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index f5bd7af..3c67c90 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -2152,7 +2152,12 @@ int virCommandExec(virCommandPtr cmd) return -1; } - return execve(cmd->args[0], cmd->args, cmd->env); + execve(cmd->args[0], cmd->args, cmd->env); + + virReportSystemError(errno, + _("cannot execute binary %s"), + cmd->args[0]); + return -1; } #else int virCommandExec(virCommandPtr cmd ATTRIBUTE_UNUSED) diff --git a/tests/commandtest.c b/tests/commandtest.c index f433ad7..8dc57d7 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1063,6 +1063,74 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) return ret; } + +static int test25(const void *unused ATTRIBUTE_UNUSED) +{ + int ret = -1; + int pipeFD[2] = { -1, -1}; + int rv = 0; + ssize_t tries = 10; + pid_t pid; + + if (pipe(pipeFD) < 0) { + fprintf(stderr, "Unable to create pipe\n"); + goto cleanup; + } + + if (virSetNonBlock(pipeFD[0]) < 0) { + fprintf(stderr, "Unable to make read end of pipe nonblocking\n"); + goto cleanup; + } + + /* Now, fork and try to exec a nonexistent binary. */ + pid = fork(); + if (pid < 0) { + fprintf(stderr, "Unable to spawn child\n"); + goto cleanup; + } + + if (pid == 0) { + /* Child */ + virCommandPtr cmd = virCommandNew("some/nonexistent/binary"); + + rv = virCommandExec(cmd); + if (safewrite(pipeFD[1], (void *) &rv, sizeof(rv)) < 0) + fprintf(stderr, "Unable to write to pipe\n"); + _exit(EXIT_FAILURE); + } + + /* Parent */ + while (tries--) { + if (saferead(pipeFD[0], (void *) &rv, sizeof(rv)) < 0) { + if (errno != EWOULDBLOCK) { + fprintf(stderr, "Unable to read from pipe\n"); + goto cleanup; + } + + usleep(1000); + } else { + break; + } + } + + if (!tries) { + fprintf(stderr, "Child hasn't returned anything\n"); + goto cleanup; + } + + if (rv >= 0) { + fprintf(stderr, "Child should have returned an error\n"); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FORCE_CLOSE(pipeFD[0]); + VIR_FORCE_CLOSE(pipeFD[1]); + return ret; +} + + static void virCommandThreadWorker(void *opaque) { virCommandTestDataPtr test = opaque; @@ -1215,6 +1283,7 @@ mymain(void) DO_TEST(test22); DO_TEST(test23); DO_TEST(test24); + DO_TEST(test25); virMutexLock(&test->lock); if (test->running) { -- 2.8.4

On Mon, Jul 11, 2016 at 04:11:22PM +0200, Michal Privoznik wrote:
In an unlikely event of execve() failing, the virCommandExec() function does not report any error, even though checks that are at the beginning of the function are verbose when failing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 7 +++++- tests/commandtest.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-)
Looks good, but the tests sometimes fails on me with "Child should have returned an error". You might want to investigate that.

On Mon, Jul 11, 2016 at 05:42:41PM +0200, Martin Kletzander wrote:
On Mon, Jul 11, 2016 at 04:11:22PM +0200, Michal Privoznik wrote:
In an unlikely event of execve() failing, the virCommandExec() function does not report any error, even though checks that are at the beginning of the function are verbose when failing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 7 +++++- tests/commandtest.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-)
Looks good, but the tests sometimes fails on me with "Child should have returned an error". You might want to investigate that.
Also I get: tests/commandtest.c:1086: pid = fork(); When running make syntax-check...
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Martin Kletzander
-
Michal Privoznik