[libvirt] [PATCH] command: avoid potential deadlock on handshake

[Michal Privoznik]

On 07.06.2012 01:37, Eric Blake wrote:
> Similar to commit 858c247, the parent process should close it's
> copy of the end of the pipe given to the child; otherwise, if there
> is an extreme bug where the parent thinks the child reported an
> error and is waiting for the message to go along with it, but the
> child thinks it reported success and is waiting for the parent
> to acknowledge the success, we would get into deadlock.
> 
> 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 - if a double close bug in
> the parent causes the parent to read the wrong fd, it might assume
> the child failed, even though the child really sent success if
> only the parent had read from the correct location.
> 
> * src/util/command.c (virCommandHandshakeWait): Close unused fds
> sooner.
> ---
> 
>  src/util/command.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/util/command.c b/src/util/command.c
> index 62ea50b..62848cd 100644
> --- 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);

ACK

Michal