[libvirt] [PATCH] command: avoid double close bugs
Wen Congyang
wency at cn.fujitsu.com
Wed May 30 03:51:56 UTC 2012
At 05/30/2012 09:20 AM, Eric Blake Wrote:
> 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.
>
> This is based on his first attempt at a patch, at
> https://bugzilla.redhat.com/show_bug.cgi?id=823716
close fd more twice is the cause of this bug. But there are some
other codes that have the same problem. I am searching all such
codes recent days.
>
> * src/util/command.c (_virCommand): Drop inpipe member.
> (virCommandProcessIO): Add argument, to avoid closing caller's fd
> without informing caller.
> (virCommandRun, virCommandNewArgs): Adjust clients.
> ---
> src/util/command.c | 18 ++++++------------
> 1 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/src/util/command.c b/src/util/command.c
> index eaa9f16..bf219e4 100644
> --- a/src/util/command.c
> +++ b/src/util/command.c
> @@ -83,7 +83,6 @@ struct _virCommand {
> char **errbuf;
>
> int infd;
> - int inpipe;
> int outfd;
> int errfd;
> int *outfdptr;
> @@ -788,7 +787,6 @@ virCommandNewArgs(const char *const*args)
> cmd->handshakeNotify[1] = -1;
>
> cmd->infd = cmd->outfd = cmd->errfd = -1;
> - cmd->inpipe = -1;
> cmd->pid = -1;
>
> virCommandAddArgSet(cmd, args);
> @@ -1676,7 +1674,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;
> @@ -1687,7 +1685,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
> @@ -1807,12 +1805,9 @@ virCommandProcessIO(virCommandPtr cmd)
> }
> } else {
> inoff += done;
> - if (inoff == inlen) {
> - int tmpfd ATTRIBUTE_UNUSED;
> - tmpfd = infd;
> - if (VIR_CLOSE(infd) < 0)
> - VIR_DEBUG("ignoring failed close on fd %d", tmpfd);
> - }
> + if (inoff == inlen && VIR_CLOSE(*inpipe) < 0)
> + VIR_DEBUG("ignoring failed close on fd %d", infd);
> + infd = -1;
if inoff != inlen, we should not set infd to -1.
> }
> }
> }
> @@ -1938,7 +1933,6 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
> return -1;
> }
> cmd->infd = infd[0];
> - cmd->inpipe = infd[1];
> }
>
> /* If caller requested the same string for stdout and stderr, then
> @@ -1981,7 +1975,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
> }
>
> if (string_io)
> - ret = virCommandProcessIO(cmd);
> + ret = virCommandProcessIO(cmd, &infd[1]);
>
> if (virCommandWait(cmd, exitstatus) < 0)
> ret = -1;
More information about the libvir-list
mailing list