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: <Z05FQ-Z6yv16lSnY@atmark-techno.com>
Date: Tue, 3 Dec 2024 08:39:47 +0900
From: 'Dominique MARTINET' <dominique.martinet@...ark-techno.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: David Laight <David.Laight@...lab.com>,
	Oliver Neukum <oneukum@...e.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"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

Jakub Kicinski wrote on Mon, Dec 02, 2024 at 06:56:00AM -0800:
> On Mon, 2 Dec 2024 17:36:51 +0900 'Dominique MARTINET' wrote:
> > The new check however no longer cares about the address globality, and
> > just basically always renames the interface if the driver provided a
> > mac ?
> 
> Any way we can identify those devices and not read the address from 
> the device? Reading a locally administered address from the device
> seems rather pointless, we can generate one ourselves.

Would you want to regenerate a local address on every boot?

This might not have a properly allocated mac address range but this at
least has a constant mac, so the devices can be easily identified
(without looking at serial properties)
.. I guess the generation might be made to be a hash from ID_USB_SERIAL
or something like that, but if the device already provides something
stable I don't see any reason not to use it?

(With that said, I don't see anything in `udevadm info` that'd easily
point at this being a modem point to point device under the wraps..)

> > If that is what was intended, I am fine with this, but I think these
> > local ppp usb interfaces are rather common in the cheap modem world.
> 
> Which will work, as long as they are marked appropriately; that is
> marked with FLAG_POINTTOPOINT.

Hmm, but the check here was either FLAG_POINTTOPOINT being unset or not
locally administered address, so to keep the usb0 name we need both?

>             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)))
>                       strscpy(net->name, "eth%d", sizeof(net->name));

i.e., something that didn't have FLAG_POINTTOPOINT in the first place
would not get into this mac consideration, so it must be set.

My problematic device here has FLAG_POINTTOPOINT and a (locally
admistered) mac address set, so it was not renamed up till now,
but the new check makes the locally admistered mac address being set
mean that it is no longer eligible to keep the usbX name.

... Would this check just be fine without any mac check at all?
e.g. anything that's not flagged as point to point will be renamed ethX,
and all non ethernet or point to point keep usbX.

-- 
Dominique