Re: [PATCH net-next v4 3/3] net: phy: own phydev->psec via PSE notifier and remove fwnode_mdio hook

[Kory Maincent]

On 8/23/26 19:09, Carlo Szelinsky wrote:
> > This patch causes a deadlock with the etop driver (lantiq arx100 soc
> > family).
>
> Thanks Aleksander, and thanks for testing it on real hardware. It's a real
> deadlock and you're right about the cause.
>
> The cause is that this patch makes phy_device_register() take rtnl_lock()
> around the psec attach. etop registers its MDIO bus from ndo_init
> (ltq_etop_mdio_init -> mdiobus_register -> mdiobus_scan ->
> phy_device_register), and ndo_init already runs under rtnl inside
> register_netdevice(). So the second rtnl_lock() deadlocks. It only shows up
> on drivers that register the bus from ndo_init - most do it from probe,
> outside rtnl - which is why it didn't turn up here earlier.

This is a known issue that some netdev hold the rtnl during phy register 
and removal and some don't depending on the path.
I faced the same issue when working on timestamping, the reply from net 
maintainers was that we should add rtnlock in the netdev drivers that 
don't take the lock. I started the work but never finished it due to 
missing available time.

> I don't think we should revert. As Jonas said, this should be fixed in the
> patch. My idea for the next version is to stop using rtnl for the psec
> attach/detach and use a small dedicated mutex instead. A private lock can't
> recurse against the caller's rtnl, so the etop case would go away.
>
> Kory, others - does that sound like the right direction to you?
>
> One thing I want to flag: rtnl there wasn't only guarding the attach, it was
> also keeping the ethtool code that reads phydev->psec from racing the detach
> that frees it. So the mutex would need to sit in two places - the phy side and
> the ethtool paths in net/ethtool/pse-pd.c - otherwise the deadlock is gone but
> the use-after-free comes back. Just so we pick the right approach.

You can try it, but don't forget to enable PROVE_LOCKING to be sure it 
works well.

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com