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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
---
 src/network/bridge_driver.c |  108 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 108 insertions(+), 0 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5d7ef19..7105a58 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -42,6 +42,8 @@
 #include <stdio.h>
 #include <sys/wait.h>
 #include <sys/ioctl.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
 
 #include "virterror_internal.h"
 #include "datatypes.h"
@@ -908,6 +910,108 @@ 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;
+    char *cur, *buf = NULL;
+    enum {MAX_ROUTE_SIZE = 1024*64};
+    struct in_addr inaddress, innetmask;
+    char netaddr[32];
+
+    if (!network->def->ipAddress || !network->def->netmask)
+        return 0;
+
+    if (inet_pton(AF_INET, network->def->ipAddress, &inaddress) <= 0) {
+        networkReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse IP address '%s'"),
+                           network->def->ipAddress);
+        goto error;
+    }
+    if (inet_pton(AF_INET, network->def->netmask, &innetmask) <= 0) {
+        networkReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse netmask '%s'"),
+                           network->def->netmask);
+        goto error;
+    }
+
+    inaddress.s_addr &= innetmask.s_addr;
+    if (!inet_ntop(AF_INET, &inaddress, netaddr, sizeof(netaddr))) {
+        virReportSystemError(errno, "%s",
+                             _("failed to format network address"));
+        goto error;
+    }
+
+    /* 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 */
+    buf[len-1] = '\0';
+
+    /* First line is just headings, skip it */
+    cur = strchr(buf, '\n');
+
+    while (cur) {
+        char *data[8];
+        char *dest, *iface, *mask;
+        unsigned int addr_val, mask_val;
+        int i;
+
+        cur++;
+
+        /* Delimit interface field */
+        for (i = 0; i < sizeof(data); ++i) {
+            data[i] = cur;
+
+            /* Parse fields and delimit */
+            while(*cur > ' ') {
+                cur++;
+            }
+            *cur++ = '\0';
+        }
+
+        iface = data[0];
+        dest  = data[1];
+        mask  = data[7];
+
+        if (virStrToLong_ui(dest, NULL, 16, &addr_val) < 0) {
+            networkReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to convert network address %s"),
+                               dest);
+            goto error;
+        }
+
+        if (virStrToLong_ui(mask, NULL, 16, &mask_val) < 0) {
+            networkReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to convert network mask %s"),
+                               mask);
+            goto error;
+        }
+
+        addr_val &= mask_val;
+
+        if ((inaddress.s_addr == addr_val) && (innetmask.s_addr == mask_val)) {
+            networkReportError(VIR_ERR_INTERNAL_ERROR,
+                              _("Network %s is already in use by "
+                                "interface %s"), netaddr, iface);
+            goto error;
+        }
+
+        cur = strchr(cur, '\n');
+    }
+
+    ret = 0;
+error:
+    VIR_FREE(buf);
+    return ret;
+}
+
 static int networkStartNetworkDaemon(struct network_driver *driver,
                                      virNetworkObjPtr network)
 {
@@ -919,6 +1023,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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]