command: avoid potential deadlock on handshake

There is a theoretical problem of an extreme bug where we can get
into deadlock due to command handshaking.  Thanks to a pair of pipes,
we have a situation where the parent thinks the child reported an
error and is waiting for a message from the child to explain the
error; but at the same time the child thinks it reported success
and is waiting for the parent to acknowledge the success; so both
processes are now blocked.

Thankfully, I don't think this deadlock is possible without at
least one other bug in the code, but I did see exactly that sort
of situation prior to commit da831af - I saw a backtrace where a
double close bug in the parent caused the parent to read from the
wrong fd and assume the child failed, even though the child really
sent success.

This potential deadlock is not quite like commit 858c247 (a deadlock
due to multiple readers on one pipe preventing a write from completing),
although the solution is similar - always close unused pipe fds before
blocking, rather than after.

* src/util/command.c (virCommandHandshakeWait): Close unused fds
sooner.

diff --git a/src/util/command.c b/src/util/command.c
index 62ea50b3d3a9702bd9274c33333780236a9f1341..62848cd874b40f5acf2523a1595d59e8b95840ce 100644 (file)
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -2507,6 +2507,11 @@ int virCommandHandshakeWait(virCommandPtr cmd)
             VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
             return -1;
         }
+        /* Close the handshakeNotify fd before trying to read anything
+         * further on the handshakeWait pipe; so that a child waiting
+         * on our acknowledgment will die rather than deadlock.  */
+        VIR_FORCE_CLOSE(cmd->handshakeNotify[1]);
+
         if ((len = saferead(cmd->handshakeWait[0], msg, 1024)) < 0) {
             VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
             VIR_FREE(msg);