[libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities

Martin Kletzander mkletzan at redhat.com
Thu Apr 6 09:12:54 UTC 2017


On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote:
>On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
>> If formatting NUMA topology fails, the function returns immediatelly,
>> but the buffer structure allocated on the stack references lot of
>> heap-allocated memory and that would get lost in such case.
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  src/conf/capabilities.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index 08907aced1b9..be95c50cfb67 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>>      if (caps->host.nnumaCell &&
>>          virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
>>                                            caps->host.numaCell) < 0)
>> -        return NULL;
>> +        goto error;
>
>Personally, I'd more like cleanup, but looking at other XML formatting methods,
>I'm fine with this as well, at least we stay consistent.
>
>>
>>      for (i = 0; i < caps->host.nsecModels; i++) {
>>          virBufferAddLit(&buf, "<secmodel>\n");
>> @@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>>          return NULL;
>>
>
>Git discarded the context here, so squash this in:
>
>diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>index be95c50cf..ea6d4b19d 100644
>--- a/src/conf/capabilities.c
>+++ b/src/conf/capabilities.c
>@@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>     virBufferAddLit(&buf, "</capabilities>\n");
>
>     if (virBufferCheckError(&buf) < 0)
>-        return NULL;
>+        goto error;
>
>     return virBufferContentAndReset(&buf);
>

I don't really understand why would I need to do that.

>
>>      return virBufferContentAndReset(&buf);
>> +
>> + error:
>> +    virBufferFreeAndReset(&buf);
>> +    return NULL;
>>  }
>>
>>  /* get the maximum ID of cpus in the host */
>> --
>> 2.12.2
>>
>
>ACK with the bit above squashed in.
>
>Erik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170406/e8507fdf/attachment-0001.sig>


More information about the libvir-list mailing list