netdev - Re: [RESEND] net: phy: fix NULL pointer dereference in phy_polling

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: <aJSx9xTrFfFm0dcx@pengutronix.de>
Date: Thu, 7 Aug 2025 16:02:31 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Andrew Lunn <andrew@...n.ch>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
	Xu Yang <xu.yang_2@....com>, hkallweit1@...il.com,
	pabeni@...hat.com, netdev@...r.kernel.org, imx@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [RESEND] net: phy: fix NULL pointer dereference in
 phy_polling_mode()

On Thu, Aug 07, 2025 at 02:58:05PM +0200, Andrew Lunn wrote:
> > Hm, I guess, with this change there will be a subtile regression.
> > In case of an external PHYs the ax88772_init_phy() is using PHYlib to
> > suspend the internal PHY.
> > 
> > May be:
> >   priv->mdio->phy_mask = ~(BIT(priv->phy_addr) | BIT(AX_EMBD_PHY_ADDR));
> 
> I looked at that:

Here we read the primary PHY address from the EEPROM. This offset may
contain either the internal or external PHY address. See commit
d0ffff8fddd5 ("USB: asix: Detect internal PHY and enable/use
accordingly")

I need to admit, asix_read_phy_addr(..., bool internal) was originally
designed to distinguish between internal and external PHYs by setting
internal = false.  But in practice, most vendors seem to follow the
existing driver behavior as reference, and only modify the primary PHY
address in the EEPROM.

> 	ret = asix_read_phy_addr(dev, true);
> 	if (ret < 0)
> 		return ret;
> 

At this point, we store the address of the internal or external PHY:

> 	priv->phy_addr = ret;

If the PHY address matches the address of the internal PHY, then
embd_phy is set to true:

> 	priv->embd_phy = ((priv->phy_addr & 0x1f) == AX_EMBD_PHY_ADDR);
> 
> So priv->phy_addr has to be the address of the internal PHY, so this
> should just work without anything special for the embedded PHY.

For most AX88772-based devices, priv->phy_addr is indeed the internal
PHY. However, on devices with an external PHY - like the "Linux
Automation GmbH USB 10Base-T1L" - both internal and external PHYs are
accessible over the MDIO bus.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |