12:32 p.m.
A few times libvirt users manually setting mac addresses have
complained of networking failure that ends up being due to a multicast
mac address being used for a guest interface. This patch prevents that
by loggin an error and failing if a multicast mac address is
encountered in each of the three following cases:
1) domain xml <interface> mac address.
2) network xml bridge mac address.
3) network xml dhcp/host mac address.
There are several other places where a mac address cna be input that
aren't controlled in this manner because failure to do so has no
consequences (e.g., if the address will be used to search through
existing interfaces for a match).
The RNG has been updated to add multiMacAddr and uniMacAddr along with
the existing macAddr, and macAddr was switched to uniMacAddr where
appropriate.
---
docs/schemas/basictypes.rng | 17 ++++++++++++++++-
docs/schemas/domaincommon.rng | 6 +++---
docs/schemas/network.rng | 4 ++--
src/conf/domain_conf.c | 8 +++++++-
src/conf/network_conf.c | 33 ++++++++++++++++++++++++---------
src/libvirt_private.syms | 2 ++
src/util/virmacaddr.c | 13 +++++++++++++
src/util/virmacaddr.h | 3 ++-
8 files changed, 69 insertions(+), 17 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 7d7911d..9dbda4a 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -55,9 +55,24 @@
</define>
<!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC"
-->
+ <!-- The lowest bit of the 1st byte is the "multicast" bit. a
-->
+ <!-- uniMacAddr requires that bit to be 0, and a multiMacAddr -->
+ <!-- requires it to be 1. Plain macAddr will accept either. -->
+ <!-- Currently there is no use of multiMacAddr in libvirt, it -->
+ <!-- is included here for documentation/comparison purposes. -->
+ <define name="uniMacAddr">
+ <data type="string">
+ <param
name="pattern">[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}</param>
+ </data>
+ </define>
+ <define name="multiMacAddr">
+ <data type="string">
+ <param
name="pattern">[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}</param>
+ </data>
+ </define>
<define name="macAddr">
<data type="string">
- <param
name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
+ <param
name="pattern">[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}</param>
</data>
</define>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5b3e5fa..f629691 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1391,7 +1391,7 @@
<optional>
<element name="mac">
<attribute name="address">
- <ref name="macAddr"/>
+ <ref name="uniMacAddr"/>
</attribute>
<empty/>
</element>
@@ -1417,7 +1417,7 @@
<optional>
<element name="mac">
<attribute name="address">
- <ref name="macAddr"/>
+ <ref name="uniMacAddr"/>
</attribute>
<empty/>
</element>
@@ -1498,7 +1498,7 @@
<optional>
<element name="mac">
<attribute name="address">
- <ref name="macAddr"/>
+ <ref name="uniMacAddr"/>
</attribute>
<empty/>
</element>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 6e1002f..2ae879e 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -57,7 +57,7 @@
<!-- <mac> element -->
<optional>
<element name="mac">
- <attribute name="address"><ref
name="macAddr"/></attribute>
+ <attribute name="address"><ref
name="uniMacAddr"/></attribute>
<empty/>
</element>
</optional>
@@ -218,7 +218,7 @@
</zeroOrMore>
<zeroOrMore>
<element name="host">
- <attribute name="mac"><ref
name="macAddr"/></attribute>
+ <attribute name="mac"><ref
name="uniMacAddr"/></attribute>
<attribute
name="name"><text/></attribute>
<attribute name="ip"><ref
name="ipv4Addr"/></attribute>
</element>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e6d0f4b..d5def1c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4416,11 +4416,17 @@ virDomainNetDefParseXML(virCapsPtr caps,
if (macaddr) {
if (virMacAddrParse((const char *)macaddr, def->mac) < 0) {
- virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ virDomainReportError(VIR_ERR_XML_ERROR,
_("unable to parse mac address
'%s'"),
(const char *)macaddr);
goto error;
}
+ if (virMacAddrIsMulticast(def->mac)) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ _("expected unicast mac address, found multicast
'%s'"),
+ (const char *)macaddr);
+ goto error;
+ }
} else {
virCapabilitiesGenerateMac(caps, def->mac);
}
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0fa58f8..87796b5 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -419,22 +419,30 @@ virNetworkDHCPRangeDefParseXML(const char *networkName,
def->nranges++;
} else if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "host")) {
- char *mac, *name, *ip;
+ char *mac = NULL, *name = NULL, *ip;
unsigned char addr[6];
virSocketAddr inaddr;
mac = virXMLPropString(cur, "mac");
- if ((mac != NULL) &&
- (virMacAddrParse(mac, &addr[0]) != 0)) {
- virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Cannot parse MAC address '%s' in
network '%s'"),
- mac, networkName);
- VIR_FREE(mac);
- return -1;
+ if (mac != NULL) {
+ if (virMacAddrParse(mac, &addr[0]) < 0) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("Cannot parse MAC address '%s'
in network '%s'"),
+ mac, networkName);
+ VIR_FREE(mac);
+ return -1;
+ }
+ if (virMacAddrIsMulticast(addr)) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("expected unicast mac address, found
multicast '%s' in network '%s'"),
+ (const char *)mac, networkName);
+ VIR_FREE(mac);
+ return -1;
+ }
}
name = virXMLPropString(cur, "name");
if ((name != NULL) && (!c_isalpha(name[0]))) {
- virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
+ virNetworkReportError(VIR_ERR_XML_ERROR,
_("Cannot use name address '%s' in
network '%s'"),
name, networkName);
VIR_FREE(name);
@@ -990,6 +998,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
VIR_FREE(tmp);
goto error;
}
+ if (virMacAddrIsMulticast(def->mac)) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("Invalid multicast bridge mac address
'%s' in network '%s'"),
+ tmp, def->name);
+ VIR_FREE(tmp);
+ goto error;
+ }
VIR_FREE(tmp);
def->mac_specified = true;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e40c80e..9a718b4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1208,6 +1208,8 @@ virKeycodeValueTranslate;
virMacAddrCompare;
virMacAddrFormat;
virMacAddrGenerate;
+virMacAddrIsMulticast;
+virMacAddrIsUnicast;
virMacAddrParse;
diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c
index 70aef56..7b40397 100644
--- a/src/util/virmacaddr.c
+++ b/src/util/virmacaddr.c
@@ -126,3 +126,16 @@ void virMacAddrGenerate(const unsigned char *prefix,
addr[4] = virRandomBits(8);
addr[5] = virRandomBits(8);
}
+
+/* The low order bit of the first byte is the "multicast" bit. */
+bool
+virMacAddrIsMulticast(const unsigned char *addr)
+{
+ return !!(addr[0] & 1);
+}
+
+bool
+virMacAddrIsUnicast(const unsigned char *addr)
+{
+ return !(addr[0] & 1);
+}
diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h
index 278f41e..f8d3e0c 100644
--- a/src/util/virmacaddr.h
+++ b/src/util/virmacaddr.h
@@ -37,5 +37,6 @@ void virMacAddrGenerate(const unsigned char *prefix,
unsigned char *addr);
int virMacAddrParse(const char* str,
unsigned char *addr) ATTRIBUTE_RETURN_CHECK;
-
+bool virMacAddrIsUnicast(const unsigned char *addr);
+bool virMacAddrIsMulticast(const unsigned char *addr);
#endif /* __VIR_MACADDR_H__ */
--
1.7.7.6