4:20 a.m.
Some of the existing usage of fork/exec in libvirt is done such that the
child process is daemonized. In particular the libvirt_proxy and the
auto-spawned libvirtd for qemu:///session. Since we want to switch these
to use virExec, we need to suport a daemon mode.
This patch removes the two alternate virExec and virExecNonBlock functions
and renames the internal __virExec to virExec. It then gains a 'flags'
parameter which can be used to specify non-blocking mode, or daemon mode.
We also add the ability to pass in a list of environment variables which
get passed on to execve(). We also now explicitly close all file handles.
Although libvirt code is careful to set O_CLOSEXEC on all its file handles,
in multi-threaded apps there is a race condition between opening a FD and
setting O_CLOSEXEC. Furthermore, we can't guarentee that all applications
using libvirt are careful to set O_CLOSEXEC. Leaking FDs to a child is a
potential security risk and often causes SELinux AVCs to be raised. Thus
explicitely closing all FDs is a important safety net.
openvz_driver.c | 4 +-
qemu_driver.c | 7 ++--
storage_backend.c | 4 +-
util.c | 78 +++++++++++++++++++++++++++++++++---------------------
util.h | 18 +++++++++---
5 files changed, 71 insertions(+), 40 deletions(-)
Daniel
diff -r 28ddf9f5791c src/openvz_driver.c
--- a/src/openvz_driver.c Tue Aug 12 22:12:42 2008 +0100
+++ b/src/openvz_driver.c Tue Aug 12 22:13:02 2008 +0100
@@ -742,7 +742,7 @@
char *endptr;
const char *cmd[] = {VZLIST, "-ovpsid", "-H" , NULL};
- ret = virExec(conn, cmd, &pid, -1, &outfd, &errfd);
+ ret = virExec(conn, cmd, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
if(ret == -1) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZLIST);
@@ -779,7 +779,7 @@
const char *cmd[] = {VZLIST, "-ovpsid", "-H", "-S",
NULL};
/* the -S options lists only stopped domains */
- ret = virExec(conn, cmd, &pid, -1, &outfd, &errfd);
+ ret = virExec(conn, cmd, NULL, &pid, -1, &outfd, &errfd, VIR_EXEC_NONE);
if(ret == -1) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZLIST);
diff -r 28ddf9f5791c src/qemu_driver.c
--- a/src/qemu_driver.c Tue Aug 12 22:12:42 2008 +0100
+++ b/src/qemu_driver.c Tue Aug 12 22:13:02 2008 +0100
@@ -951,8 +951,9 @@
vm->stdout_fd = vm->stderr_fd = -1;
- ret = virExecNonBlock(conn, argv, &vm->pid,
- vm->stdin_fd, &vm->stdout_fd,
&vm->stderr_fd);
+ ret = virExec(conn, argv, NULL, &vm->pid,
+ vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd,
+ VIR_EXEC_NONBLOCK);
if (ret == 0) {
vm->def->id = driver->nextvmid++;
vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
@@ -1199,7 +1200,7 @@
if (qemudBuildDnsmasqArgv(conn, network, &argv) < 0)
return -1;
- ret = virExecNonBlock(conn, argv, &network->dnsmasqPid, -1, NULL, NULL);
+ ret = virExec(conn, argv, NULL, &network->dnsmasqPid, -1, NULL, NULL,
VIR_EXEC_NONBLOCK);
for (i = 0; argv[i]; i++)
VIR_FREE(argv[i]);
diff -r 28ddf9f5791c src/storage_backend.c
--- a/src/storage_backend.c Tue Aug 12 22:12:42 2008 +0100
+++ b/src/storage_backend.c Tue Aug 12 22:13:02 2008 +0100
@@ -403,7 +403,7 @@
/* Run the program and capture its output */
- if (virExec(conn, prog, &child, -1, &fd, NULL) < 0) {
+ if (virExec(conn, prog, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0)
{
goto cleanup;
}
@@ -537,7 +537,7 @@
v[i] = NULL;
/* Run the program and capture its output */
- if (virExec(conn, prog, &child, -1, &fd, NULL) < 0) {
+ if (virExec(conn, prog, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0)
{
goto cleanup;
}
diff -r 28ddf9f5791c src/util.c
--- a/src/util.c Tue Aug 12 22:12:42 2008 +0100
+++ b/src/util.c Tue Aug 12 22:13:02 2008 +0100
@@ -103,11 +103,14 @@
return 0;
}
-static int
-_virExec(virConnectPtr conn,
- const char *const*argv,
- int *retpid, int infd, int *outfd, int *errfd, int non_block) {
- int pid, null, i;
+int
+virExec(virConnectPtr conn,
+ const char *const*argv,
+ const char *const*envp,
+ int *retpid,
+ int infd, int *outfd, int *errfd,
+ int flags) {
+ int pid, null, i, openmax;
int pipeout[2] = {-1,-1};
int pipeerr[2] = {-1,-1};
int childout = -1, childerr = -1;
@@ -141,7 +144,7 @@
goto cleanup;
}
- if (non_block &&
+ if ((flags & VIR_EXEC_NONBLOCK) &&
virSetNonBlock(pipeout[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor
flag"));
@@ -172,7 +175,7 @@
goto cleanup;
}
- if (non_block &&
+ if ((flags & VIR_EXEC_NONBLOCK) &&
virSetNonBlock(pipeerr[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor
flag"));
@@ -259,10 +262,40 @@
return -1;
}
- if (pipeout[0] > 0)
- close(pipeout[0]);
- if (pipeerr[0] > 0)
- close(pipeerr[0]);
+ openmax = sysconf (_SC_OPEN_MAX);
+ for (i = 3; i < openmax; i++)
+ if (i != infd &&
+ i != null &&
+ i != childout &&
+ i != childerr)
+ close(i);
+
+ if (flags & VIR_EXEC_DAEMON) {
+ if (setsid() < 0) {
+ ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("cannot become session leader: %s"),
+ strerror(errno));
+ _exit(1);
+ }
+
+ if (chdir("/") < 0) {
+ ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("cannot change to root directory: %s"),
+ strerror(errno));
+ _exit(1);
+ }
+
+ pid = fork();
+ if (pid < 0) {
+ ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("cannot fork child process: %s"),
+ strerror(errno));
+ _exit(1);
+ }
+
+ if (pid > 0)
+ _exit(0);
+ }
if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) {
@@ -290,7 +323,10 @@
childerr != childout)
close(childerr);
- execvp(argv[0], (char **) argv);
+ if (envp)
+ execve(argv[0], (char **) argv, (char**)envp);
+ else
+ execvp(argv[0], (char **) argv);
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot execute binary '%s': %s"),
@@ -320,22 +356,6 @@
return -1;
}
-int
-virExec(virConnectPtr conn,
- const char *const*argv,
- int *retpid, int infd, int *outfd, int *errfd) {
-
- return(_virExec(conn, argv, retpid, infd, outfd, errfd, 0));
-}
-
-int
-virExecNonBlock(virConnectPtr conn,
- const char *const*argv,
- int *retpid, int infd, int *outfd, int *errfd) {
-
- return(_virExec(conn, argv, retpid, infd, outfd, errfd, 1));
-}
-
/**
* @conn connection to report errors against
* @argv NULL terminated argv to run
@@ -357,7 +377,7 @@
int *status) {
int childpid, exitstatus, ret;
- if ((ret = virExec(conn, argv, &childpid, -1, NULL, NULL)) < 0)
+ if ((ret = virExec(conn, argv, NULL, &childpid, -1, NULL, NULL, VIR_EXEC_NONE))
< 0)
return ret;
while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno ==
EINTR);
diff -r 28ddf9f5791c src/util.h
--- a/src/util.h Tue Aug 12 22:12:42 2008 +0100
+++ b/src/util.h Tue Aug 12 22:13:02 2008 +0100
@@ -27,10 +27,20 @@
#include "util-lib.h"
#include "verify.h"
-int virExec(virConnectPtr conn, const char *const*argv, int *retpid,
- int infd, int *outfd, int *errfd);
-int virExecNonBlock(virConnectPtr conn, const char *const*argv, int *retpid,
- int infd, int *outfd, int *errfd);
+enum {
+ VIR_EXEC_NONE = 0,
+ VIR_EXEC_NONBLOCK = (1 << 0),
+ VIR_EXEC_DAEMON = (1 << 1),
+};
+
+int virExec(virConnectPtr conn,
+ const char *const*argv,
+ const char *const*envp,
+ int *retpid,
+ int infd,
+ int *outfd,
+ int *errfd,
+ int flags);
int virRun(virConnectPtr conn, const char *const*argv, int *status);
int __virFileReadAll(const char *path,
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|