Re: [Qemu-devel] [PATCH] qemu-img: Fix crash in amend invocation

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On 08/18/2015 05:03 PM, Max Reitz wrote:
> Example:
> $ ./qemu-img create -f qcow2 /tmp/t.qcow2 64M
> $ ./qemu-img amend -f qcow2 -o backing_file=/tmp/t.qcow2, -o help \
>     /tmp/t.qcow2
> 
> This should not crash. This actually is tested by iotest 082, but not
> caught due to the segmentation fault being silent (which is something
> that needs to be fixed, too).

As long as we don't forget to do that, I'm okay with having this patch
separate from the testsuite enhancement.  However...

> 
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: qemu-stable <qemu-stable@nongnu.org>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 75f4ee4..3ddb391 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2930,8 +2930,7 @@ static int img_amend(int argc, char **argv)
>              case 'o':
>                  if (!is_valid_option_list(optarg)) {
>                      error_report("Invalid option list: %s", optarg);
> -                    ret = -1;
> -                    goto out;
> +                    return 1;
>                  }
>                  if (!options) {
>                      options = g_strdup(optarg);

This leaks memory if I call 'qemu-img amend -f qcow2 -o help -o
backing_file=/tmp/t.qcow2, -o help /tmp/t.qcow2', because it allocates
'options' on the first pass, and only cleans up the allocation in the
out label.  I think you HAVE to goto out, but fix THAT part of the
function to not crash when in this inconsistent state.

Looking forward to v2.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]