netdev - Re: Race in PHY subsystem? Attaching to PHY devices before they get probed

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: <c3282db2-b1e5-422a-b62f-c081042da9de@lunn.ch>
Date: Wed, 24 Jan 2024 18:52:35 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
	Rafał Miłecki <zajec5@...il.com>,
	Network Development <netdev@...r.kernel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Robert Marko <robimarko@...il.com>,
	Daniel Golle <daniel@...rotopia.org>
Subject: Re: Race in PHY subsystem? Attaching to PHY devices before they get
 probed

> Well if we start having more and more PHY that require loading a FW then
> this will become a big problem...
> 
> I wasted some good time on this and if the MDIO is slow enough loading
> the FW can take even 100s resulting in probe still having to finish and
> config_init called later.

If its going to take 100s of seconds, i don't think we can have
'ip set link up' stall for that long. It needs to return an error code,
and hopefully a useful error message asking the user to throw the machine
away and get a better one!

> Since the FW has not been loaded config_init returns bad data and fails
> to configure. (and after a while probe is complete)
> 
> I don't know if it would be ok as a solution but I think moving the
> fw_load call in the config_init seems to "handle" this problem but IMHO
> it's still and hack for a fragile implementation.

Just throwing out ideas, but i think we need to split this into two
different use cases:

1) Firmware loading is fast, only 1-2 seconds. We can block operations
on the PHY until it is ready

2) Firmware loading is slow, we return -EBUSY or -EAGAIN, or similar.

Maybe we could add a struct completion to struct phy_device, which has
compete() called on it when probe finishes. phy_attach_direct() does a
wait_for_completion_timeout()?

This is assuming we cannot actually fix phylib to correctly use the
driver model, PHYs are not visible until probe is complete, and the
MAC drivers can handle that.

	Andrew