netdev - Re: [PATCH v1 2/2] net: phy: marvell-88q2xxx: add driver for the Marvell 88Q2110 PHY

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 13 Jul 2023 07:31:01 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Stefan Eichenberger <eichest@...il.com>
Cc: netdev@...r.kernel.org, andrew@...n.ch, hkallweit1@...il.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, francesco.dolcini@...adex.com
Subject: Re: [PATCH v1 2/2] net: phy: marvell-88q2xxx: add driver for the
 Marvell 88Q2110 PHY

On Mon, Jul 03, 2023 at 02:44:40PM +0200, Stefan Eichenberger wrote:
> +static int check_link(struct phy_device *phydev)
> +{
> +	u16 ret1, ret2;
> +
> +	if (phydev->speed == SPEED_1000) {
> +		ret1 = phy_read_mmd(phydev, 3, 0x0901);
> +		ret1 = phy_read_mmd(phydev, 3, 0x0901);

This looks like some kind of BMSR register, where the link status
latches on failure until the following read. By reading the register
twice, you are discarding the information that the link _had_ failed
since we last read it. You don't want to do that - it could come up with
different parameters and because the link to phylib appears to remain
up, it won't respond to the new link parameters.

Always report the latched link status, not the current status.

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