netdev - Re: [PATCH net] net: mdio: Prevent Clause 45 scan on SMSC PHYs

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: <cf86ad14e88362952c9f746dfb04cff4@walle.cc>
Date: Tue, 02 Jan 2024 09:50:39 +0100
From: Michael Walle <michael@...le.cc>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: ezra@...ergy-village.org, Andrew Lunn <andrew@...n.ch>, Russell King
 <linux@...linux.org.uk>, Tristram Ha <Tristram.Ha@...rochip.com>, Jesse
 Brandeburg <jesse.brandeburg@...el.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: mdio: Prevent Clause 45 scan on SMSC PHYs

Hi,

>> Since commit 1a136ca2e089 ("net: mdio: scan bus based on bus
>> capabilities for C22 and C45") our AT91SAM9G25-based GARDENA smart
>> Gateway will no longer boot.
>> 
>> Prior to the mentioned change, probe_capabilities would be set to
>> MDIOBUS_NO_CAP (0) and therefore, no Clause 45 scan was performed.
>> Running a Clause 45 scan on an SMSC/Microchip LAN8720A PHY will (at
>> least with our setup) considerably slow down kernel startup and
>> ultimately result in a board reset.
>> 
>> AFAICT all SMSC/Microchip PHYs are Clause 22 devices. Some have a
>> "Clause 45 protection" feature (e.g. LAN8830) and others like the
>> LAN8804 will explicitly state the following in the datasheet:
>> 
>>     This device may respond to Clause 45 accesses and so must not be
>>     mixed with Clause 45 devices on the same MDIO bus.

If implemented correctly, c22 phys should never respond to c45
accesses. Correct? So the "Clause 45 protection" sounds like the
normal behavior here and the "may respond to c45 accesses" looks
like it's broken.

> I'm not convinced that some heuristic based on vendors is a
> sustainable approach. Also I'd like to avoid (as far as possible)
> that core code includes vendor driver headers. Maybe we could use
> a new PHY driver flag. Approaches I could think of:
> 
> Approach 1:
> Add a PHY driver flag to state: PHY is not c45-access-safe
> Then c45 scanning would be omitted if at least one c22 PHY
> with this flag was found.
> 
> Approach 2:
> Add a PHY driver flag to state: PHY is c45-access-safe
> Then c45 scanning would only be done if all found c22 devices
> 
> Not sure which options have been discussed before. Any feedback
> welcome.

I had a similar idea and IIRC Andrew said this would be a layering
violation. But I can't find the thread anymore.

> Related: How common are setups where c22 and c45 devices are attached
> to a single MDIO bus?

At least we have boards which has c22 and c45 PHYs on one bus. And
on one board, we even have a Micrel/Microchip PHY on this bus, which
forces us to use c22-over-c45 for the c45 PHY. I really need to repost 
my
c45-over-c22 series, although there was no consensus there 
unfortunately.

-michael