linux-stable.git/drivers/net/phy/phy.c, branch v6.7

net: phy: fix regression with AX88772A PHY driver

2023-09-19T15:03:44+00:00

Marek reports that a deadlock occurs with the AX88772A PHY used on the
ASIX USB network driver:

asix 1-1.4:1.0 (unnamed net_device) (uninitialized): PHY [usb-001:003:10] driver [Asix Electronics AX88772A] (irq=POLL)
Asix Electronics AX88772A usb-001:003:10: attached PHY driver(mii_bus:phy_addr=usb-001:003:10, irq=POLL)
asix 1-1.4:1.0 eth0: register 'asix' at usb-12110000.usb-1.4, ASIX AX88772 USB 2.0 Ethernet, a2:99:b6:cd:11:eb
asix 1-1.4:1.0 eth0: configuring for phy/internal link mode

============================================
WARNING: possible recursive locking detected
6.6.0-rc1-00239-g8da77df649c4-dirty #13949 Not tainted
--------------------------------------------
kworker/3:3/71 is trying to acquire lock:
c6c704cc (&dev->lock){+.+.}-{3:3}, at: phy_start_aneg+0x1c/0x38

but task is already holding lock:
c6c704cc (&dev->lock){+.+.}-{3:3}, at: phy_state_machine+0x100/0x2b8

This is because we now consistently call phy_process_state_change()
while holding phydev->lock, but the AX88772A PHY driver then goes on
to call phy_start_aneg() which tries to grab the same lock - causing
deadlock.

Fix this by exporting the unlocked version, and use this in the PHY
driver instead.

Reported-by: Marek Szyprowski 
Tested-by: Marek Szyprowski 
Fixes: ef113a60d0a9 ("net: phy: call phy_error_precise() while holding the lock")
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Andrew Lunn 
Link: https://lore.kernel.org/r/E1qiEFs-007g7b-Lq@rmk-PC.armlinux.org.uk
Signed-off-by: Paolo Abeni

net: phy: convert phy_stop() to use split state machine

2023-09-17T13:31:23+00:00

Convert phy_stop() to use the new locked-section and unlocked-section
parts of the PHY state machine.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: split locked and unlocked section of phy_state_machine()

2023-09-17T13:31:23+00:00

Split out the locked and unlocked sections of phy_state_machine() into
two separate functions which can be called inside the phydev lock and
outside the phydev lock as appropriate, thus allowing us to combine
the locked regions in the caller of phy_state_machine() with the
locked region inside phy_state_machine().

This avoids unnecessarily dropping the phydev lock which may allow
races to occur.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: move phy_state_machine()

2023-09-17T13:31:23+00:00

Move phy_state_machine() before phy_stop() to avoid subsequent patches
introducing forward references.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: move phy_suspend() to end of phy_state_machine()

2023-09-17T13:31:23+00:00

Move the call to phy_suspend() to the end of phy_state_machine() after
we release the lock so that we can combine the locked areas.
phy_suspend() can not be called while holding phydev->lock as it has
caused deadlocks in the past.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: move call to start aneg

2023-09-17T13:31:23+00:00

Move the call to start auto-negotiation inside the lock in the PHYLIB
state machine, calling the locked variant _phy_start_aneg(). This
avoids unnecessarily releasing and re-acquiring the lock.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: call phy_error_precise() while holding the lock

2023-09-17T13:31:23+00:00

Move the locking out of phy_error_precise() and to its only call site,
merging with the locked region that has already been taken.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

net: phy: always call phy_process_state_change() under lock

2023-09-17T13:31:23+00:00

phy_stop() calls phy_process_state_change() while holding the phydev
lock, so also arrange for phy_state_machine() to do the same, so that
this function is called with consistent locking.

Tested-by: Jijie Shao 
Signed-off-by: Russell King (Oracle) 
Reviewed-by: Florian Fainelli 
Signed-off-by: David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net

2023-08-24T17:51:39+00:00

Cross-merge networking fixes after downstream PR.

Conflicts:

include/net/inet_sock.h
  f866fbc842de ("ipv4: fix data-races around inet->inet_id")
  c274af224269 ("inet: introduce inet->inet_flags")
https://lore.kernel.org/all/679ddff6-db6e-4ff6-b177-574e90d0103d@tessares.net/

Adjacent changes:

drivers/net/bonding/bond_alb.c
  e74216b8def3 ("bonding: fix macvlan over alb bond support")
  f11e5bd159b0 ("bonding: support balance-alb with openvswitch")

drivers/net/ethernet/broadcom/bgmac.c
  d6499f0b7c7c ("net: bgmac: Return PTR_ERR() for fixed_phy_register()")
  23a14488ea58 ("net: bgmac: Fix return value check for fixed_phy_register()")

drivers/net/ethernet/broadcom/genet/bcmmii.c
  32bbe64a1386 ("net: bcmgenet: Fix return value check for fixed_phy_register()")
  acf50d1adbf4 ("net: bcmgenet: Return PTR_ERR() for fixed_phy_register()")

net/sctp/socket.c
  f866fbc842de ("ipv4: fix data-races around inet->inet_id")
  b09bde5c3554 ("inet: move inet->mc_loop to inet->inet_frags")

Signed-off-by: Jakub Kicinski

net: phy: Fix deadlocking in phy_error() invocation

2023-08-19T18:26:50+00:00

Since commit 91a7cda1f4b8 ("net: phy: Fix race condition on link status
change") all the phy_error() method invocations have been causing the
nested-mutex-lock deadlock because it's normally done in the PHY-driver
threaded IRQ handlers which since that change have been called with the
phydev->lock mutex held. Here is the calls thread:

IRQ: phy_interrupt()
     +-> mutex_lock(&phydev->lock); <--------------------+
         drv->handle_interrupt()                         | Deadlock due
         +-> ERROR: phy_error()                          + to the nested
                    +-> phy_process_error()              | mutex lock
                        +-> mutex_lock(&phydev->lock); <-+
                            phydev->state = PHY_ERROR;
                            mutex_unlock(&phydev->lock);
         mutex_unlock(&phydev->lock);

The problem can be easily reproduced just by calling phy_error() from any
PHY-device threaded interrupt handler. Fix it by dropping the phydev->lock
mutex lock from the phy_process_error() method and printing a nasty error
message to the system log if the mutex isn't held in the caller execution
context.

Note for the fix to work correctly in the PHY-subsystem itself the
phydev->lock mutex locking must be added to the phy_error_precise()
function.

Link: https://lore.kernel.org/netdev/20230816180944.19262-1-fancer.lancer@gmail.com
Fixes: 91a7cda1f4b8 ("net: phy: Fix race condition on link status change")
Suggested-by: Andrew Lunn 
Signed-off-by: Serge Semin 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller