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]
Date:	Tue, 3 Mar 2015 14:42:10 +0000
From:	"claudiu.manoil@...escale.com" <claudiu.manoil@...escale.com>
To:	Guenter Roeck <linux@...ck-us.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	"David S. Miller" <davem@...emloft.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] gianfar: Reduce logging noise seen due to phy polling if
 link is down

> -----Original Message-----
> From: Guenter Roeck [mailto:linux@...ck-us.net]
> Sent: Monday, March 02, 2015 10:03 PM
> 
[...]
> 
> Cc: Claudiu Manoil <claudiu.manoil@...escale.com>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
[...]
>  	if (unlikely(phydev->link != priv->oldlink ||
> -		     phydev->duplex != priv->oldduplex ||
> -		     phydev->speed != priv->oldspeed))
> +		     (phydev->link && (phydev->duplex != priv->oldduplex ||
> +				       phydev->speed != priv->oldspeed))))
>  		gfar_update_link_state(priv);
>  }

I did a quick check and, indeed, phydev->duplex or phydev->speed
may change even if phydev->link is 0.  I could reproduce the issue
with the following test for an eth with a polling mode phy:
1) initially link is up - autoneg on, speed 1000;
2) taking the link down - link down message printed only once;
3) turning autoneg to off for the same eth  - link down message
is printed continuously;

So, given this, I agree that the driver needs this protection before
accessing the phydev->duplex/speed fields, namely to check first whether
phydev->link is 1.  If link is 0, phydev->speed/duplex may be bogus.

Thanks for spotting this.

Reviewed-by: Claudiu Manoil <claudiu.manoil@...escale.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists