netdev - Re: [PATCH net] net: phy: mscc: fix deadlock in phy_ethtool_{get,set}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZBCgBoLk1G+NZqz/@corigine.com>
Date:   Tue, 14 Mar 2023 17:25:42 +0100
From:   Simon Horman <simon.horman@...igine.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: phy: mscc: fix deadlock in
 phy_ethtool_{get,set}_wol()

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@....com>

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