[PATCH 5.4] lan743x: fix deadlock in lan743x_phy_link_status_change()

[PATCH 5.4] lan743x: fix deadlock in lan743x_phy_link_status_change()

[dann frazier]

From: Heiner Kallweit <hkallweit1@gmail.com>

[ Upstream commit ddb826c2c92d461f290a7bab89e7c28696191875 ]

Usage of phy_ethtool_get_link_ksettings() in the link status change
handler isn't needed, and in combination with the referenced change
it results in a deadlock. Simply remove the call and replace it with
direct access to phydev->speed. The duplex argument of
lan743x_phy_update_flowcontrol() isn't used and can be removed.

Fixes: c10a485c3de5 ("phy: phy_ethtool_ksettings_get: Lock the phy for consistency")
Reported-by: Alessandro B Maurici <abmaurici@gmail.com>
Tested-by: Alessandro B Maurici <abmaurici@gmail.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://lore.kernel.org/r/40e27f76-0ba3-dcef-ee32-a78b9df38b0f@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[dannf: adjust context]
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---

The patch this Fixes: was applied back through 5.4.y. But this fix for it
was only applied back through 5.10.y. It did require some minor context
adjustment for 5.4.y, perhaps that is why? At any rate, this looks to
be a fix for a problem one of our users reported on our 5.4-based kernel.

Compile-tested only.

 drivers/net/ethernet/microchip/lan743x_main.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 22beeb5be9c4..c69ffcfe6168 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -916,8 +916,7 @@ static int lan743x_phy_reset(struct lan743x_adapter *adapter)
 }
 
 static void lan743x_phy_update_flowcontrol(struct lan743x_adapter *adapter,
-					   u8 duplex, u16 local_adv,
-					   u16 remote_adv)
+					   u16 local_adv, u16 remote_adv)
 {
 	struct lan743x_phy *phy = &adapter->phy;
 	u8 cap;
@@ -944,22 +943,17 @@ static void lan743x_phy_link_status_change(struct net_device *netdev)
 
 	phy_print_status(phydev);
 	if (phydev->state == PHY_RUNNING) {
-		struct ethtool_link_ksettings ksettings;
 		int remote_advertisement = 0;
 		int local_advertisement = 0;
 
-		memset(&ksettings, 0, sizeof(ksettings));
-		phy_ethtool_get_link_ksettings(netdev, &ksettings);
 		local_advertisement =
 			linkmode_adv_to_mii_adv_t(phydev->advertising);
 		remote_advertisement =
 			linkmode_adv_to_mii_adv_t(phydev->lp_advertising);
 
-		lan743x_phy_update_flowcontrol(adapter,
-					       ksettings.base.duplex,
-					       local_advertisement,
+		lan743x_phy_update_flowcontrol(adapter, local_advertisement,
 					       remote_advertisement);
-		lan743x_ptp_update_latency(adapter, ksettings.base.speed);
+		lan743x_ptp_update_latency(adapter, phydev->speed);
 	}
 }
 
-- 
2.35.1

Re: [PATCH 5.4] lan743x: fix deadlock in lan743x_phy_link_status_change()

[Greg KH]

On Wed, Feb 23, 2022 at 04:14:32PM -0700, dann frazier wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
> 
> [ Upstream commit ddb826c2c92d461f290a7bab89e7c28696191875 ]
> 
> Usage of phy_ethtool_get_link_ksettings() in the link status change
> handler isn't needed, and in combination with the referenced change
> it results in a deadlock. Simply remove the call and replace it with
> direct access to phydev->speed. The duplex argument of
> lan743x_phy_update_flowcontrol() isn't used and can be removed.
> 
> Fixes: c10a485c3de5 ("phy: phy_ethtool_ksettings_get: Lock the phy for consistency")
> Reported-by: Alessandro B Maurici <abmaurici@gmail.com>
> Tested-by: Alessandro B Maurici <abmaurici@gmail.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Link: https://lore.kernel.org/r/40e27f76-0ba3-dcef-ee32-a78b9df38b0f@gmail.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> [dannf: adjust context]
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> ---
> 
> The patch this Fixes: was applied back through 5.4.y. But this fix for it
> was only applied back through 5.10.y. It did require some minor context
> adjustment for 5.4.y, perhaps that is why? At any rate, this looks to
> be a fix for a problem one of our users reported on our 5.4-based kernel.

Now queued up, thanks.

greg k-h