Re: [Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Stefan Hajnoczi]

On Thu, Oct 17, 2013 at 07:06:28PM +0400, Dmitry Krivenok wrote:
 Added explicit check of MAC address specified via macaddr option.
 Multicast MAC addresses are no longer allowed.
 This fixes bug #495566.
 
 Signed-off-by: Dmitry V. Krivenok krivenok.dmi...@gmail.com
 ---
  net/net.c  | 5 +
  net/util.c | 5 +
  net/util.h | 2 ++
  3 files changed, 12 insertions(+)

I dropped the ? true : false when merging the patch.

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan

Re: [Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Stefan Hajnoczi]

On Thu, Oct 17, 2013 at 07:06:28PM +0400, Dmitry Krivenok wrote:
 +bool net_macaddr_is_multicast(uint8_t *macaddr)
 +{
 +return (macaddr[0] % 2) ? true : false;
 +}

Please use is_multicast_ether_addr() instead.  Thanks Amos for pointing
out this function is duplicated.

I have dropped this patch and will merge the next revision instead.

[Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Dmitry Krivenok]

Added explicit check of MAC address specified via macaddr option.
Multicast MAC addresses are no longer allowed.
This fixes bug #495566.

Signed-off-by: Dmitry V. Krivenok krivenok.dmi...@gmail.com
---
 net/net.c  | 5 +
 net/util.c | 5 +
 net/util.h | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/net/net.c b/net/net.c
index c330c9a..b3a42e5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -689,6 +689,11 @@ static int net_init_nic(const NetClientOptions
*opts, const char *name,
 error_report(invalid syntax for ethernet address);
 return -1;
 }
+if (nic-has_macaddr 
+net_macaddr_is_multicast(nd-macaddr.a)) {
+error_report(NIC cannot have multicast MAC address (odd 1st byte));
+return -1;
+}
 qemu_macaddr_default_if_unset(nd-macaddr);

 if (nic-has_vectors) {
diff --git a/net/util.c b/net/util.c
index 7e95076..b86ac03 100644
--- a/net/util.c
+++ b/net/util.c
@@ -58,3 +58,8 @@ int net_parse_macaddr(uint8_t *macaddr, const char *p)

 return 0;
 }
+
+bool net_macaddr_is_multicast(uint8_t *macaddr)
+{
+return (macaddr[0] % 2) ? true : false;
+}
diff --git a/net/util.h b/net/util.h
index 10c7da9..4581cb7 100644
--- a/net/util.h
+++ b/net/util.h
@@ -26,7 +26,9 @@
 #define QEMU_NET_UTIL_H

 #include stdint.h
+#include stdbool.h

 int net_parse_macaddr(uint8_t *macaddr, const char *p);
+bool net_macaddr_is_multicast(uint8_t *macaddr);

 #endif /* QEMU_NET_UTIL_H */
-- 
1.8.3

Re: [Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Eric Blake]

On 10/17/2013 09:06 AM, Dmitry Krivenok wrote:
 Added explicit check of MAC address specified via macaddr option.
 Multicast MAC addresses are no longer allowed.
 This fixes bug #495566.
 
 Signed-off-by: Dmitry V. Krivenok krivenok.dmi...@gmail.com
 ---

  }
 +
 +bool net_macaddr_is_multicast(uint8_t *macaddr)
 +{
 +return (macaddr[0] % 2) ? true : false;

Personally, I find 'expr ? true : false' rather verbose; why not just:

return macaddr[0] % 2;

But as you're not the first person to do this (a quick grep found two
other offenders in the code base), it's not a strong reason for a respin.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Dmitry Krivenok]

 Personally, I find 'expr ? true : false' rather verbose; why not just:

 return macaddr[0] % 2;

I agree, your variant is shorter and easier to read.

[Qemu-devel] [PATCH] net: disallow to specify multicast MAC address

[Dmitry Krivenok]

Added explicit check of MAC address specified via macaddr option.
Multicast MAC addresses are no longer allowed.
This fixes bug #495566.

Signed-off-by: Dmitry V. Krivenok krivenok.dmi...@gmail.com
---
 net/net.c  | 5 +
 net/util.c | 5 +
 net/util.h | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/net/net.c b/net/net.c
index c330c9a..b3a42e5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -689,6 +689,11 @@ static int net_init_nic(const NetClientOptions
*opts, const char *name,
 error_report(invalid syntax for ethernet address);
 return -1;
 }
+if (nic-has_macaddr 
+net_macaddr_is_multicast(nd-macaddr.a)) {
+error_report(NIC cannot have multicast MAC address (odd 1st byte));
+return -1;
+}
 qemu_macaddr_default_if_unset(nd-macaddr);

 if (nic-has_vectors) {
diff --git a/net/util.c b/net/util.c
index 7e95076..b86ac03 100644
--- a/net/util.c
+++ b/net/util.c
@@ -58,3 +58,8 @@ int net_parse_macaddr(uint8_t *macaddr, const char *p)

 return 0;
 }
+
+bool net_macaddr_is_multicast(uint8_t *macaddr)
+{
+return (macaddr[0] % 2) ? true : false;
+}
diff --git a/net/util.h b/net/util.h
index 10c7da9..4581cb7 100644
--- a/net/util.h
+++ b/net/util.h
@@ -26,7 +26,9 @@
 #define QEMU_NET_UTIL_H

 #include stdint.h
+#include stdbool.h

 int net_parse_macaddr(uint8_t *macaddr, const char *p);
+bool net_macaddr_is_multicast(uint8_t *macaddr);

 #endif /* QEMU_NET_UTIL_H */
-- 
1.8.3