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  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:   Sat, 23 May 2020 19:37:31 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Jeremy Linton <jeremy.linton@....com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, andrew@...n.ch,
        f.fainelli@...il.com, hkallweit1@...il.com,
        madalin.bucur@....nxp.com, calvin.johnson@....nxp.com,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC 04/11] net: phy: Handle c22 regs presence better

On Fri, May 22, 2020 at 04:30:52PM -0500, Jeremy Linton wrote:
> Until this point, we have been sanitizing the c22
> regs presence bit out of all the MMD device lists.
> This is incorrect as it causes the 0xFFFFFFFF checks
> to incorrectly fail. Further, it turns out that we
> want to utilize this flag to make a determination that
> there is actually a phy at this location and we should
> be accessing it using c22.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@....com>
> ---
>  drivers/net/phy/phy_device.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index f0761fa5e40b..2d677490ecab 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -689,9 +689,6 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
>  		return -EIO;
>  	*devices_in_package |= phy_reg;
>  
> -	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
> -	*devices_in_package &= ~BIT(0);
> -
>  	return 0;
>  }
>  
> @@ -742,6 +739,8 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
>  	int i;
>  	const int num_ids = ARRAY_SIZE(c45_ids->device_ids);
>  	u32 *devs = &c45_ids->devices_in_package;
> +	bool c22_present = false;
> +	bool valid_id = false;
>  
>  	/* Find first non-zero Devices In package. Device zero is reserved
>  	 * for 802.3 c45 complied PHYs, so don't probe it at first.
> @@ -770,6 +769,10 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
>  		return 0;
>  	}
>  
> +	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
> +	c22_present = *devs & BIT(0);
> +	*devs &= ~BIT(0);
> +
>  	/* Now probe Device Identifiers for each device present. */
>  	for (i = 1; i < num_ids; i++) {
>  		if (!(c45_ids->devices_in_package & (1 << i)))
> @@ -778,6 +781,13 @@ static int get_phy_c45_ids(struct mii_bus *bus, int addr, u32 *phy_id,
>  		ret = _get_phy_id(bus, addr, i, &c45_ids->device_ids[i], true);
>  		if (ret < 0)
>  			return ret;
> +		if (valid_phy_id(c45_ids->device_ids[i]))
> +			valid_id = true;

Here you are using your "devices in package" validator to validate the
PHY ID value.  One of the things it does is mask this value with
0x1fffffff.  That means you lose some of the vendor OUI.  To me, this
looks completely wrong.

> +	}
> +
> +	if (!valid_id && c22_present) {
> +		*phy_id = 0xffffffff;
> +	        return 0;
>  	}
>  	*phy_id = 0;
>  	return 0;
> -- 
> 2.26.2
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up

Powered by blists - more mailing lists