[libvirt] [PATCH v2 01/36] network: restrict usage of port management APIs

Daniel P. Berrangé berrange at redhat.com
Wed Feb 27 16:28:52 UTC 2019


The port allocation APIs are currently called unconditionally for all
types of NIC, but (mostly) only do anything for NICs with type=network.

The exception is the port allocate API which does some validation even
for NICs with type!=network. Relying on this validation is flawed,
however, since the network driver may not even be installed. IOW virt
drivers must not delegate validation to the network driver for NICs
with type != network.

This change allows us to report errors when the virtual network driver
is not registered.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 src/conf/domain_conf.c      | 26 ++++++++------
 src/libxl/libxl_domain.c    |  6 ++--
 src/libxl/libxl_driver.c    |  9 +++--
 src/lxc/lxc_driver.c        |  6 ++--
 src/lxc/lxc_process.c       |  9 +++--
 src/network/bridge_driver.c | 72 +++++++++++++++++++------------------
 src/qemu/qemu_driver.c      |  6 ++--
 src/qemu/qemu_hotplug.c     | 17 +++++----
 src/qemu/qemu_process.c     |  9 +++--
 9 files changed, 94 insertions(+), 66 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 477deb777e..a2ce9c8ef8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30803,13 +30803,11 @@ int
 virDomainNetAllocateActualDevice(virDomainDefPtr dom,
                                  virDomainNetDefPtr iface)
 {
-    /* Just silently ignore if network driver isn't present. If something
-     * has tried to use a NIC with type=network, other code will already
-     * cause an error. This ensures type=bridge doesn't break when
-     * network driver is compiled out.
-     */
-    if (!netAllocate)
-        return 0;
+    if (!netAllocate) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netAllocate(dom, iface);
 }
@@ -30839,8 +30837,11 @@ bool
 virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface,
                                    virNetDevBandwidthPtr newBandwidth)
 {
-    if (!netBandwidthChangeAllowed)
-        return 0;
+    if (!netBandwidthChangeAllowed) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netBandwidthChangeAllowed(iface, newBandwidth);
 }
@@ -30849,8 +30850,11 @@ int
 virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
                             virNetDevBandwidthPtr newBandwidth)
 {
-    if (!netBandwidthUpdate)
-        return 0;
+    if (!netBandwidthUpdate) {
+        virReportError(VIR_ERR_NO_SUPPORT, "%s",
+                       _("Virtual networking driver is not available"));
+        return -1;
+    }
 
     return netBandwidthUpdate(iface, newBandwidth);
 }
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index ffafa7967d..575748a76f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -898,7 +898,8 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
 
             /* cleanup actual device */
             virDomainNetRemoveHostdev(vm->def, net);
-            virDomainNetReleaseActualDevice(vm->def, net);
+            if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+                virDomainNetReleaseActualDevice(vm->def, net);
         }
     }
 
@@ -1055,7 +1056,8 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
          * network's pool of devices, or resolve bridge device name
          * to the one defined in the network definition.
          */
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             return -1;
 
         actualType = virDomainNetGetActualType(net);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 31b842aeee..b2aeea6468 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3397,7 +3397,8 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         goto cleanup;
 
     actualType = virDomainNetGetActualType(net);
@@ -3447,7 +3448,8 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
         vm->def->nets[vm->def->nnets++] = net;
     } else {
         virDomainNetRemoveHostdev(vm->def, net);
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
     virObjectUnref(cfg);
     return ret;
@@ -3870,7 +3872,8 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver,
  cleanup:
     libxl_device_nic_dispose(&nic);
     if (!ret) {
-        virDomainNetReleaseActualDevice(vm->def, detach);
+        if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, detach);
         virDomainNetRemove(vm->def, detachidx);
     }
     virObjectUnref(cfg);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 516a6b4de3..f0b97670e2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3856,7 +3856,8 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         return -1;
 
     actualType = virDomainNetGetActualType(net);
