From a83fe2c23efad190a1e00e448f607fe032650fd6 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Thu, 20 May 2010 19:31:16 -0400 Subject: [PATCH] network: bridge: Don't start network if it collides with host routing Fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=235961 If using the default virtual network, an easy way to lose guest network connectivity is to install libvirt inside the VM. The autostarted default network inside the guest collides with host virtual network routing. This is a long standing issue that has caused users quite a bit of pain and confusion. On network startup, parse /proc/net/route and compare the requested IP+netmask against host routing destinations: if any matches are found, refuse to start the network. v2: Drop sscanf, fix a comment typo, comment that function could use libnl instead of /proc v3: Consider route netmask. Compare binary data rather than convert to string. v4: Return to using sscanf, drop inet functions in favor of virSocket, parsing safety checks. Don't make parse failures fatal, in case expected format changes. v5: Try and continue if we receive unexpected. Delimit parsed lines to prevent scanning past newline --- src/network/bridge_driver.c | 113 ++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5d7ef19241..7ab3f3eec4 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -57,6 +57,7 @@ #include "bridge.h" #include "logging.h" #include "dnsmasq.h" +#include "util/network.h" #define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network" #define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network" @@ -908,6 +909,114 @@ cleanup: return ret; } +#define PROC_NET_ROUTE "/proc/net/route" + +/* XXX: This function can be a lot more exhaustive, there are certainly + * other scenarios where we can ruin host network connectivity. + * XXX: Using a proper library is preferred over parsing /proc + */ +static int networkCheckRouteCollision(virNetworkObjPtr network) +{ + int ret = -1, len; + unsigned int net_dest; + char *cur, *buf = NULL; + enum {MAX_ROUTE_SIZE = 1024*64}; + virSocketAddr inaddress, innetmask; + + if (!network->def->ipAddress || !network->def->netmask) + return 0; + + if (virSocketParseAddr(network->def->ipAddress, &inaddress, 0) < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse IP address '%s'"), + network->def->ipAddress); + goto error; + } + + if (virSocketParseAddr(network->def->netmask, &innetmask, 0) < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse netmask '%s'"), + network->def->netmask); + goto error; + } + + if (inaddress.stor.ss_family != AF_INET || + innetmask.stor.ss_family != AF_INET) { + /* Only support collision check for IPv4 */ + goto out; + } + + net_dest = (inaddress.inet4.sin_addr.s_addr & + innetmask.inet4.sin_addr.s_addr); + + /* Read whole routing table into memory */ + if ((len = virFileReadAll(PROC_NET_ROUTE, MAX_ROUTE_SIZE, &buf)) < 0) + goto error; + + /* Dropping the last character shouldn't hurt */ + if (len > 0) + buf[len-1] = '\0'; + + VIR_DEBUG("%s output:\n%s", PROC_NET_ROUTE, buf); + + if (!STRPREFIX (buf, "Iface")) + goto out; + + /* First line is just headings, skip it */ + cur = strchr(buf, '\n'); + if (cur) + cur++; + + while (cur) { + char iface[17], dest[128], mask[128]; + unsigned int addr_val, mask_val; + int num; + + /* NUL-terminate the line, so sscanf doesn't go beyond a newline. */ + char *nl = strchr(cur, '\n'); + if (nl) { + *nl++ = '\0'; + } + + num = sscanf(cur, "%16s %127s %*s %*s %*s %*s %*s %127s", + iface, dest, mask); + cur = nl; + + if (num != 3) { + VIR_DEBUG("Failed to parse %s", PROC_NET_ROUTE); + continue; + } + + if (virStrToLong_ui(dest, NULL, 16, &addr_val) < 0) { + VIR_DEBUG("Failed to convert network address %s to uint", dest); + continue; + } + + if (virStrToLong_ui(mask, NULL, 16, &mask_val) < 0) { + VIR_DEBUG("Failed to convert network mask %s to uint", mask); + continue; + } + + addr_val &= mask_val; + + if ((net_dest == addr_val) && + (innetmask.inet4.sin_addr.s_addr == mask_val)) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("Network %s/%s is already in use by " + "interface %s"), + network->def->ipAddress, + network->def->netmask, iface); + goto error; + } + } + +out: + ret = 0; +error: + VIR_FREE(buf); + return ret; +} + static int networkStartNetworkDaemon(struct network_driver *driver, virNetworkObjPtr network) { @@ -919,6 +1028,10 @@ static int networkStartNetworkDaemon(struct network_driver *driver, return -1; } + /* Check to see if network collides with an existing route */ + if (networkCheckRouteCollision(network) < 0) + return -1; + if ((err = brAddBridge(driver->brctl, network->def->bridge))) { virReportSystemError(err, _("cannot create bridge '%s'"),