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]

Date: Tue, 2 Jan 2024 19:08:32 +0100
From: Ezra Buehler <ezra@...yb.ch>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, 
	Tristram Ha <Tristram.Ha@...rochip.com>, Michael Walle <michael@...le.cc>, 
	Jesse Brandeburg <jesse.brandeburg@...el.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: mdio: Prevent Clause 45 scan on SMSC PHYs

Hi Andrew,

On Tue, Jan 2, 2024 at 4:50 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> I do however disagree with this statement in the original patch:
>
> > AFAICT all SMSC/Microchip PHYs are Clause 22 devices.

Excuse my ignorance, I am by no means an expert here. I guess what I
wanted to say was:

By skimming over some datasheets for similar SMSC/Microchip PHYs, I
could not find any evidence that they support Clause 45 scanning
(other than not responding).

> drivers/net/phy/smsc.c has a number of phy_write_mmd()/phy_read_mmd()
> in it. But that device has a different OUI.

I guess I am confused here, AFAICT all PHYs in smsc.c have the same OUI
(phy_id >> 10).

> However, the commit message says:
>
> > 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.
>
> So we need to clarify the real issue here. Does the C45 scan work
> correctly, but the board watchdog timer is too short and fires? We
> should not be extended this workaround when its a bad watchdog
> configuration issue...

Changing the watchdog configuration is not an option here. We are
talking about a slowdown of several seconds here, that is not acceptable
on its own.

Cheers,
Ezra.

On Tue, Jan 2, 2024 at 4:50 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > Excluding all PHY's from a vendor for me is a quite big hammer.
>
> Maybe it serves them right for getting this wrong?
>
> Micrel is now part of Microchip, so in effect, this is the same broken
> IP just with a different name and OUI. We have not seen any other
> vendor get this wrong.
>
> I do however disagree with this statement in the original patch:
>
> > AFAICT all SMSC/Microchip PHYs are Clause 22 devices.
>
> drivers/net/phy/smsc.c has a number of phy_write_mmd()/phy_read_mmd()
> in it. But that device has a different OUI.
>
> > I think we should make this more granular.
> > And mdio-bus.c including micrel_phy.h also isn't too nice.
> > Maybe we should move all OUI definitions in drivers to a
> > core header. Because the OUI seems to be all we need from
> > these headers.
>
> That does seem a big change to make for 'one' broken PHY IP.
>
> However, the commit message says:
>
> > 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.
>
> So we need to clarify the real issue here. Does the C45 scan work
> correctly, but the board watchdog timer is too short and fires? We
> should not be extended this workaround when its a bad watchdog
> configuration issue...
>
>        Andrew