[libvirt] [PATCH v5] network: bridge: Don't start network if it collides with host routing

[Cole Robinson]

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

Signed-off-by: Cole Robinson <crobinso at redhat.com>
---
 src/network/bridge_driver.c |  113 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5d7ef19..7ab3f3e 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'"),
-- 
1.6.6.1