@@ -4410,7 +4411,8 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
     ret = 0;
  cleanup:
     if (!ret) {
-        virDomainNetReleaseActualDevice(vm->def, detach);
+        if (detach->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, detach);
         virDomainNetRemove(vm->def, detachidx);
         virDomainNetDefFree(detach);
     }
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index a3481bfa08..780e72ee56 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -224,7 +224,8 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
                                 iface->ifname));
             ignore_value(virNetDevVethDelete(iface->ifname));
         }
-        virDomainNetReleaseActualDevice(vm->def, iface);
+        if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, iface);
     }
 
     virDomainConfVMNWFilterTeardown(vm);
@@ -558,7 +559,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
         if (virLXCProcessValidateInterface(net) < 0)
             goto cleanup;
 
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             goto cleanup;
 
         type = virDomainNetGetActualType(net);
@@ -637,7 +639,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
                 ignore_value(virNetDevOpenvswitchRemovePort(
                                 virDomainNetGetActualBridgeName(iface),
                                 iface->ifname));
-            virDomainNetReleaseActualDevice(def, iface);
+            if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+                virDomainNetReleaseActualDevice(def, iface);
         }
     }
     return ret;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b3ca5b8a15..644d51829a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4381,8 +4381,11 @@ networkAllocateActualDevice(virDomainDefPtr dom,
     size_t i;
     int ret = -1;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        goto validate;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected a interface for a virtual network"));
+        goto error;
+    }
 
     virDomainActualNetDefFree(iface->data.network.actual);
     iface->data.network.actual = NULL;
@@ -4701,7 +4704,6 @@ networkAllocateActualDevice(virDomainDefPtr dom,
     if (virNetDevVPortProfileCheckComplete(virtport, true) < 0)
         goto error;
 
- validate:
     /* make sure that everything now specified for the device is
      * actually supported on this type of network. NB: network,
      * netdev, and iface->data.network.actual may all be NULL.
@@ -4720,19 +4722,11 @@ networkAllocateActualDevice(virDomainDefPtr dom,
               (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE &&
                virtport && virtport->virtPortType
                == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) {
-            if (netdef) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("an interface connecting to network '%s' "
-                                 "is requesting a vlan tag, but that is not "
-                                 "supported for this type of network"),
-                               netdef->name);
-            } else {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("an interface of type '%s' "
-                                 "is requesting a vlan tag, but that is not "
-                                 "supported for this type of connection"),
-                               virDomainNetTypeToString(iface->type));
-            }
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("an interface connecting to network '%s' "
+                             "is requesting a vlan tag, but that is not "
+                             "supported for this type of network"),
+                           netdef->name);
             goto error;
         }
     }
@@ -4748,22 +4742,20 @@ networkAllocateActualDevice(virDomainDefPtr dom,
         }
     }
 
-    if (netdef) {
-        netdef->connections++;
+    netdef->connections++;
+    if (dev)
+        dev->connections++;
+    /* finally we can call the 'plugged' hook script if any */
+    if (networkRunHook(obj, dom, iface,
+                       VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
+                       VIR_HOOK_SUBOP_BEGIN) < 0) {
+        /* adjust for failure */
+        netdef->connections--;
         if (dev)
-            dev->connections++;
-        /* finally we can call the 'plugged' hook script if any */
-        if (networkRunHook(obj, dom, iface,
-                           VIR_HOOK_NETWORK_OP_IFACE_PLUGGED,
-                           VIR_HOOK_SUBOP_BEGIN) < 0) {
-            /* adjust for failure */
-            netdef->connections--;
-            if (dev)
-                dev->connections--;
-            goto error;
-        }
-        networkLogAllocation(netdef, actualType, dev, iface, true);
+            dev->connections--;
+        goto error;
     }
+    networkLogAllocation(netdef, actualType, dev, iface, true);
 
     ret = 0;
 
@@ -4804,8 +4796,11 @@ networkNotifyActualDevice(virDomainDefPtr dom,
     size_t i;
     char *master = NULL;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        return;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected a interface for a virtual network"));
+        goto error;
+    }
 
     obj = virNetworkObjFindByName(driver->networks, iface->data.network.name);
     if (!obj) {
@@ -5037,8 +5032,11 @@ networkReleaseActualDevice(virDomainDefPtr dom,
     size_t i;
     int ret = -1;
 
-    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
-        return 0;
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected a interface for a virtual network"));
+        goto error;
+    }
 
     obj = virNetworkObjFindByName(driver->networks, iface->data.network.name);
     if (!obj) {
@@ -5529,6 +5527,12 @@ networkBandwidthUpdate(virDomainNetDefPtr iface,
     int plug_ret;
     int ret = -1;
 
+    if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Expected a interface for a virtual network"));
+        return -1;
+    }
+
     if (!networkBandwidthGenericChecks(iface, newBandwidth))
         return 0;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d5b5f8653..7820c7f5db 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11753,12 +11753,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
                    sizeof(*newBandwidth->out));
         }
 
-        if (!virDomainNetBandwidthChangeAllowed(net, newBandwidth))
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            !virDomainNetBandwidthChangeAllowed(net, newBandwidth))
             goto endjob;
 
         if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
                                   !virDomainNetTypeSharesHostView(net)) < 0 ||
