From 9bf517748d9fed50a0bee163625f119b25fdb0ea Mon Sep 17 00:00:00 2001
Message-Id: <9bf517748d9fed50a0bee163625f119b25fdb0ea@dist-git>
From: Laine Stump <laine@redhat.com>
Date: Thu, 26 Sep 2019 18:05:26 -0400
Subject: [PATCH] conf: utility function to update entry in def->nets array

A virDomainNetDef object in a domain's nets array might contain a
virDomainHostdevDef, and when this is the case, the domain's hostdevs
array will also have a pointer to this embedded hostdev (this is done
so that internal functions that need to perform some operation on all
hostdevs won't leave out the type='hostdev' network interfaces).

When a network device was updated with virDomainUpdateDeviceFlags(),
we were replacing the entry in the nets array (and free'ing the
original) but forgetting about the pointer in the hostdevs array
(which would then point to the now-free'd hostdev contained in the old
net object.) This often resulted in a libvirtd crash.

The solution is to add a function, virDomainNetUpdate(), called by
qemuDomainUpdateDeviceConfig(), that updates the hostdevs array
appropriately along with the nets array.

Resolves: https://bugzilla.redhat.com/1558934

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 7e490cdad6364c82b326d5d9251826c757e8f77b)
Message-Id: <20190926220526.12970-1-laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/conf/domain_conf.c   | 41 ++++++++++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h   |  1 +
 src/libvirt_private.syms |  1 +
 src/lxc/lxc_driver.c     |  6 ++++--
 src/qemu/qemu_driver.c   |  6 ++++--
 5 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0352fad245..60f426df04 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17119,6 +17119,47 @@ virDomainNetRemove(virDomainDefPtr def, size_t i)
     return net;
 }
 
+
+int
+virDomainNetUpdate(virDomainDefPtr def,
+                   size_t netidx,
+                   virDomainNetDefPtr newnet)
+{
+    size_t hostdevidx;
+    virDomainNetDefPtr oldnet = def->nets[netidx];
+    virDomainHostdevDefPtr oldhostdev = virDomainNetGetActualHostdev(oldnet);
+    virDomainHostdevDefPtr newhostdev = virDomainNetGetActualHostdev(newnet);
+
+    /*
+     * if newnet or oldnet has a valid hostdev*, we need to update the
+     * hostdevs list
+     */
+    if (oldhostdev) {
+        for (hostdevidx = 0; hostdevidx < def->nhostdevs; hostdevidx++) {
+            if (def->hostdevs[hostdevidx] == oldhostdev)
+                break;
+        }
+    }
+
+    if (oldhostdev && hostdevidx < def->nhostdevs) {
+        if (newhostdev) {
+            /* update existing entry in def->hostdevs */
+            def->hostdevs[hostdevidx] = newhostdev;
+        } else {
+            /* delete oldhostdev from def->hostdevs */
+            virDomainHostdevRemove(def, hostdevidx);
+        }
+    } else if (newhostdev) {
+        /* add newhostdev to end of def->hostdevs */
+        if (VIR_APPEND_ELEMENT(def->hostdevs, def->nhostdevs, newhostdev) < 0)
+            return -1;
+    }
+
+    def->nets[netidx] = newnet;
+    return 0;
+}
+
+
 int virDomainControllerInsert(virDomainDefPtr def,
                               virDomainControllerDefPtr controller)
 {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1c4556cecd..ef8f061ae2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3168,6 +3168,7 @@ virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
 virDomainNetDefPtr virDomainNetFindByName(virDomainDefPtr def, const char *ifname);
 bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net);
 int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
+int virDomainNetUpdate(virDomainDefPtr def, size_t netidx, virDomainNetDefPtr newnet);
 virDomainNetDefPtr virDomainNetRemove(virDomainDefPtr def, size_t i);
 void virDomainNetRemoveHostdev(virDomainDefPtr def, virDomainNetDefPtr net);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9b5e1350f0..b213e5764b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -476,6 +476,7 @@ virDomainNetSetDeviceImpl;
 virDomainNetTypeFromString;
 virDomainNetTypeSharesHostView;
 virDomainNetTypeToString;
+virDomainNetUpdate;
 virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;
 virDomainObjAssignDef;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5978183e7f..0f2ef47bb5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3526,8 +3526,10 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
                                          false) < 0)
             return -1;
 
-        virDomainNetDefFree(vmdef->nets[idx]);
-        vmdef->nets[idx] = net;
+        if (virDomainNetUpdate(vmdef, idx, net) < 0)
+            return -1;
+
+        virDomainNetDefFree(oldDev.data.net);
         dev->data.net = NULL;
         ret = 0;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d95c97928d..2852b96edd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8341,8 +8341,10 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
                                          false) < 0)
             return -1;
 
-        virDomainNetDefFree(vmdef->nets[pos]);
-        vmdef->nets[pos] = net;
+        if (virDomainNetUpdate(vmdef, pos, net))
+            return -1;
+
+        virDomainNetDefFree(oldDev.data.net);
         dev->data.net = NULL;
         break;
 
-- 
2.23.0