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

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 before taking anything.
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. Without 4, an unbind that loses the race to an
attach still leaves that consumer with a PHY whose driver is gone.

On the lock inversion (Paolo): patch 3 says why the PHY's device lock
cannot be used here. Removing the inversion would at least mean moving
the SFP registration out of probe and remove. The SFP upstream ops write
netdev state that rtnl protects. Attach does not always run under rtnl
(DSA connects its ports before taking it), so the registration cannot
follow the attach either. Nothing under the new mutex takes rtnl. On the
generic-driver path, device_bind_driver() can take a supplier's device
lock for sync_state, as it already does today.

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". What I looked at:
- A, phy_remove() waits for the detach (patch 4, kept): the unbind
  blocks, uninterruptibly, until the consumer detaches. For DSA that is
  switch teardown, even with the port down.
- B, stop at patch 3: the oops moves one frame up. Patch 4 has the
  phylink trace.
- C, let the caller hold the lock across attach and bringup: widens the
  series to phylink and still leaves every use after bringup.
- A with a killable wait: ->remove cannot fail, so after a kill it could
  only go on removing the driver, back into the oops.
- suppress_bind_attrs on PHY drivers: removes the sysfs unbind your use
  case relies on.
- A managed device link MAC -> PHY: the unbind would take the whole MAC
  or switch down with it.

A is a compromise: an unbind of a PHY in use hangs instead of failing.
It has other costs too. The hung-task detector, when enabled, reports
the blocked unbind after its timeout. System suspend and reboot wait on
that PHY's device lock meanwhile. device_shutdown() takes it and does
not detach PHYs, so a reboot behind a blocked unbind hangs. sysfs shows
the driver link gone while ->remove is still waiting, because the driver
core removes it first.

This series does not close one more window. phy_attach_direct() still
binds the generic driver without the device lock the driver core
expects. A real driver binding through the driver core at the same
moment can still collide with it.

On v4 (OpenWrt 6.18 backport, PROVE_LOCKING), a test module attached the
free EN8811H of a KN-1012 twice without a netdev. The second attach got
-EBUSY, the air_en8811h refcount went 0, 1, 1 and back to 0 after one
detach, and lockdep stayed quiet. Without the series the second attach
succeeded with a duplicate phy_standalone sysfs warning, and one
reference was left after the detach.

On the KN-1012, lan4 is a DSA port on an EN8811H. Without the series,
unbinding air_en8811h returns at once, and the switch teardown faults
later. With the series (an earlier revision with the same attach and
wait code, before the device-deletion change), the same unbind blocks
with lan4 up or down, and returns when the switch is unbound.

Patches 2 to 4 changed in v4 only by the rebase and the fixes listed
below, so their v3 results on the KN-1012 (OpenWrt 6.18 backport,
PROVE_LOCKING) still apply. They are in the v3 cover letter:
https://lore.kernel.org/netdev/20260924215951.2127682-1-f@lex.la/ (local)

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

Changes in v4 (since v3):
https://lore.kernel.org/netdev/20260924215951.2127682-1-f@lex.la/ (local)
- Retargeted to net-next, as asked. There the detach code lives in
  phy_detach_internal(), and bind_lock also covers the new
  notify_phy_attach() bus hook.
- Patch 1: the test also catches a PHY attached without a netdev, as DSA
  does for its CPU and link ports.
- Patch 3: a Return: section for __phy_probe(). The forward declaration
  stays: dropping it means moving about 200 or over 500 lines, details
  in 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/ (local)
- Retargeted to net, and the race closed with locking rather than a NULL
  test at attach entry, as asked in review.
- The module reference fix is its own patch.
- New patch 1: a second attach no longer detaches the first consumer.
- An unbind of an attached PHY now waits for the detach.

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 | 98 ++++++++++++++++++++++++++++++------
 include/linux/phy.h          | 12 +++++
 2 files changed, 95 insertions(+), 15 deletions(-)

-- 
2.53.0