From dd52444f231c205fb624fdeb59145722ba887aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 26 Jul 2018 11:37:32 +0100 Subject: [PATCH] network: restrict usage of port management APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Cole Robinson Signed-off-by: Daniel P. Berrangé --- 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 17e8975be3..2520728493 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30307,13 +30307,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); } @@ -30343,8 +30341,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); } @@ -30353,8 +30354,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 6e4bf68921..adc6d6d104 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -904,7 +904,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); } } @@ -1061,7 +1062,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 a9edc8211d..d5cd3fc834 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3390,7 +3390,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); @@ -3440,7 +3441,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; @@ -3863,7 +3865,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 e981f8e901..027c4fd990 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3834,7 +3834,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); @@ -4388,7 +4389,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 e0729a24bf..7849aaf5b9 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 a091ed7181..cf1abe6bb2 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4391,8 +4391,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 an interface for a virtual network")); + goto error; + } virDomainActualNetDefFree(iface->data.network.actual); iface->data.network.actual = NULL; @@ -4711,7 +4714,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. @@ -4730,19 +4732,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; } } @@ -4758,22 +4752,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; @@ -4814,8 +4806,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 an interface for a virtual network")); + goto error; + } obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); if (!obj) { @@ -5047,8 +5042,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 an interface for a virtual network")); + goto error; + } obj = virNetworkObjFindByName(driver->networks, iface->data.network.name); if (!obj) { @@ -5539,6 +5537,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 an 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 4510b0ce60..811762f307 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11623,12 +11623,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 ef14b1977c..48489d3cdc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1385,7 +1385,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); @@ -1689,7 +1690,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); @@ -4126,7 +4128,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 @@ -4157,7 +4160,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; @@ -4750,7 +4753,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); } @@ -4852,7 +4856,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 55f4074ea1..8b0ae901e9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3288,7 +3288,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); } } @@ -5474,7 +5475,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); @@ -7321,7 +7323,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: -- GitLab