lib: Avoid double close when passing FDs with virCommandPassFD()

If an FD is passed into a child using:

  virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);

then the parent should refrain from touching @fd thereafter. This
is even documented in virCommandPassFD() comment. The reason is
that either at virCommandRun()/virCommandRunAsync() or
virCommandFree() time the @fd will be closed. Closing it earlier,
e.g. right after virCommandPassFD() call might result in
undesired results. Another thread might open a file and receive
the same FD which is then unexpectedly closed by virCommandFree()
or virCommandRun().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Michal Privoznik 2019-04-30 11:17:22 +02:00
parent e5df4edefa
commit 5cdd5d380b
3 changed files with 10 additions and 7 deletions

View File

@ -8997,17 +8997,19 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
if (qemuSecuritySetTapFDLabel(driver->securityManager, if (qemuSecuritySetTapFDLabel(driver->securityManager,
def, tapfd[i]) < 0) def, tapfd[i]) < 0)
goto cleanup; goto cleanup;
virCommandPassFD(cmd, tapfd[i],
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0) if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0)
goto cleanup; goto cleanup;
virCommandPassFD(cmd, tapfd[i],
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
tapfd[i] = -1;
} }
for (i = 0; i < vhostfdSize; i++) { for (i = 0; i < vhostfdSize; i++) {
virCommandPassFD(cmd, vhostfd[i],
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0) if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0)
goto cleanup; goto cleanup;
virCommandPassFD(cmd, vhostfd[i],
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
vhostfd[i] = -1;
} }
if (chardev) if (chardev)
@ -9054,14 +9056,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virSetError(saved_err); virSetError(saved_err);
virFreeError(saved_err); virFreeError(saved_err);
} }
for (i = 0; vhostfd && i < vhostfdSize && vhostfd[i] >= 0; i++) { for (i = 0; vhostfd && i < vhostfdSize; i++) {
if (ret < 0) if (ret < 0)
VIR_FORCE_CLOSE(vhostfd[i]); VIR_FORCE_CLOSE(vhostfd[i]);
if (vhostfdName) if (vhostfdName)
VIR_FREE(vhostfdName[i]); VIR_FREE(vhostfdName[i]);
} }
VIR_FREE(vhostfdName); VIR_FREE(vhostfdName);
for (i = 0; tapfd && i < tapfdSize && tapfd[i] >= 0; i++) { for (i = 0; tapfd && i < tapfdSize; i++) {
if (ret < 0) if (ret < 0)
VIR_FORCE_CLOSE(tapfd[i]); VIR_FORCE_CLOSE(tapfd[i]);
if (tapfdName) if (tapfdName)

View File

@ -187,6 +187,7 @@ virPolkitAgentCreate(void)
virCommandSetOutputFD(agent->cmd, &outfd); virCommandSetOutputFD(agent->cmd, &outfd);
virCommandSetErrorFD(agent->cmd, &errfd); virCommandSetErrorFD(agent->cmd, &errfd);
virCommandPassFD(agent->cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandPassFD(agent->cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
pipe_fd[1] = -1;
if (virCommandRunAsync(agent->cmd, NULL) < 0) if (virCommandRunAsync(agent->cmd, NULL) < 0)
goto error; goto error;

View File

@ -1024,6 +1024,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
virCommandDaemonize(cmd); virCommandDaemonize(cmd);
virCommandPassFD(cmd, newfd2, VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandPassFD(cmd, newfd2, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT); virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
newfd2 = newfd3 = -1;
virCommandPassListenFDs(cmd); virCommandPassListenFDs(cmd);
if (virCommandRun(cmd, NULL) < 0) { if (virCommandRun(cmd, NULL) < 0) {
@ -1053,7 +1054,6 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
VIR_FREE(prefix); VIR_FREE(prefix);
virCommandFree(cmd); virCommandFree(cmd);
VIR_FORCE_CLOSE(newfd1); VIR_FORCE_CLOSE(newfd1);
/* coverity[double_close] */
VIR_FORCE_CLOSE(newfd2); VIR_FORCE_CLOSE(newfd2);
VIR_FORCE_CLOSE(newfd3); VIR_FORCE_CLOSE(newfd3);
return ret; return ret;