[libvirt] [PATCHv2 11/12] pci: fix dangling pointer in qemuDomainReAttachHostdevDevices

[Laine Stump]

(This isn't as bad as it sounds - it's only a problem in case of an
OOM error.)

qemuGetActivePciHostDeviceList() had been creating a list that
contained pointers to objects that were also on the activePciHostdevs
list. In case of an OOM error, this newly created list would be
virObjectUnref'ed, which would cause everything on the list to be
freed. But all of those objects would still be on the
activePciHostdevs list, which could have very bad consequences if that
list was ever again accessed.

The solution used here is to populate the new list with *copies* of
the objects from the original list. It turns out that on return from
qemuGetActivePciHostDeviceList(), the caller would almost immediately
go through all the device objects and "steal" them (i.e. remove the
pointer from the list but not delete it) all from either one list or
the other; we now instead just *delete* (remove from the list and
free) each device from one list or the other, so in the end we have
the same state.
---
 src/qemu/qemu_hostdev.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index d7d54d7..09ac6ad 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -86,6 +86,12 @@ qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
 }
 
 /*
+ * qemuGetActivePciHostDeviceList - make a new list with a *copy* of
+ *   every virPCIDevice object that is found on the activePciHostdevs
+ *   list *and* is in the hostdev list for this domain.
+ *
+ * Return the new list, or NULL if there was a failure.
+ *
  * Pre-condition: driver->activePciHostdevs is locked
  */
 static virPCIDeviceListPtr
@@ -101,7 +107,7 @@ qemuGetActivePciHostDeviceList(virQEMUDriverPtr driver,
 
     for (i = 0; i < nhostdevs; i++) {
         virDomainHostdevDefPtr hostdev = hostdevs[i];
-        virPCIDevicePtr dev;
+        virDevicePCIAddressPtr addr;
         virPCIDevicePtr activeDev;
 
         if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
@@ -109,24 +115,14 @@ qemuGetActivePciHostDeviceList(virQEMUDriverPtr driver,
         if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
             continue;
 
-        dev = virPCIDeviceNew(hostdev->source.subsys.u.pci.addr.domain,
-                              hostdev->source.subsys.u.pci.addr.bus,
-                              hostdev->source.subsys.u.pci.addr.slot,
-                              hostdev->source.subsys.u.pci.addr.function);
-        if (!dev) {
+        addr = &hostdev->source.subsys.u.pci.addr;
+        activeDev = virPCIDeviceListFindByIDs(driver->activePciHostdevs,
+                                              addr->domain, addr->bus,
+                                              addr->slot, addr->function);
+        if (activeDev && virPCIDeviceListAddCopy(list, activeDev) < 0) {
             virObjectUnref(list);
             return NULL;
         }
-
-        if ((activeDev = virPCIDeviceListFind(driver->activePciHostdevs, dev))) {
-            if (virPCIDeviceListAdd(list, activeDev) < 0) {
-                virPCIDeviceFree(dev);
-                virObjectUnref(list);
-                return NULL;
-            }
-        }
-
-        virPCIDeviceFree(dev);
     }
 
     return list;
@@ -1084,20 +1080,24 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
         virPCIDevicePtr activeDev = NULL;
 
-        /* Never delete the dev from list driver->activePciHostdevs
-         * if it's used by other domain.
+        /* delete the copy of the dev from pcidevs if it's used by
+         * other domain. Or delete it from activePciHostDevs if it had
+         * been used by this domain.
          */
         activeDev = virPCIDeviceListFind(driver->activePciHostdevs, dev);
         if (activeDev &&
             STRNEQ_NULLABLE(name, virPCIDeviceGetUsedBy(activeDev))) {
-            virPCIDeviceListSteal(pcidevs, dev);
+            virPCIDeviceListDel(pcidevs, dev);
             continue;
         }
 
-        /* virObjectUnref() will take care of freeing the dev. */
-        virPCIDeviceListSteal(driver->activePciHostdevs, dev);
+        virPCIDeviceListDel(driver->activePciHostdevs, dev);
     }
 
+    /* At this point, any device that had been used by the guest is in
+     * pcidevs, but has been removed from activePciHostdevs.
+     */
+
     /*
      * For SRIOV net host devices, unset mac and port profile before
      * reset and reattach device
-- 
1.7.11.7