On 10/14/2011 02:41 PM, Roopa Prabhu wrote:



On 10/13/11 3:49 PM, "Eric Blake"<ebl...@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   ebl...@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to