Debug lock detection revealed a possible circular locking dependency between phylink_major_config() and phylink_bringup_phy().

This was introduced by the addition of phy_config_inband(), which acquires the phydev lock. This made the locking order in phylink_bringup_phy() inconsistent, as it acquires the phydev lock before phylink's state_mutex.

A deadlock can occur when phylink_major_config() is called from phylink_resolve(), where the state_mutex is taken first and then the phydev lock. This is the reverse of the order used in phylink_bringup_phy().

To avoid this circular dependency, change the locking order in phylink_bringup_phy() to match the pattern used in phylink_resolve(): take state_mutex first, then the phydev lock.

A sample lockdep warning is included below for reference:

[ 147.749178] [ 147.750682] ====================================================== [ 147.756850] WARNING: possible circular locking dependency detected [ 147.763019] 6.14.0-next-20250404+ #0 Tainted: G O [ 147.769189] ------------------------------------------------------ [ 147.775356] kworker/u16:0/12 is trying to acquire lock: [ 147.780571] ffffff80ce9bcf08 (&dev->lock#2){+.+.}-{4:4}, at: phy_config_inband+0x44/0x90 [ 147.788672] [ 147.788672] but task is already holding lock: [ 147.794492] ffffff80c0dfbda0 (&pl->state_mutex){+.+.}-{4:4}, at: phylink_resolve+0x2c/0x6a8 [ 147.802840] [ 147.802840] which lock already depends on the new lock. [ 147.802840] [ 147.811002] [ 147.811002] the existing dependency chain (in reverse order) is: [ 147.818472] [ 147.818472] -> #1 (&pl->state_mutex){+.+.}-{4:4}: [ 147.824647] __mutex_lock+0x90/0x924 [ 147.828738] mutex_lock_nested+0x20/0x28 [ 147.833173] phylink_bringup_phy+0x210/0x700 [ 147.837954] phylink_fwnode_phy_connect+0xe0/0x124 [ 147.843256] phylink_of_phy_connect+0x18/0x20 [ 147.848124] dsa_user_create+0x210/0x414 [ 147.852561] dsa_port_setup+0xd4/0x14c [ 147.856823] dsa_register_switch+0xbb0/0xe40 [ 147.861605] mt7988_probe+0xf8/0x140 [ 147.865694] platform_probe+0x64/0xbc [ 147.869869] really_probe+0xbc/0x388 [ 147.873955] __driver_probe_device+0x78/0x154 [ 147.878823] driver_probe_device+0x3c/0xd4 [ 147.883430] __device_attach_driver+0xb0/0x144 [ 147.888383] bus_for_each_drv+0x6c/0xb0 [ 147.892732] __device_attach+0x9c/0x19c [ 147.897078] device_initial_probe+0x10/0x18 [ 147.901771] bus_probe_device+0xa8/0xac [ 147.906118] deferred_probe_work_func+0xb8/0x118 [ 147.911245] process_one_work+0x224/0x610 [ 147.915769] worker_thread+0x1b8/0x35c [ 147.920029] kthread+0x11c/0x1e8 [ 147.923769] ret_from_fork+0x10/0x20 [ 147.927857] [ 147.927857] -> #0 (&dev->lock#2){+.+.}-{4:4}: [ 147.933686] __lock_acquire+0x12b8/0x1ff0 [ 147.938209] lock_acquire+0xf4/0x2d8 [ 147.942295] __mutex_lock+0x90/0x924 [ 147.946383] mutex_lock_nested+0x20/0x28 [ 147.950817] phy_config_inband+0x44/0x90 [ 147.955252] phylink_major_config+0x684/0xa64 [ 147.960120] phylink_resolve+0x24c/0x6a8 [ 147.964554] process_one_work+0x224/0x610 [ 147.969075] worker_thread+0x1b8/0x35c [ 147.973335] kthread+0x11c/0x1e8 [ 147.977075] ret_from_fork+0x10/0x20 [ 147.981162] [ 147.981162] other info that might help us debug this: [ 147.981162] [ 147.989150] Possible unsafe locking scenario: [ 147.989150] [ 147.995056] CPU0 CPU1 [ 147.999575] ---- ---- [ 148.004094] lock(&pl->state_mutex); [ 148.007748] lock(&dev->lock#2); [ 148.013572] lock(&pl->state_mutex); [ 148.019742] lock(&dev->lock#2); [ 148.023051] [ 148.023051] *** DEADLOCK *** [ 148.023051] [ 148.028958] 3 locks held by kworker/u16:0/12: [ 148.033304] #0: ffffff80c0011d48 ((wq_completion)events_power_efficient){+.+.}-{0:0}, at: process_one_work+0x1a8/0x610 [ 148.044082] #1: ffffffc081ca3dd8 ((work_completion)(&pl->resolve)){+.+.}-{0:0}, at: process_one_work+0x1d0/0x610 [ 148.054338] #2: ffffff80c0dfbda0 (&pl->state_mutex){+.+.}-{4:4}, at: phylink_resolve+0x2c/0x6a8 [ 148.063119] [ 148.063119] stack backtrace: [ 148.067465] CPU: 3 UID: 0 PID: 12 Comm: kworker/u16:0 Tainted: G O 6.14.0-next-20250404+ #0 NONE [ 148.067472] Tainted: [O]=OOT_MODULE [ 148.067474] Hardware name: Bananapi BPI-R4 2.5GE PoE (DT) [ 148.067476] Workqueue: events_power_efficient phylink_resolve [ 148.067482] Call trace: [ 148.067484] show_stack+0x14/0x1c (C) [ 148.067492] dump_stack_lvl+0x84/0xc0 [ 148.067497] dump_stack+0x14/0x1c [ 148.067500] print_circular_bug+0x330/0x43c [ 148.067505] check_noncircular+0x124/0x134 [ 148.067508] __lock_acquire+0x12b8/0x1ff0 [ 148.067512] lock_acquire+0xf4/0x2d8 [ 148.067516] __mutex_lock+0x90/0x924 [ 148.067521] mutex_lock_nested+0x20/0x28 [ 148.067527] phy_config_inband+0x44/0x90 [ 148.067531] phylink_major_config+0x684/0xa64 [ 148.067535] phylink_resolve+0x24c/0x6a8 [ 148.067539] process_one_work+0x224/0x610 [ 148.067544] worker_thread+0x1b8/0x35c [ 148.067548] kthread+0x11c/0x1e8 [ 148.067552] ret_from_fork+0x10/0x20

Cc: stable@vger.kernel.org Fixes: 5fd0f1a02e75 ("net: phylink: add negotiation of in-band capabilities") Signed-off-by: Christian Marangi ansuelsmth@gmail.com --- drivers/net/phy/phylink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 69ca765485db..4a1edf19dfad 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -2072,8 +2072,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy, dev_name(&phy->mdio.dev), phy->drv->name, irq_str); kfree(irq_str);

- mutex_lock(&phy->lock); mutex_lock(&pl->state_mutex); + mutex_lock(&phy->lock); pl->phydev = phy; pl->phy_state.interface = interface; pl->phy_state.pause = MLO_PAUSE_NONE; @@ -2115,8 +2115,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy, phy_disable_eee(phy); }

- mutex_unlock(&pl->state_mutex); mutex_unlock(&phy->lock); + mutex_unlock(&pl->state_mutex);

phylink_dbg(pl, "phy: %s setting supported %*pb advertising %*pb\n",