netdev - Re: mii_bus->read return checking in 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1FBC63C1-A8CE-4EFA-8864-62E12C0CFCB3@freescale.com>
Date:	Fri, 4 Mar 2011 18:06:20 +0000
From:	Fleming Andy-AFLEMING <afleming@...escale.com>
To:	Florian Fainelli <florian@...nwrt.org>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>
Subject: Re: mii_bus->read return checking in phy_device.c



On Mar 4, 2011, at 11:24, "Florian Fainelli" <florian@...nwrt.org> wrote:

> Hello Andy,
> 
> While debugging a PHY probing issue with the au1000_eth, I stumbled upon this 
> in drivers/net/phy/phy_device.c:
> 
>        phy_reg = bus->read(bus, addr, MII_PHYSID1);                                                  
> 
>        if (phy_reg < 0)                        
>                return -EIO;
> 
> most drivers implement phylib's mdio_read callback by simply returning the 
> contents of their MDIO register after a readl, ioread ... which is unsigned. 
> Would not it rather make sense to check for phy_reg <= 0 instead?


That isn't a check for a non-existent PHY.  PHY registers are unsigned 16-bit quantities.  The negative 32-bit return value would be the result of something going wrong in the bus transaction.

Notice that later the code actually checks to see if the read value was mostly 1s...


> 
> This can lead for instance to believing that a PHY is present at a wrong 
> address because the MDIO read function returns 0 for that particular register, 
> which is logical because no PHY is present at that address.
> 
> I am asking in case I just miss something.
> 
> Thank you.
> --
> Florian
> 

--
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