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; -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list