Re: [PATCH v2] platform: ignore permanent MAC addresses of all ones (FF:FF:FF:FF:FF:FF)

[Dan Williams <dcbw redhat com>]

On Tue, 2016-01-26 at 13:41 -0500, Chuck Anderson wrote:
> While we're on this topic, what about other types of invalid MAC
> addresses?  Like ones with the Multicast bit set (odd first octet),
> or
> ones with the LAA (Locallay Administered Address) bit set (2nd least
> significant bit of first octet, e,g, x2: x6: xa: xe:).
>
> For Multicast Ethernet addresses, I don't see how that could ever
> work
> and it would screw up any network it gets attached to because normal
> switches can't learn them.
>
> For LAA, they aren't supposed to be permanently programmed into a
> device, but there are unfortunately cheap devices out there that use
> non-IEEE-registered OUIs, some of them LAA, so it may be
> unfortunately
> unworkable to ban those.  Network bridge devices and other software
> devices may also use them.

Yes, Multicast should be rejected too.  There are also some older
drivers that used special addresses to indicate some states that we
used to ignore as well.

But I think we need to allow LAA if the device reports it :(

Dan

> On Tue, Jan 26, 2016 at 11:33:49AM -0600, Dan Williams wrote:
> > Drivers are stupid, and just like the platform ignores an all zeros
> > permanent address, so should it ignore all ones.
> >
> > NetworkManager[509]: <debug> [1453743778.854919] [devices/nm
> > -device.c:8885] nm_device_update_hw_address(): [0x190370] (eth0):
> > hardware address now 86:18:52:xx:xx:xx
> > NetworkManager[509]: <debug> [1453743778.855438] [devices/nm
> > -device.c:9138] constructed(): [0x190370] (eth0): read initial MAC
> > address 86:18:52:xx:xx:xx
> > NetworkManager[509]: <debug> [1453743778.861602] [devices/nm
> > -device.c:9148] constructed(): [0x190370] (eth0): read permanent
> > MAC address FF:FF:FF:FF:FF:FF
> > ---
> >  src/platform/nm-platform-utils.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm
> > -platform-utils.c
> > index 953ac8c..b0b0b56 100644
> > --- a/src/platform/nm-platform-utils.c
> > +++ b/src/platform/nm-platform-utils.c
> > @@ -142,7 +142,8 @@ nmp_utils_ethtool_get_permanent_address (const
> > char *ifname,
> >             struct ethtool_perm_addr e;
> >             guint8 _extra_data[NM_UTILS_HWADDR_LEN_MAX + 1];
> >     } edata;
> > -   guint zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> > +   static const guint8 zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0
> > };
> > +   static guint8 ones[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> >  
> >     if (!ifname)
> >             return FALSE;
> > @@ -161,6 +162,12 @@ nmp_utils_ethtool_get_permanent_address (const
> > char *ifname,
> >     if (memcmp (edata.e.data, zeros, edata.e.size) == 0)
> >             return FALSE;
> >  
> > +   /* Some drivers return a permanent address of all ones.
> > Reject that too */
> > +   if (G_UNLIKELY (ones[0] != 0xFF))
> > +           memset (ones, 0xFF, sizeof (ones));
> > +   if (memcmp (edata.e.data, ones, edata.e.size) == 0)
> > +           return FALSE;
> > +
> >     memcpy (buf, edata.e.data, edata.e.size);
> >     *length = edata.e.size;
> >     return TRUE;