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 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); --