[libvirt] [PATCH] virsh: avoid null pointer dereference

[Daniel P. Berrange]

On Tue, May 03, 2011 at 10:45:36AM -0600, Eric Blake wrote:
> Clang detected that vol-download will call unlink(NULL) if there
> is a parse error during option parsing.  Also, mingw doesn't like
> unlinking an open file.
> 
> * tools/virsh.c (cmdVolDownload): Only unlink file if created.
> ---
>  tools/virsh.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 3d4ed2f..5d8b025 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7259,10 +7259,10 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd)
>      virStreamPtr st = NULL;
>      const char *name = NULL;
>      unsigned long long offset = 0, length = 0;
> -    bool created = true;
> +    bool created = false;
> 
>      if (!vshConnectionUsability(ctl, ctl->conn))
> -        goto cleanup;
> +        return false;
> 
>      if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
>          vshError(ctl, _("Unable to parse integer"));
> @@ -7283,12 +7283,13 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd)
>      }
> 
>      if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) {
> -        created = false;
>          if (errno != EEXIST ||
>              (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) {
>              vshError(ctl, _("cannot create %s"), file);
>              goto cleanup;
>          }
> +    } else {
> +        created = true;
>      }
> 
>      st = virStreamNew(ctl->conn, 0);
> @@ -7316,13 +7317,13 @@ cmdVolDownload (vshControl *ctl, const vshCmd *cmd)
>      ret = true;
> 
>  cleanup:
> +    VIR_FORCE_CLOSE(fd);
>      if (ret == false && created)
>          unlink(file);
>      if (vol)
>          virStorageVolFree(vol);
>      if (st)
>          virStreamFree(st);
> -    VIR_FORCE_CLOSE(fd);
>      return ret;
>  }

ACK


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