[PATCH net] net: phy: don't require RTNL to detach a PHY from an unregistered netdev

[PATCH net] net: phy: don't require RTNL to detach a PHY from an unregistered netdev

[Nguyen Ngoc Thang]

phy_detach_internal() reads dev->hwprov with rtnl_dereference(), but
MAC drivers commonly attach and detach PHYs while their net_device is
not registered, without holding RTNL. Before register_netdev(), a failed
phylink_connect_phy() or phy_attach_direct() unwinds through
phy_detach_internal(). After unregister_netdev(), a driver's remove path
may call phy_disconnect(). Both trip lockdep:

  WARNING: suspicious RCU usage
  drivers/net/phy/phy_device.c:1831 suspicious rcu_dereference_protected() usage!
   phy_detach_internal+0x6a5/0x830 drivers/net/phy/phy_device.c:1831
   phylink_connect_phy+0x22e/0x2b0 drivers/net/phy/phylink.c:2452
   ax88772_bind+0xbb1/0x1520 drivers/net/usb/asix_devices.c:925
   usbnet_probe+0xbb3/0x27e0 drivers/net/usb/usbnet.c:1812

syzbot hits this with an emulated asix device. phy_eee_rx_clock_stop()
fails on a USB MDIO error, so phylink_bringup_phy() fails and the PHY is
detached from inside usbnet_probe(), before the netdev is registered.

hwprov is only set through ethtool, under RTNL, on a registered device.
reg_state also changes only under RTNL. So if the device is not
registered, no one else can reach hwprov, and RTNL is not needed. Tell
lockdep that. A detach from a registered device without RTNL still
warns.

Fixes: 35f7cad1743e ("net: Add the possibility to support a selected hwtstamp in netdevice")
Reported-by: syzbot+583f42a41879bb60de32@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=583f42a41879bb60de32
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Tested in QEMU (x86_64, syzbot config, KASAN + PROVE_RCU) with the syzbot
C reproducer on 6b4b543fa187:
 - before: "WARNING: suspicious RCU usage" at phy_device.c:1831 via
   phylink_connect_phy() -> phy_detach_internal() from ax88772_bind()
 - after: 11/11 runs hit the same "Could not connect PHY" failure path
   with no lockdep/KASAN report

 drivers/net/phy/phy_device.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5d4508bcfbe9..06c5a8c8cc2a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1826,9 +1826,13 @@ static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
 		struct hwtstamp_provider *hwprov;
 
 		/* hwprov may technically be protected by ops lock but
-		 * not for devices with a phydev, see phy_link_topo_add_phy()
+		 * not for devices with a phydev, see phy_link_topo_add_phy().
+		 * Drivers may detach without RTNL while dev is unregistered,
+		 * but then nothing else can reach hwprov.
 		 */
-		hwprov = rtnl_dereference(dev->hwprov);
+		hwprov = rcu_dereference_protected(dev->hwprov,
+						   lockdep_rtnl_is_held() ||
+						   dev->reg_state != NETREG_REGISTERED);
 		/* Disable timestamp if it is the one selected */
 		if (hwprov && hwprov->phydev == phydev) {
 			rcu_assign_pointer(dev->hwprov, NULL);
-- 
2.43.0

Re: [PATCH net] net: phy: don't require RTNL to detach a PHY from an unregistered netdev

[Kory Maincent]

On 10/29/26 15:24, Nguyen Ngoc Thang wrote:
> phy_detach_internal() reads dev->hwprov with rtnl_dereference(), but
> MAC drivers commonly attach and detach PHYs while their net_device is
> not registered, without holding RTNL. Before register_netdev(), a failed
> phylink_connect_phy() or phy_attach_direct() unwinds through
> phy_detach_internal(). After unregister_netdev(), a driver's remove path
> may call phy_disconnect(). Both trip lockdep:
> 
>    WARNING: suspicious RCU usage
>    drivers/net/phy/phy_device.c:1831 suspicious rcu_dereference_protected() usage!
>     phy_detach_internal+0x6a5/0x830 drivers/net/phy/phy_device.c:1831
>     phylink_connect_phy+0x22e/0x2b0 drivers/net/phy/phylink.c:2452
>     ax88772_bind+0xbb1/0x1520 drivers/net/usb/asix_devices.c:925
>     usbnet_probe+0xbb3/0x27e0 drivers/net/usb/usbnet.c:1812
> 
> syzbot hits this with an emulated asix device. phy_eee_rx_clock_stop()
> fails on a USB MDIO error, so phylink_bringup_phy() fails and the PHY is
> detached from inside usbnet_probe(), before the netdev is registered.
> 
> hwprov is only set through ethtool, under RTNL, on a registered device.
> reg_state also changes only under RTNL. So if the device is not
> registered, no one else can reach hwprov, and RTNL is not needed. Tell
> lockdep that. A detach from a registered device without RTNL still
> warns.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

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