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. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/network/bridge_driver.c | 104 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 104 insertions(+), 0 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5d7ef19..9dc956b 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,105 @@ 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); + + /* First line is just headings, skip it */ + if (STRPREFIX (buf, "Iface")) + cur = strchr(buf, '\n'); + + while (cur) { + char dest[128], iface[17], mask[128]; + unsigned int addr_val, mask_val; + int num; + + cur++; + num = sscanf(cur, "%16s %127s %*s %*s %*s %*s %*s %127s", + iface, dest, mask); + if (num != 3) { + VIR_DEBUG("Failed to parse %s", PROC_NET_ROUTE); + break; + } + + if (virStrToLong_ui(dest, NULL, 16, &addr_val) < 0) { + VIR_DEBUG("Failed to convert network address %s to uint", dest); + break; + } + + if (virStrToLong_ui(mask, NULL, 16, &mask_val) < 0) { + VIR_DEBUG("Failed to convert network mask %s to uint", mask); + break; + } + + 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; + } + + cur = strchr(cur, '\n'); + } + +out: + ret = 0; +error: + VIR_FREE(buf); + return ret; +} + static int networkStartNetworkDaemon(struct network_driver *driver, virNetworkObjPtr network) { @@ -919,6 +1019,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'"), -- 1.6.6.1