8:44 a.m.
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(a)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);
--