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

Re: [libvirt] [PATCH v2] dots should be valid characters in interface names



On 05/19/2010 10:05 PM, Laine Stump wrote:
On 05/19/2010 10:41 PM, Charles Duffy wrote:
The attached one-liner (built against RHEL6b1's libvirt but still
applicable against current master as of 10c681622) fixes this issue.

I think we need to add ":" there as well (at least - maybe there are
others), don't we?

In dev_valid_name() in net/core/dev.c, the following restrictions are enforced as of 2.6.27:

- Empty strings are disallowed
- Length must not meet or exceed 16 (IFNAMSIZ)
- '.' and '..' are each invalid as freestanding names (but accepted as part of a longer string)
- whitespace and '/' characters are prohibited at any point in the string

...and those are the only limitations in play.

Frankly, I'm a bit surprised colons are allowed -- my intuition was that they would be permitted only for named IPs rather than devices -- but such is the logic given here, and a bit of experimentation shows that the kernel accepts them (though they trigger bugs in iproute2, which appears in at least one place to make the same assumption I did; fun!)

A revised patch is attached. This lifts its logic from its kernel counterpart, and is updated only to permit forward slashes (which, while disallowed for interface names with the kernel, are required for *device* names -- for which the ESX driver happens to overload this field. Ugh).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 20c9c51..142a37b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <ctype.h>
 #include <fcntl.h>
 #include <dirent.h>
 #include <sys/time.h>
@@ -1802,8 +1803,29 @@ cleanup:
 
 
 static bool
-isValidIfname(const char *ifname) {
-    return ifname[strspn(ifname, VALID_IFNAME_CHARS)] == 0;
+isValidIfname(const char *ifname)
+{
+    if (*ifname == 0)
+        return false;
+    if (strlen(ifname) >= IFNAME_MAX_LENGTH)
+        return false;
+    if (!strcmp(ifname, ".") || !strcmp(ifname, ".."))
+        return false;
+    while (*ifname) {
+        /* in the kernel, forward slashes aren't allowed; however, the vmxnet
+         * driver depends on them, so we're slightly more permissive and
+         * disallow only spaces -- however, this will break for drivers other
+         * than VMware.
+         *
+         * Enforcing IFNAME_MAX_LENGTH is probably not appropriate for VMware
+         * either. Perhaps it should be using a different attribute name?
+         */
+        if (isspace(*ifname)) {
+            return false;
+        }
+        ifname++;
+    }
+    return true;
 }
 
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fadc8bd..b68818e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -298,8 +298,8 @@ struct _virDomainNetDef {
     virNWFilterHashTablePtr filterparams;
 };
 
-# define VALID_IFNAME_CHARS \
- "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_/"
+/* corresponds with kernel constant IFNAMSIZ from <linux/if.h> */
+# define IFNAME_MAX_LENGTH 16
 
 enum virDomainChrTargetType {
     VIR_DOMAIN_CHR_TARGET_TYPE_NULL = 0,

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