[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: 4/5: Support daemonizing child & env variables



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]