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

[Eric Blake]

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;


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org