[libvirt] [PATCH] lib: Avoid double free 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(a)redhat.com>
---
src/qemu/qemu_command.c | 10 ++++++----
src/util/virpolkit.c | 1 +
tests/commandtest.c | 2 +-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bf1fb539b1..92bd1524db 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8978,17 +8978,19 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
if (qemuSecuritySetTapFDLabel(driver->securityManager,
def, tapfd[i]) < 0)
goto cleanup;
- virCommandPassFD(cmd, tapfd[i],
- VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&tapfdName[i], "%d", tapfd[i]) < 0)
goto cleanup;
+ virCommandPassFD(cmd, tapfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ tapfd[i] = -1;
}
for (i = 0; i < vhostfdSize; i++) {
- virCommandPassFD(cmd, vhostfd[i],
- VIR_COMMAND_PASS_FD_CLOSE_PARENT);
if (virAsprintf(&vhostfdName[i], "%d", vhostfd[i]) < 0)
goto cleanup;
+ virCommandPassFD(cmd, vhostfd[i],
+ VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ vhostfd[i] = -1;
}
if (chardev)
diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index 25eaad2c63..634b46e82b 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -187,6 +187,7 @@ virPolkitAgentCreate(void)
virCommandSetOutputFD(agent->cmd, &outfd);
virCommandSetErrorFD(agent->cmd, &errfd);
virCommandPassFD(agent->cmd, pipe_fd[1], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ pipe_fd[1] = -1;
if (virCommandRunAsync(agent->cmd, NULL) < 0)
goto error;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 816a70860f..146cc4c1bf 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1024,6 +1024,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
virCommandDaemonize(cmd);
virCommandPassFD(cmd, newfd2, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
virCommandPassFD(cmd, newfd3, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ newfd2 = newfd3 = -1;
virCommandPassListenFDs(cmd);
if (virCommandRun(cmd, NULL) < 0) {
@@ -1053,7 +1054,6 @@ static int test24(const void *unused ATTRIBUTE_UNUSED)
VIR_FREE(prefix);
virCommandFree(cmd);
VIR_FORCE_CLOSE(newfd1);
- /* coverity[double_close] */
VIR_FORCE_CLOSE(newfd2);
VIR_FORCE_CLOSE(newfd3);
return ret;
--
2.21.0