[PATCH net-next v5 0/4] net: phy: make PHY driver unbind safe against attach and use

[Aleksei Sviridkin <f@lex.la>]

Unbinding a PHY driver through sysfs while its MAC brings the port up
can oops. On a Keenetic KN-1012 (MT7981, mtk_eth_soc), an unbind of the
wan PHY driver racing "ip link set wan up" faulted on the first attempt,
with no delay added anywhere. The PHY driver's config_init read
phydev->drv after phy_remove() had cleared it. Patch 3 has the trace.

Serialising attach against unbind is not enough on its own. The unbind
that loses the race then removes the driver from a PHY that is now
attached, and the consumer faults one step later. A DSA port gets there
without any race, because DSA keeps its PHYs attached from switch setup
to teardown. Patch 4 has both cases.

1: refuse a second attach of an attached PHY up front.
2: put the module reference phy_attach_direct() took, not whatever
   driver is bound at detach time.
3: a per-PHY mutex and a "bound" flag. An attach either is done with the
   driver before phy_remove() starts tearing it down, or is refused with
   -EAGAIN.
4: phy_remove() waits for phy_detach() when the PHY is attached, except
   when the PHY device itself is being deleted.

1 to 3 do not need 4, but without it the winner of the race still
ends up with a PHY whose driver is gone.

On the lock inversion (Paolo): phy_probe() and phy_remove() take rtnl
under the device lock (SFP bus, netdev LED trigger), and attach may run
under rtnl, so attach cannot take the device lock. Nothing under the
new mutex takes rtnl.

Vladimir, patch 4 falls short of what you asked for in [1]: that
unbinding a PHY driver should not "explode ... even in uncontrolled
situations where the netdev isn't carefully disconnected from the PHY
first". It makes phy_remove() wait for the detach, so the unbind blocks
until the consumer lets go. For DSA that is switch teardown. Other
options, and why not:
- stop at patch 3: the oops moves one frame up;
- the caller holds the lock across bringup: widens into phylink,
  leaves every later use;
- a killable wait: ->remove cannot fail, so a kill ends in the oops;
- suppress_bind_attrs: removes the unbind your use case relies on;
- a managed device link MAC -> PHY: takes the MAC or switch down too.

The wait has costs. The hung-task detector reports the blocked unbind.
Suspend waits behind it and reboot hangs, because both take the PHY's
device lock. Patch 4 names a device-link case where the wait never ends.

Attach still binds the generic driver without the device lock, so a
real driver binding at the same moment can collide with it. Two
consumers attaching and detaching one PHY at once are unsupported: the
end of a detach can still reset the other's PHY, and with patch 4 its
generic driver release waits for it (a deadlock if both hold rtnl).

Tested on a KN-1012 (OpenWrt 6.18 backport, PROVE_LOCKING, hung-task
panic) with lan4 holding its EN8811H. The image carried 03101a02b2fa,
which differs from this posting only in the patch 4 message, one
comment and the WRITE_ONCE() on removing.
- second attach of lan4's PHY, with or without a netdev: -EBUSY
- air_en8811h unbind under lan4: waits, returns on switch unbind
- free PHY: attach 0, again -EBUSY, refcount 0/1/1/0; on genphy too
- irq back to the bus number after each generic-driver detach
- two attaches racing without rtnl, 200 rounds: one winner each time
- unbind of an attached PHY waits for detach, with both drivers
- phy_device_remove() releases a waiting unbind
Lockdep stayed quiet. The one splat was a known kernfs warning from the
DSA teardown order.

[1] https://lore.kernel.org/netdev/20260311203410.rio7m6nuf72hs5p6@skbuf/

Changes in v5 (since v4):
https://lore.kernel.org/netdev/20261001130120.104628-1-f@lex.la/
- Rebased on the applied phylib interrupt series. Patch 4 moves its
  detach-side restore ahead of the point where a new attach can start.
- Patches 1 and 3: the "already attached" test runs under bind_lock.
- Patches 3 and 4: detach clears phy_link_change, the recorded module
  and the attached flag in one bind_lock section.
- Patch 2: the driver module is read once.
- Patch 4: READ_ONCE()/WRITE_ONCE() on attached, WRITE_ONCE() on
  removing.
- Messages: patch 1 on the generic driver, patch 3 on its exception
  and -EAGAIN at probe time, patch 4 on the reboot hang and the
  device-link case.
- The bind_lock kernel-doc names phy_probe() and phy_remove().

Changes in v4 (since v3):
https://lore.kernel.org/netdev/20260924215951.2127682-1-f@lex.la/
- Retargeted to net-next, as asked.
- Patch 1: the test also catches a PHY attached without a netdev.
- Patch 3: a Return: section for __phy_probe(). The forward declaration
  stays, see the v4 notes of patch 3.
- Patch 4: a line over 80 columns wrapped.

Changes in v3 (since v2):
https://lore.kernel.org/netdev/20260919015340.499675-1-f@lex.la/
- Retargeted to net. The race is closed with locking instead of a NULL
  test, as asked in review.
- Patch 2 split out. New patch 1 (no detach of the first consumer on a
  second attach) and patch 4 (the unbind waits).

Aleksei Sviridkin (4):
  net: phy: refuse a second attach before touching the PHY
  net: phy: put the driver module the attach took
  net: phy: serialise attach and detach with PHY driver bind and unbind
  net: phy: make an unbind wait for the attached consumer to detach

 drivers/net/phy/phy_device.c | 123 +++++++++++++++++++++++++++++------
 include/linux/phy.h          |  12 ++++
 2 files changed, 116 insertions(+), 19 deletions(-)

-- 
2.53.0