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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



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

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]