netdev - Re: [PATCH v2] phy: unmask link partner capabilities

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] [day] [month] [year] [list]

Message-ID: <1393284993.6823.107.camel@deadeye.wl.decadent.org.uk>
Date:	Mon, 24 Feb 2014 23:36:33 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	cristian.bercaru@...escale.com
Cc:	netdev@...r.kernel.org, madalin.bucur@...escale.com,
	shaohui.xie@...escale.com, shruti@...escale.com
Subject: Re: [PATCH v2] phy: unmask link partner capabilities

On Mon, 2014-02-24 at 17:52 +0200, cristian.bercaru@...escale.com wrote:
> From: Cristian Bercaru <cristian.bercaru@...escale.com>
> 
> Masking the link partner's capabilities with local capabilities can be
> misleading in autonegotiation scenarios such as PAUSE frame
> autonegotiation.
> This patch calculates the join between the local capabilities and the
> link parner capabilities, when it calculates the speed and duplex
> settings, but does not mask any of the link partner capabilities
> when it calculates PAUSE frame settings.
> 
> Signed-off-by: Cristian Bercaru <cristian.bercaru@...escale.com>
> ---
> v2 caches (lpa & adv) and (lpa & adv << 2) in intermediate variables.
> 
>  drivers/net/phy/phy_device.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index a70b604..4b021a3 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -933,6 +933,8 @@ int genphy_read_status(struct phy_device *phydev)
>  	int err;
>  	int lpa;
>  	int lpagb = 0;
> +	int common_adv;
> +	int common_adv_gb;

I think common_adv_gb needs to be initialised to 0...

>  
>  	/* Update the link, but return if there was an error */
>  	err = genphy_update_link(phydev);
> @@ -954,7 +956,7 @@ int genphy_read_status(struct phy_device *phydev)
>  
>  			phydev->lp_advertising =
>  				mii_stat1000_to_ethtool_lpa_t(lpagb);
> -			lpagb &= adv << 2;
> +			common_adv_gb = lpagb & adv << 2;
>  		}
[...]

...as this assignment is conditional.

Ben.

-- 
Ben Hutchings
Beware of bugs in the above code;
I have only proved it correct, not tried it. - Donald Knuth

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)