netdev - Re: [PATCH net-next] net: phy: fix check in get_phy_c45

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]

Message-Id: <20200722.171416.680374889497844164.davem@davemloft.net>
Date:   Wed, 22 Jul 2020 17:14:16 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     olteanv@...il.com
Cc:     netdev@...r.kernel.org, linux@...linux.org.uk,
        f.fainelli@...il.com, andrew@...n.ch, hkallweit1@...il.com,
        claudiu.manoil@....com, alexandru.marginean@....com,
        ioana.ciornei@....com, michael@...le.cc, colin.king@...onical.com
Subject: Re: [PATCH net-next] net: phy: fix check in get_phy_c45_ids

From: Vladimir Oltean <olteanv@...il.com>
Date: Mon, 20 Jul 2020 20:26:54 +0300

> From: Vladimir Oltean <vladimir.oltean@....com>
> 
> After the patch below, the iteration through the available MMDs is
> completely short-circuited, and devs_in_pkg remains set to the initial
> value of zero.
> 
> Due to devs_in_pkg being zero, the rest of get_phy_c45_ids() is
> short-circuited too: the following loop never reaches below this point
> either (it executes "continue" for every device in package, failing to
> retrieve PHY ID for any of them):
> 
> 	/* Now probe Device Identifiers for each device present. */
> 	for (i = 1; i < num_ids; i++) {
> 		if (!(devs_in_pkg & (1 << i)))
> 			continue;
> 
> So c45_ids->device_ids remains populated with zeroes. This causes an
> Aquantia AQR412 PHY (same as any C45 PHY would, in fact) to be probed by
> the Generic PHY driver.
> 
> The issue seems to be a case of submitting partially committed work (and
> therefore testing something other than was submitted).
> 
> The intention of the patch was to delay exiting the loop until one more
> condition is reached (the devs_in_pkg read from hardware is either 0, OR
> mostly f's). So fix the patch to reflect that.
> 
> Tested with traffic on a LS1028A-QDS, the PHY is now probed correctly
> using the Aquantia driver. The devs_in_pkg bit field is set to
> 0xe000009a, and the MMDs that are present have the following IDs:
 ...
> Fixes: bba238ed037c ("net: phy: continue searching for C45 MMDs even if first returned ffff:ffff")
> Reported-by: Colin King <colin.king@...onical.com>
> Reported-by: Ioana Ciornei <ioana.ciornei@....com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Applied, thanks.

I waited on this because I wanted to get a review from someone, and I
try to always give Andrew/Florian/Heiner/etc. a day or two on core PHY
stuff so that they can have a chance to do a review.