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

[Chuck Anderson <cra WPI EDU>]

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.

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;
> -- 
> 2.4.3