[libvirt] [PATCH] macvtap: plug memory leak for 802.1Qbh

[Roopa Prabhu]

On 10/14/11 1:55 PM, "Eric Blake" <eblake at redhat.com> wrote:

> On 10/14/2011 02:41 PM, Roopa Prabhu wrote:
>> 
>> 
>> 
>> On 10/13/11 3:49 PM, "Eric Blake"<eblake at redhat.com>  wrote:
>> 
>>> Detected by Coverity.  Leak present since commit ca3b22b.
>>> 
>>> * src/util/macvtap.c (doPortProfileOp8021Qbh): Release device name.
>>> ---
>>> getPhysfnDev allocates physfndev, but nothing freed it.
>>> Pushing under the trivial rule.
>> 
>> Actually looks like physfndev is conditionally allocated in getPhysfnDev
>> Its better to modify getPhysfnDev to allocate physfndev every time.
> 
> Also a good catch - otherwise we might have a double free or otherwise
> crash on freeing an invalid pointer.
> 
>> 
>> 
>> I ACK this patch with the additional change below (looks ok ?)
>> 
>> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
>> index a020c90..b50b7d2 100644
>> --- a/src/util/macvtap.c
>> +++ b/src/util/macvtap.c
>> @@ -964,7 +964,7 @@ getPhysfnDev(const char *linkdev,
>>            */
>> 
>>           *vf = PORT_SELF_VF;
>> -        *physfndev = (char *)linkdev;
>> +        *physfndev = strdup(linkdev);
> 
> I had already pushed mine.  Yours is missing a virReportOOMError() on
> allocation failure; but ACK with that improvement.
> 
> Meanwhile, we need to scrub this file - it uses a convention of 1 on
> error, instead of the more typical -1 on error in the rest of the code
> base; but I want this bug fix to be separate from that subsequent cleanup.
> 
> diff --git i/src/util/macvtap.c w/src/util/macvtap.c
> index b50b7d2..33f0a13 100644
> --- i/src/util/macvtap.c
> +++ w/src/util/macvtap.c
> @@ -965,6 +965,10 @@ getPhysfnDev(const char *linkdev,
> 
>           *vf = PORT_SELF_VF;
>           *physfndev = strdup(linkdev);
> +        if (!*physfndev) {
> +            virReportOOMError();
> +            rc = -1;
> +        }
>       }
> 
>       return rc;
> 
Great. Thanks!. I will send out a patch for the bug fix and later work on
the fixing the error return convention.