netdev - Re: [PATCH net-next] net: phy: fix regression with AX88772A PHY driver

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]

Date: Mon, 18 Sep 2023 14:57:45 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: phy: fix regression with AX88772A PHY
 driver

On Mon, Sep 18, 2023 at 03:49:32PM +0200, Andrew Lunn wrote:
> On Mon, Sep 18, 2023 at 02:25:36PM +0100, Russell King (Oracle) wrote:
> > Marek reports that a deadlock occurs with the AX88772A PHY used on the
> > ASIX USB network driver:
> > 
> > asix 1-1.4:1.0 (unnamed net_device) (uninitialized): PHY [usb-001:003:10] driver [Asix Electronics AX88772A] (irq=POLL)
> > Asix Electronics AX88772A usb-001:003:10: attached PHY driver(mii_bus:phy_addr=usb-001:003:10, irq=POLL)
> > asix 1-1.4:1.0 eth0: register 'asix' at usb-12110000.usb-1.4, ASIX AX88772 USB 2.0 Ethernet, a2:99:b6:cd:11:eb
> > asix 1-1.4:1.0 eth0: configuring for phy/internal link mode
> > 
> > ============================================
> > WARNING: possible recursive locking detected
> > 6.6.0-rc1-00239-g8da77df649c4-dirty #13949 Not tainted
> > --------------------------------------------
> > kworker/3:3/71 is trying to acquire lock:
> > c6c704cc (&dev->lock){+.+.}-{3:3}, at: phy_start_aneg+0x1c/0x38
> > 
> > but task is already holding lock:
> > c6c704cc (&dev->lock){+.+.}-{3:3}, at: phy_state_machine+0x100/0x2b8
> > 
> > This is because we now consistently call phy_process_state_change()
> > while holding phydev->lock, but the AX88772A PHY driver then goes on
> > to call phy_start_aneg() which tries to grab the same lock - causing
> > deadlock.
> > 
> > Fix this by exporting the unlocked version, and use this in the PHY
> > driver instead.
> > 
> > Reported-by: Marek Szyprowski <m.szyprowski@...sung.com>
> > Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
> > Fixes: ef113a60d0a9 ("net: phy: call phy_error_precise() while holding the lock")
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> 
> Hi Russell
> 
> Yes, this fixes the problem for stable.
> 
> Reviewed-by: Andrew Lunn <andrew@...n.ch>
> 
> But maybe it would be better to move the hardware workaround into the
> PHY driver? Its the PHY which is broken, so why is the MAC working
> around it?

Err? Sorry, but your comment makes little sense given that my patch
only touches the PHY core (to export _phy_start_aneg()) and the PHY
driver (ax88796b.c) which is where the work-around is already located.

I'm not having to touch the MAC driver at all to fix this, because
afaics the MAC driver isn't involved in _this_ particular workaround.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!