On Tue, Aug 21, 2012 at 11:01:44AM +0200, Ján Tomko wrote:
Fix possible double close in the child process after the fork in case infd and outfd are equal, just like they are after being called from virNetSocketNewConnectCommand. --- src/util/command.c | 14 +++++--------- 1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c index 7755572..49ec178 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -547,17 +547,13 @@ virExecWithHook(const char *const*argv, goto fork_error; }
- if (infd != STDIN_FILENO && infd != null) + if (infd != STDIN_FILENO && infd != null && infd != childerr && + infd != childout) VIR_FORCE_CLOSE(infd); - if (childout > STDERR_FILENO && childout != null) { - tmpfd = childout; /* preserve childout value */ - VIR_FORCE_CLOSE(tmpfd); - } - if (childerr > STDERR_FILENO && - childerr != childout && - childerr != null) { + if (childout > STDERR_FILENO && childout != null && childout != childerr) + VIR_FORCE_CLOSE(childout); + if (childerr > STDERR_FILENO && childerr != null) VIR_FORCE_CLOSE(childerr); - } VIR_FORCE_CLOSE(null);
/* Initialize full logging for a while */
ACK. I'm wondering if there's some way we can test this in tests/commandtest.c All I can think of is using a nasty LD_PRELOAD hack again to override dup/open/close/etc and look for a double-close. Perhaps just leave it up to valgrind Daniel; -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|