qemu: Remove hostdev entry when freeing the depending network entry

When using a <interface type="network"> that points to a network with
hostdev forwarding mode a hostdev alias is created for the network. This
allias is inserted into the hostdev list, but is backed with a part of
the network object that it is connected to.

When a VM is being stopped qemuProcessStop() calls
networkReleaseActualDevice() which eventually frees the memory for the
hostdev object. Afterwards when the domain definition is being freed by
virDomainDefFree() an invalid pointer is accessed by
virDomainHostdevDefFree() and may cause a crash of the daemon.

This patch removes the entry in the hostdev list before freeing the
depending memory to avoid this issue.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1000973

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b1387d62b1ed34d288676d5516b9f1b04cba34f..f8fbf79de5d4ff8a5b9215c036942e266b33dc99 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9965,26 +9965,36 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)
     return matchidx;
 }
 
-virDomainNetDefPtr
-virDomainNetRemove(virDomainDefPtr def, size_t i)
-{
-    virDomainNetDefPtr net = def->nets[i];
 
+void
+virDomainNetRemoveHostdev(virDomainDefPtr def,
+                          virDomainNetDefPtr net)
+{
     if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
         /* hostdev net devices are normally also be in the hostdevs
          * array, but might have already been removed by the time we
          * get here.
          */
         virDomainHostdevDefPtr hostdev = &net->data.hostdev.def;
-        size_t h;
+        size_t i;
 
-        for (h = 0; h < def->nhostdevs; h++) {
-            if (def->hostdevs[h] == hostdev) {
-                virDomainHostdevRemove(def, h);
+        for (i = 0; i < def->nhostdevs; i++) {
+            if (def->hostdevs[i] == hostdev) {
+                virDomainHostdevRemove(def, i);
                 break;
             }
         }
     }
+}
+
+
+virDomainNetDefPtr
+virDomainNetRemove(virDomainDefPtr def, size_t i)
+{
+    virDomainNetDefPtr net = def->nets[i];
+
+    virDomainNetRemoveHostdev(def, net);
+
     if (def->nnets > 1) {
         memmove(def->nets + i,
                 def->nets + i + 1,

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0ac576a82e2e7009cf9d103659c053b5c253c308..56739b7bf49c9503fdae74e5fcedf6f3d25789a8 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2371,6 +2371,7 @@ int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
 virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
 int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
 virDomainNetDefPtr virDomainNetRemove(virDomainDefPtr def, size_t i);
+void virDomainNetRemoveHostdev(virDomainDefPtr def, virDomainNetDefPtr net);
 
 int virDomainHostdevInsert(virDomainDefPtr def, virDomainHostdevDefPtr hostdev);
 virDomainHostdevDefPtr

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d4006ae82a35703533d3b10393fe3bc507be4ef5..35f0f1bb83e9f07b590c9b5f78d786f0e9135916 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -303,6 +303,7 @@ virDomainNetGetActualVirtPortProfile;
 virDomainNetGetActualVlan;
 virDomainNetInsert;
 virDomainNetRemove;
+virDomainNetRemoveHostdev;
 virDomainNetTypeToString;
 virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5943816c059cb84c177c8496a269cee30c5a7697..a7256f357d31cc98ea37ea658ce07a95bcdbcc6f 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1066,6 +1066,8 @@ cleanup:
                                virDomainNetGetActualBridgeName(net), net->ifname));
         }
 
+        virDomainNetRemoveHostdev(vm->def, net);
+
         networkReleaseActualDevice(net);
     }
 
@@ -2107,6 +2109,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
         /* the changes above warrant replacing olddev with newdev in
          * the domain's nets list.
          */
+
+        /* this function doesn't work with HOSTDEV networks yet, thus
+         * no need to change the pointer in the hostdev structure */
         networkReleaseActualDevice(olddev);
         virDomainNetDefFree(olddev);
         /* move newdev into the nets list, and NULL it out from the

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 128618bf53bf8871dd637abeab10360c93ada5e1..dfe81428e70ffe9b43899ce122a597850920f1f2 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4259,6 +4259,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
                                        virDomainNetGetActualBridgeName(net),
                                        net->ifname));
 
+        /* kick the device out of the hostdev list too */
+        virDomainNetRemoveHostdev(def, net);
         networkReleaseActualDevice(net);
     }