External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-22 11:22:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

command: avoid leaking fds across fork

Browse Source 
Since libvirt is multi-threaded, we should use FD_CLOEXEC as much
as possible in the parent, and only relax fds to inherited after
forking, to avoid leaking an fd created in one thread to a fork
run in another thread.  This gets us closer to that ideal, by
making virCommand automatically clear FD_CLOEXEC on fds intended
for the child, as well as avoiding a window of time with non-cloexec
pipes created for capturing output.

* src/util/command.c (virExecWithHook): Use CLOEXEC in parent.  In
child, guarantee that all fds to pass to child are inheritable.
(getDevNull): Use CLOEXEC.
(prepareStdFd): New helper function.
(virCommandRun, virCommandRequireHandshake): Use pipe2.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Simplify caller.
This commit is contained in:
Eric Blake 2011-07-12 14:56:03 -06:00
parent 4289114518
commit 5d804ffae4
2 changed files with 29 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/qemu/qemu_command.c
View File

@ -4575,14 +4575,6 @@ qemuBuildCommandLine(virConnectPtr conn,
} else if (STREQ(migrateFrom, "stdio")) {
if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
/* migrateFd might be cloexec, but qemu must inherit
* it if vmop indicates qemu will be executed */
if (vmop != VIR_VM_OP_NO_OP &&
virSetInherit(migrateFd, true) < 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to clear cloexec flag"));
goto error;
}
virCommandPreserveFD(cmd, migrateFd);
} else if (qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_EXEC)) {
virCommandAddArg(cmd, "exec:cat");
@ -4612,14 +4604,6 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
/* migrateFd might be cloexec, but qemu must inherit
* it if vmop indicates qemu will be executed */
if (vmop != VIR_VM_OP_NO_OP &&
virSetInherit(migrateFd, true) < 0) {
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to clear cloexec flag"));
goto error;
}
virCommandPreserveFD(cmd, migrateFd);
} else if (STRPREFIX(migrateFrom, "unix")) {
if (!qemuCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_UNIX)) {

57
src/util/command.c
View File

@ -259,7 +259,7 @@ cleanup:
static int
getDevNull(int *null)
{
if (*null == -1 && (*null = open("/dev/null", O_RDWR)) < 0) {
if (*null == -1 && (*null = open("/dev/null", O_RDWR|O_CLOEXEC)) < 0) {
virReportSystemError(errno,
_("cannot open %s"),
"/dev/null");
@ -268,6 +268,18 @@ getDevNull(int *null)
return 0;
}
/* Ensure that STD is an inheritable copy of FD. Return 0 on success,
* -1 on failure. */
static int
prepareStdFd(int fd, int std)
{
if (fd == std)
return virSetInherit(fd, true);
if (dup2(fd, std) != std)
return -1;
return 0;
}
/*
* @argv argv to exec
* @envp optional environment to use for exec
@ -327,7 +339,7 @@ virExecWithHook(const char *const*argv,
if (outfd != NULL) {
if (*outfd == -1) {
if (pipe(pipeout) < 0) {
if (pipe2(pipeout, O_CLOEXEC) < 0) {
virReportSystemError(errno,
"%s", _("cannot create pipe"));
goto cleanup;
@ -340,12 +352,6 @@ virExecWithHook(const char *const*argv,
goto cleanup;
}
if (virSetCloseExec(pipeout[0]) == -1) {
virReportSystemError(errno,
"%s", _("Failed to set close-on-exec file descriptor flag"));
goto cleanup;
}
childout = pipeout[1];
} else {
childout = *outfd;
@ -358,7 +364,7 @@ virExecWithHook(const char *const*argv,
if (errfd != NULL) {
if (*errfd == -1) {
if (pipe(pipeerr) < 0) {
if (pipe2(pipeerr, O_CLOEXEC) < 0) {
virReportSystemError(errno,
"%s", _("Failed to create pipe"));
goto cleanup;
@ -371,12 +377,6 @@ virExecWithHook(const char *const*argv,
goto cleanup;
}
if (virSetCloseExec(pipeerr[0]) == -1) {
virReportSystemError(errno,
"%s", _("Failed to set close-on-exec file descriptor flag"));
goto cleanup;
}
childerr = pipeerr[1];
} else {
childerr = *errfd;
@ -426,28 +426,29 @@ virExecWithHook(const char *const*argv,
}
openmax = sysconf(_SC_OPEN_MAX);
for (i = 3; i < openmax; i++)
if (i != infd &&
i != childout &&
i != childerr &&
(!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd))) {
for (i = 3; i < openmax; i++) {
if (i == infd || i == childout || i == childerr)
continue;
if (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd)) {
tmpfd = i;
VIR_FORCE_CLOSE(tmpfd);
} else if (virSetInherit(i, true) < 0) {
virReportSystemError(errno, _("failed to preserve fd %d"), i);
goto fork_error;
}
}
if (dup2(infd, STDIN_FILENO) < 0) {
if (prepareStdFd(infd, STDIN_FILENO) < 0) {
virReportSystemError(errno,
"%s", _("failed to setup stdin file handle"));
goto fork_error;
}
if (childout > 0 &&
dup2(childout, STDOUT_FILENO) < 0) {
if (childout > 0 && prepareStdFd(childout, STDOUT_FILENO) < 0) {
virReportSystemError(errno,
"%s", _("failed to setup stdout file handle"));
goto fork_error;
}
if (childerr > 0 &&
dup2(childerr, STDERR_FILENO) < 0) {
if (childerr > 0 && prepareStdFd(childerr, STDERR_FILENO) < 0) {
virReportSystemError(errno,
"%s", _("failed to setup stderr file handle"));
goto fork_error;
@ -1805,7 +1806,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
/* If we have an input buffer, we need
* a pipe to feed the data to the child */
if (cmd->inbuf) {
if (pipe(infd) < 0) {
if (pipe2(infd, O_CLOEXEC) < 0) {
virReportSystemError(errno, "%s",
_("unable to open pipe"));
cmd->has_error = -1;
@ -2295,11 +2296,11 @@ void virCommandRequireHandshake(virCommandPtr cmd)
return;
}
if (pipe(cmd->handshakeWait) < 0) {
if (pipe2(cmd->handshakeWait, O_CLOEXEC) < 0) {
cmd->has_error = errno;
return;
}
if (pipe(cmd->handshakeNotify) < 0) {
if (pipe2(cmd->handshakeNotify, O_CLOEXEC) < 0) {
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
VIR_FORCE_CLOSE(cmd->handshakeWait[1]);
cmd->has_error = errno;