Re: [PATCH net] net: phy: mscc: fix deadlock in phy_ethtool_{get,set}_wol()

[Simon Horman <simon.horman@corigine.com>]

On Tue, Mar 14, 2023 at 05:30:25PM +0200, Vladimir Oltean wrote:
> 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: 2f987d486610 ("net: phy: Add locks to ethtool functions")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>