Skip to content

Commit cd35601

Browse files
vladimirolteankuba-moo
authored andcommitted
net: phy: mscc: fix deadlock in phy_ethtool_{get,set}_wol()
Since the blamed commit, phy_ethtool_get_wol() and phy_ethtool_set_wol() acquire phydev->lock, but the mscc phy driver implementations, vsc85xx_wol_get() and vsc85xx_wol_set(), acquire the same lock as well, resulting in a deadlock. $ ip link set swp3 down ============================================ WARNING: possible recursive locking detected mscc_felix 0000:00:00.5 swp3: Link is Down -------------------------------------------- ip/375 is trying to acquire lock: ffff3d7e82e987a8 (&dev->lock){+.+.}-{4:4}, at: vsc85xx_wol_get+0x2c/0xf4 but task is already holding lock: ffff3d7e82e987a8 (&dev->lock){+.+.}-{4:4}, at: phy_ethtool_get_wol+0x3c/0x6c other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&dev->lock); lock(&dev->lock); *** DEADLOCK *** May be due to missing lock nesting notation 2 locks held by ip/375: #0: ffffd43b2a955788 (rtnl_mutex){+.+.}-{4:4}, at: rtnetlink_rcv_msg+0x144/0x58c #1: ffff3d7e82e987a8 (&dev->lock){+.+.}-{4:4}, at: phy_ethtool_get_wol+0x3c/0x6c Call trace: __mutex_lock+0x98/0x454 mutex_lock_nested+0x2c/0x38 vsc85xx_wol_get+0x2c/0xf4 phy_ethtool_get_wol+0x50/0x6c phy_suspend+0x84/0xcc phy_state_machine+0x1b8/0x27c phy_stop+0x70/0x154 phylink_stop+0x34/0xc0 dsa_port_disable_rt+0x2c/0xa4 dsa_slave_close+0x38/0xec __dev_close_many+0xc8/0x16c __dev_change_flags+0xdc/0x218 dev_change_flags+0x24/0x6c do_setlink+0x234/0xea4 __rtnl_newlink+0x46c/0x878 rtnl_newlink+0x50/0x7c rtnetlink_rcv_msg+0x16c/0x58c Removing the mutex_lock(&phydev->lock) calls from the driver restores the functionality. Fixes: 2f987d4 ("net: phy: Add locks to ethtool functions") Signed-off-by: Vladimir Oltean <[email protected]> Reviewed-by: Simon Horman <[email protected]> Reviewed-by: Andrew Lunn <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 7c10131 commit cd35601

File tree

1 file changed

+8
-16
lines changed

1 file changed

+8
-16
lines changed

drivers/net/phy/mscc/mscc_main.c

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -280,12 +280,9 @@ static int vsc85xx_wol_set(struct phy_device *phydev,
280280
u16 pwd[3] = {0, 0, 0};
281281
struct ethtool_wolinfo *wol_conf = wol;
282282

283-
mutex_lock(&phydev->lock);
284283
rc = phy_select_page(phydev, MSCC_PHY_PAGE_EXTENDED_2);
285-
if (rc < 0) {
286-
rc = phy_restore_page(phydev, rc, rc);
287-
goto out_unlock;
288-
}
284+
if (rc < 0)
285+
return phy_restore_page(phydev, rc, rc);
289286

290287
if (wol->wolopts & WAKE_MAGIC) {
291288
/* Store the device address for the magic packet */
@@ -323,30 +320,27 @@ static int vsc85xx_wol_set(struct phy_device *phydev,
323320

324321
rc = phy_restore_page(phydev, rc, rc > 0 ? 0 : rc);
325322
if (rc < 0)
326-
goto out_unlock;
323+
return rc;
327324

328325
if (wol->wolopts & WAKE_MAGIC) {
329326
/* Enable the WOL interrupt */
330327
reg_val = phy_read(phydev, MII_VSC85XX_INT_MASK);
331328
reg_val |= MII_VSC85XX_INT_MASK_WOL;
332329
rc = phy_write(phydev, MII_VSC85XX_INT_MASK, reg_val);
333330
if (rc)
334-
goto out_unlock;
331+
return rc;
335332
} else {
336333
/* Disable the WOL interrupt */
337334
reg_val = phy_read(phydev, MII_VSC85XX_INT_MASK);
338335
reg_val &= (~MII_VSC85XX_INT_MASK_WOL);
339336
rc = phy_write(phydev, MII_VSC85XX_INT_MASK, reg_val);
340337
if (rc)
341-
goto out_unlock;
338+
return rc;
342339
}
343340
/* Clear WOL iterrupt status */
344341
reg_val = phy_read(phydev, MII_VSC85XX_INT_STATUS);
345342

346-
out_unlock:
347-
mutex_unlock(&phydev->lock);
348-
349-
return rc;
343+
return 0;
350344
}
351345

352346
static void vsc85xx_wol_get(struct phy_device *phydev,
@@ -358,10 +352,9 @@ static void vsc85xx_wol_get(struct phy_device *phydev,
358352
u16 pwd[3] = {0, 0, 0};
359353
struct ethtool_wolinfo *wol_conf = wol;
360354

361-
mutex_lock(&phydev->lock);
362355
rc = phy_select_page(phydev, MSCC_PHY_PAGE_EXTENDED_2);
363356
if (rc < 0)
364-
goto out_unlock;
357+
goto out_restore_page;
365358

366359
reg_val = __phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
367360
if (reg_val & SECURE_ON_ENABLE)
@@ -377,9 +370,8 @@ static void vsc85xx_wol_get(struct phy_device *phydev,
377370
}
378371
}
379372

380-
out_unlock:
373+
out_restore_page:
381374
phy_restore_page(phydev, rc, rc > 0 ? 0 : rc);
382-
mutex_unlock(&phydev->lock);
383375
}
384376

385377
#if IS_ENABLED(CONFIG_OF_MDIO)

0 commit comments

Comments
 (0)