[libvirt] [PATCH] command: avoid double close in virExecWithHook
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 */ -- 1.7.8.6
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 :|
On 08/21/2012 10:28 AM, Daniel P. Berrange wrote:
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(-)
ACK.
Pushed.
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
Well, we _do_ log double-close via VIR_FORCE_CLOSE; maybe it's sufficient to set up a test case that opens a file for O_RDWR and passes that fd for stdin and stdout, then tracking if our logging flags anything? But I have not yet tried doing that in the testsuite. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Ján Tomko