linux-kernel - Re: [PATCH net-next 1/1] net: mdio: reset PHY before attempting to access ID register

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: <aSgzRMf1LZycQoni@debianbuilder>
Date: Thu, 27 Nov 2025 12:17:24 +0100
From: Buday Csaba <buday.csaba@...lan.hu>
To: Jakub Kicinski <kuba@...nel.org>
CC: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
	<pabeni@...hat.com>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/1] net: mdio: reset PHY before attempting to
 access ID register

On Tue, Nov 25, 2025 at 06:43:35PM -0800, Jakub Kicinski wrote:
> On Tue, 25 Nov 2025 12:15:51 +0100 Buday Csaba wrote:
> > When the ID of an Ethernet PHY is not provided by the 'compatible'
> > string in the device tree, its actual ID is read via the MDIO bus.
> > For some PHYs this could be unsafe, since a hard reset may be
> > necessary to safely access the MDIO registers.
> 
> You may be missing exports because it doesn't build with allmodconfig:
> 
> ERROR: modpost: "mdio_device_register_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> ERROR: modpost: "mdio_device_unregister_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> -- 
> pw-bot: cr
> 

I require some advice on how to do it properly.
The high level functionality belongs to either fwnode_mdio.c or maybe
phy_device.c

The latter may be better, since get_phy_device() already performs some
kind of recovery for PHYs with a certain unexpected behaviour.

But the low level functionality: registering the reset properties are
now in their proper place in mdio_device.c

These three source files build into three different modules, so I only
see the following options:

a) make mdio_device_register_reset() and its counterpart public
   But we have already agreed that they should not be, and keep them
   internal

b) create a new helper function in mdio_device.c, and make that public
   This could work, but then what is the point of hiding
   mdio_device_register_reset()? My idea was something like
   mdio_device_reset_strobe(), which calls register/unregister reset
   while also performing the assertion/deassertion of the reset.
   But such a function is unsafe on an already established mdio_device,
   so making that exported may be questionable as well.
   It is possible to work around that, but then it is getting out of hand
   fast, and does not follow the keep it simple and stupid principle.

c) what about using EXPORT_SYMBOL_FOR_MODULES() on the problematic
   functions? Are there any objections against it?
   This type of export is rarely used in the kernel, so I am uncertain
   about that. Is using it on functions declared in private headers
   also discouraged?

Thank you in advance,
Csaba