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

[Libvir] [patch 7/9] Fix our FD_CLOEXEC usage



Implement a sane policy around our use of FD_CLOEXEC:

  1) Every descriptor which shouldn't be passed to
     child processes should have the flag set

  2) Let exec() do the descriptor closing, rather
     than us doing it ourselves

Signed-off-by: Mark McLoughlin <markmc redhat com>

Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c
+++ libvirt/qemud/qemud.c
@@ -87,7 +87,7 @@ static int qemudGoDaemon(void) {
         {
             int stdinfd = -1;
             int stdoutfd = -1;
-            int i, open_max, nextpid;
+            int nextpid;
 
             if ((stdinfd = open(_PATH_DEVNULL, O_RDONLY)) < 0)
                 goto cleanup;
@@ -106,13 +106,6 @@ static int qemudGoDaemon(void) {
                 goto cleanup;
             stdoutfd = -1;
 
-            open_max = sysconf (_SC_OPEN_MAX);
-            for (i = 0; i < open_max; i++)
-                if (i != STDIN_FILENO &&
-                    i != STDOUT_FILENO &&
-                    i != STDERR_FILENO)
-                    close(i);
-
             if (setsid() < 0)
                 goto cleanup;
 
@@ -355,23 +348,8 @@ static int qemudDispatchServer(struct qe
 
 
 static int
-qemudLeaveFdOpen(int *openfds, int fd)
-{
-    int i;
-
-    if (!openfds)
-        return 0;
-
-    for (i = 0; openfds[i] != -1; i++)
-        if (fd == openfds[i])
-            return 1;
-
-    return 0;
-}
-
-static int
 qemudExec(struct qemud_server *server, char **argv,
-          int *retpid, int *outfd, int *errfd, int *openfds) {
+          int *retpid, int *outfd, int *errfd) {
     int pid, null;
     int pipeout[2] = {-1,-1};
     int pipeerr[2] = {-1,-1};
@@ -400,11 +378,13 @@ qemudExec(struct qemud_server *server, c
         if (outfd) {
             close(pipeout[1]);
             qemudSetNonBlock(pipeout[0]);
+            qemudSetCloseExec(pipeout[0]);
             *outfd = pipeout[0];
         }
         if (errfd) {
             close(pipeerr[1]);
             qemudSetNonBlock(pipeerr[0]);
+            qemudSetCloseExec(pipeerr[0]);
             *errfd = pipeerr[0];
         }
         *retpid = pid;
@@ -425,13 +405,11 @@ qemudExec(struct qemud_server *server, c
     if (dup2(pipeerr[1] > 0 ? pipeerr[1] : null, STDERR_FILENO) < 0)
         _exit(1);
 
-    int i, open_max = sysconf (_SC_OPEN_MAX);
-    for (i = 0; i < open_max; i++)
-        if (i != STDOUT_FILENO &&
-            i != STDERR_FILENO &&
-            i != STDIN_FILENO &&
-            !qemudLeaveFdOpen(openfds, i))
-            close(i);
+    close(null);
+    if (pipeout[1] > 0)
+        close(pipeout[1]);
+    if (pipeerr[1] > 0)
+        close(pipeerr[1]);
 
     execvp(argv[0], argv);
 
@@ -441,13 +419,13 @@ qemudExec(struct qemud_server *server, c
 
  cleanup:
     if (pipeerr[0] > 0)
-        close(pipeerr[0] > 0);
-    if (pipeerr[1])
-        close(pipeerr[1] > 0);
-    if (pipeout[0])
-        close(pipeout[0] > 0);
-    if (pipeout[1])
-        close(pipeout[1] > 0);
+        close(pipeerr[0]);
+    if (pipeerr[1] > 0)
+        close(pipeerr[1]);
+    if (pipeout[0] > 0)
+        close(pipeout[0]);
+    if (pipeout[1] > 0)
+        close(pipeout[1]);
     if (null > 0)
         close(null);
     return -1;
@@ -467,7 +445,7 @@ int qemudStartVMDaemon(struct qemud_serv
     if (qemudBuildCommandLine(server, vm, &argv) < 0)
         return -1;
 
-    if (qemudExec(server, argv, &vm->pid, &vm->stdout, &vm->stderr, vm->tapfds) == 0) {
+    if (qemudExec(server, argv, &vm->pid, &vm->stdout, &vm->stderr) == 0) {
         vm->id = server->nextvmid++;
         ret = 0;
     }
@@ -863,7 +841,7 @@ dhcpStartDhcpDaemon(struct qemud_server 
     if (qemudBuildDnsmasqArgv(server, network, &argv) < 0)
         return -1;
 
-    ret = qemudExec(server, argv, &network->dnsmasqPid, NULL, NULL, NULL);
+    ret = qemudExec(server, argv, &network->dnsmasqPid, NULL, NULL);
 
     for (i = 0; argv[i]; i++)
         free(argv[i]);
Index: libvirt/qemud/bridge.c
===================================================================
--- libvirt.orig/qemud/bridge.c
+++ libvirt/qemud/bridge.c
@@ -54,6 +54,7 @@ int
 brInit(brControl **ctlp)
 {
     int fd;
+    int flags;
 
     if (!ctlp || *ctlp)
         return EINVAL;
@@ -62,6 +63,13 @@ brInit(brControl **ctlp)
     if (fd < 0)
         return errno;
 
+    if ((flags = fcntl(fd, F_GETFD)) < 0 ||
+        fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
+        int err = errno;
+        close(fd);
+        return err;
+    }
+
     *ctlp = (brControl *)malloc(sizeof(struct _brControl));
     if (!*ctlp)
         return ENOMEM;
Index: libvirt/qemud/iptables.c
===================================================================
--- libvirt.orig/qemud/iptables.c
+++ libvirt/qemud/iptables.c
@@ -317,15 +317,11 @@ iptablesSpawn(int errors, char * const *
     }
 
     if (pid == 0) { /* child */
-        int i, open_max = sysconf(_SC_OPEN_MAX);
-
-        for (i = 0; i < open_max; i++) {
-            if (i != STDOUT_FILENO &&
-                i != STDERR_FILENO &&
-                i != STDIN_FILENO)
-                close(i);
-        else if (errors == NO_ERRORS)
-            dup2(null, i);
+        if (errors == NO_ERRORS) {
+            dup2(null, STDIN_FILENO);
+            dup2(null, STDOUT_FILENO);
+            dup2(null, STDERR_FILENO);
+            close(null);
         }
 
         execvp(argv[0], argv);

-- 


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