netdev - RE: [PATCH net] net: usb: usbnet: fix name regression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e53631b5108b4d0fb796da2a56bc137f@AcuMS.aculab.com>
Date: Mon, 2 Dec 2024 08:17:59 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Dominique MARTINET' <dominique.martinet@...ark-techno.com>, Oliver Neukum
	<oneukum@...e.com>
CC: "edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Greg Thelen
	<gthelen@...gle.com>, John Sperbeck <jsperbeck@...gle.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>
Subject: RE: [PATCH net] net: usb: usbnet: fix name regression

From: Dominique MARTINET
> Sent: 02 December 2024 03:50
> 
> Hi,
> 
> Oliver Neukum wrote on Thu, Oct 17, 2024 at 09:18:37AM +0200:
> > The fix for MAC addresses broke detection of the naming convention
> > because it gave network devices no random MAC before bind()
> > was called. This means that the check for the local assignment bit
> > was always negative as the address was zeroed from allocation,
> > instead of from overwriting the MAC with a unique hardware address.
> 
> So we hit the exact inverse problem with this patch: our device ships an
> LTE modem which exposes a cdc-ethernet interface that had always been
> named usb0, and with this patch it started being named eth1, breaking
> too many hardcoded things expecting the name to be usb0 and making our
> devices unable to connect to the internet after updating the kernel.

Erm does that mean your modem has a locally administered MAC address?
It really shouldn't.

> Long term we'll probably add an udev rule or something to make the name
> explicit in userspace and not risk this happening again, but perhaps
> there's a better way to keep the old behavior?
> 
> (In particular this hit all stable kernels last month so I'm sure we
> won't be the only ones getting annoyed with this... Perhaps reverting
> both patches for stable branches might make sense if no better way
> forward is found -- I've added stable@ in cc for heads up/opinions)
> 
> 
> > +++ b/drivers/net/usb/usbnet.c
> > @@ -1767,7 +1767,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> >  		// can rename the link if it knows better.
> >  		if ((dev->driver_info->flags & FLAG_ETHER) != 0 &&
> >  		    ((dev->driver_info->flags & FLAG_POINTTOPOINT) == 0 ||
> > -		     (net->dev_addr [0] & 0x02) == 0))
> > +		     /* somebody touched it*/
> > +		     !is_zero_ether_addr(net->dev_addr)))
> 
> ... or actually now I'm looking at it again, perhaps is the check just
> backwards, or am I getting this wrong?
> previous check was rename if (mac[0] & 0x2 == 0), which reads to me as
> "nobody set the 2nd bit"
> new check now renames if !is_zero, so renames if it was set, which is
> the opposite?...

The 2nd bit (aka mac[0] & 2) is the 'locally administered' bit.
The intention of the standard was that all manufacturers would get
a valid 14-bit OUI and the EEPROM (or equivalent) would contain an
addresses with that bit clear, such addresses should be globally unique.
Alternatively the local network administrator could assign an address
with that bit set, required by protocols like DECnet.

This has never actually been strictly true, a few manufacturers used
'locally administered addresses' (02:cf:1f:xx:xx:xx comes to mind)
and systems typically allow any (non-broadcast) be set.

So basing any decision on whether a MAC address is local or global
is always going to be confusing.

Linux will allocate a random (locally administered) address if none
is found - usually due to a corrupt eeprom.

	David

> 
> >  			strscpy(net->name, "eth%d", sizeof(net->name));
> >  		/* WLAN devices should always be named "wlan%d" */
> 
> Thanks,
> --
> Dominique Martinet

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)