| Message ID | CAJN_NGZ_CosR42CkUiPMd56DW2x1P6gCUAroyib62Howy1OuFA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.dmitry@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.
> 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.
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.dmitry@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
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.
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 */
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.dmitry@gmail.com> --- net/net.c | 5 +++++ net/util.c | 5 +++++ net/util.h | 2 ++ 3 files changed, 12 insertions(+)