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

[Carlo Szelinsky <github@szelinsky.de>]

Hi Paolo,

Thanks, I traced this and I think the review is right.

A phy that has been device_del()'d but is still pinned (an attached
netdev holds a get_device() from phy_attach_direct()) is off the
mdio_bus_type klist, so the PSE_UNREGISTERED walk (bus_for_each_dev)
does not see it and phy_pse_detach_one() never clears its phydev->psec.
When it is finally released, after pse_release_pis() has already freed
pcdev->pi, __pse_control_release() reads pcdev->pi[] and pcdev->owner
-> use-after-free. So the commit message is wrong for the off-klist
case: bus_for_each_dev() only defers the release for phys it can still
reach.

A few questions so I fix it the right way:

1. The trigger I can see is unbinding the MDIO bus while a netdev still
   has the phy attached (mdiobus_unregister -> phy_device_remove ->
   device_del, and the phy stays alive on the netdev's reference), and
   then the PSE controller unbinds. Is that the path you have in mind,
   or is there an easier one I am missing?

2. On keeping pse_control_put() in phy_device_remove(): wouldn't that
   bring back the reason it was moved out? phy_device_remove() and the
   walk's phy_pse_detach_one() would both touch phydev->psec, and
   serializing them means taking rtnl in phy_device_remove() - which
   the sfp caller already holds, so it would deadlock. Did you mean a
   plain put there, or something narrower?

3. Simon raised the same psec-vs-pcdev lifetime on the net regulator
   patch [1] and suggested either draining the references on unregister
   or having pse_control hold a refcount on pcdev. This series does the
   drain, which (as you show) misses off-klist phys. Would having
   pse_control pin pcdev; so pcdev->pi and pcdev->owner cannot be
   freed while any pse_control is still out... be the direction you
   prefer? That makes the deferred put safe no matter what the klist
   walk sees.

I'll send a v5 once the direction is clear.

Thanks,
Carlo

[1] https://lore.kernel.org/netdev/20260624151251.1137250-1-horms@kernel.org/