[libvirt] [PATCH 1/2] qemu: caps: Fix segfault on daemon startup

[Cole Robinson]

On 01/27/2012 03:57 PM, Eric Blake wrote:
> On 01/27/2012 11:34 AM, Cole Robinson wrote:
>> On my f16 box at least, this tries to free p unconditionally which isn't
>> always okay, 'p' is freeable only under a limited scope.
> 
> You must not have yajl-devel installed.  No wonder I didn't see that I
> had introduced a regression.
> 
>> ---
>>  src/qemu/qemu_capabilities.c |    3 +--
>>  1 files changed, 1 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 6dee9d8..5487ecd 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -1336,10 +1336,9 @@ fail:
>>      qemuReportError(VIR_ERR_INTERNAL_ERROR,
>>                      _("cannot parse %s version number in '%s'"),
>>                      qemu, p ? p : help);
>> -
>> -cleanup:
>>      VIR_FREE(p);
>>  
>> +cleanup:
> 
> Yuck.  Why are we even calling strndup in the first place, just to
> produce a nicer error message?  I'm pushing this instead.
> 
> From ab6f1c9814f88e0547a5567176282f501fb138e2 Mon Sep 17 00:00:00 2001
> From: Eric Blake <eblake at redhat.com>
> Date: Fri, 27 Jan 2012 13:53:11 -0700
> Subject: [PATCH] qemu: avoid double free of qemu help output
> 
> If yajl was not compiled in, we end up freeing an incoming
> parameter, which leads to a bogus free later on.  Regression
> introduced in commit 6e769eb.
> 
> * src/qemu/qemu_capabilities.c (qemuCapsParseHelpStr): Avoid alloc
> on failure path, which in turn fixes bogus free.
> Reported by Cole Robinson.
> ---
>  src/qemu/qemu_capabilities.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 6dee9d8..426637c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1330,16 +1330,14 @@ int qemuCapsParseHelpStr(const char *qemu,
> 
>  fail:
>      p = strchr(help, '\n');
> -    if (p)
> -        p = strndup(help, p - help);
> +    if (!p)
> +        p = strchr(help, '\0');
> 
>      qemuReportError(VIR_ERR_INTERNAL_ERROR,
> -                    _("cannot parse %s version number in '%s'"),
> -                    qemu, p ? p : help);
> +                    _("cannot parse %s version number in '%.*s'"),
> +                    qemu, (int) (p - help), help);
> 
>  cleanup:
> -    VIR_FREE(p);
> -
>      return -1;
>  }
> 

Cool, works for me.

Thanks,
Cole