-            virDomainNetBandwidthUpdate(net, newBandwidth) < 0) {
+            (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+             virDomainNetBandwidthUpdate(net, newBandwidth) < 0)) {
             ignore_value(virNetDevBandwidthSet(net->ifname,
                                                net->bandwidth,
                                                false,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 00dbff6b2a..30cbbd7449 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1318,7 +1318,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
      * network's pool of devices, or resolve bridge device name
      * to the one defined in the network definition.
      */
-    if (virDomainNetAllocateActualDevice(vm->def, net) < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        virDomainNetAllocateActualDevice(vm->def, net) < 0)
         goto cleanup;
 
     actualType = virDomainNetGetActualType(net);
@@ -1622,7 +1623,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
 
         virDomainNetRemoveHostdev(vm->def, net);
 
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
 
     VIR_FREE(nicstr);
@@ -4042,7 +4044,8 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 
         /* this function doesn't work with HOSTDEV networks yet, thus
          * no need to change the pointer in the hostdev structure */
-        virDomainNetReleaseActualDevice(vm->def, olddev);
+        if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, olddev);
         virDomainNetDefFree(olddev);
         /* move newdev into the nets list, and NULL it out from the
          * virDomainDeviceDef that we were given so that the caller
@@ -4073,7 +4076,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
      * that the changes were minor enough that we didn't need to
      * replace the entire device object.
      */
-    if (newdev)
+    if (newdev && newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK)
         virDomainNetReleaseActualDevice(vm->def, newdev);
 
     return ret;
@@ -4631,7 +4634,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
     virDomainHostdevDefFree(hostdev);
 
     if (net) {
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
         virDomainNetDefFree(net);
     }
 
@@ -4727,7 +4731,8 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
 
     qemuDomainNetDeviceVportRemove(net);
 
-    virDomainNetReleaseActualDevice(vm->def, net);
+    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+        virDomainNetReleaseActualDevice(vm->def, net);
     virDomainNetDefFree(net);
     ret = 0;
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 68c670d3f2..449f6eb9b9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3276,7 +3276,8 @@ qemuProcessNotifyNets(virDomainDefPtr def)
         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
            ignore_value(virNetDevMacVLanReserveName(net->ifname, false));
 
-        virDomainNetNotifyActualDevice(def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetNotifyActualDevice(def, net);
     }
 }
 
@@ -5462,7 +5463,8 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def)
          * network's pool of devices, or resolve bridge device name
          * to the one defined in the network definition.
          */
-        if (virDomainNetAllocateActualDevice(def, net) < 0)
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+            virDomainNetAllocateActualDevice(def, net) < 0)
             goto cleanup;
 
         actualType = virDomainNetGetActualType(net);
@@ -7304,7 +7306,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
         /* kick the device out of the hostdev list too */
         virDomainNetRemoveHostdev(def, net);
-        virDomainNetReleaseActualDevice(vm->def, net);
+        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK)
+            virDomainNetReleaseActualDevice(vm->def, net);
     }
 
  retry:
-- 
2.20.1




More information about the libvir-list mailing list