From 1a6bad7108c68d17dec863500cc40f8557b5e782 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 29 May 2012 17:47:58 -0600
Subject: [PATCH] command: avoid double close bugs

KAMEZAWA Hiroyuki reported a nasty double-free bug when virCommand
is used to convert a string into input to a child command.  The
problem is that the poll() loop of virCommandProcessIO would close()
the write end of the pipe in order to let the child see EOF, then
the caller virCommandRun() would also close the same fd number, with
the second close possibly nuking an fd opened by some other thread
in the meantime.  This in turn can have all sorts of bad effects.

The bug has been present since the introduction of virCommand in
commit f16ad06f.

This is based on his first attempt at a patch, at
https://bugzilla.redhat.com/show_bug.cgi?id=823716

* src/util/command.c (_virCommand): Drop inpipe member.
(virCommandProcessIO): Add argument, to avoid closing caller's fd
without informing caller.
(virCommandRun, virCommandNewArgs): Adjust clients.
(cherry picked from commit da831afcf2f6c0d3ed1ea3128a6208f548a05d8f)

Conflicts:

	src/util/command.c
---
 src/util/command.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/util/command.c b/src/util/command.c
index e444f237c9..d62d9d1a15 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -86,7 +86,6 @@ struct _virCommand {
     char **errbuf;
 
     int infd;
-    int inpipe;
     int outfd;
     int errfd;
     int *outfdptr;
@@ -690,7 +689,6 @@ virCommandNewArgs(const char *const*args)
     FD_ZERO(&cmd->preserve);
     FD_ZERO(&cmd->transfer);
     cmd->infd = cmd->outfd = cmd->errfd = -1;
-    cmd->inpipe = -1;
     cmd->pid = -1;
 
     virCommandAddArgSet(cmd, args);
@@ -1560,7 +1558,7 @@ virCommandTranslateStatus(int status)
  * Manage input and output to the child process.
  */
 static int
-virCommandProcessIO(virCommandPtr cmd)
+virCommandProcessIO(virCommandPtr cmd, int *inpipe)
 {
     int infd = -1, outfd = -1, errfd = -1;
     size_t inlen = 0, outlen = 0, errlen = 0;
@@ -1571,7 +1569,7 @@ virCommandProcessIO(virCommandPtr cmd)
      * via pipe */
     if (cmd->inbuf) {
         inlen = strlen(cmd->inbuf);
-        infd = cmd->inpipe;
+        infd = *inpipe;
     }
 
     /* With out/err buffer, the outfd/errfd have been filled with an
@@ -1692,9 +1690,9 @@ virCommandProcessIO(virCommandPtr cmd)
                 } else {
                     inoff += done;
                     if (inoff == inlen) {
-                        int tmpfd = infd;
-                        if (VIR_CLOSE(infd) < 0)
-                            VIR_DEBUG("ignoring failed close on fd %d", tmpfd);
+                        if (VIR_CLOSE(*inpipe) < 0)
+                            VIR_DEBUG("ignoring failed close on fd %d", infd);
+                        infd = -1;
                     }
                 }
             }
@@ -1834,7 +1832,6 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
             return -1;
         }
         cmd->infd = infd[0];
-        cmd->inpipe = infd[1];
     }
 
     /* If caller hasn't requested capture of stdout/err, then capture
@@ -1870,7 +1867,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
     }
 
     if (string_io)
-        ret = virCommandProcessIO(cmd);
+        ret = virCommandProcessIO(cmd, &infd[1]);
 
     if (virCommandWait(cmd, exitstatus) < 0)
         ret = -1;
-- 
2.47.3