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,
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list