[libvirt] [PATCH] command: avoid double close in virExecWithHook

[Daniel P. Berrange]

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 :|