netdev - Re: [PATCH] net: smsc911x: If PHY doesn't have an interrupt then POLL

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: <57606574.5080700@arm.com>
Date:	Tue, 14 Jun 2016 15:13:40 -0500
From:	Jeremy Linton <jeremy.linton@....com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	netdev@...r.kernel.org
Cc:	steve.glendinning@...well.net
Subject: Re: [PATCH] net: smsc911x: If PHY doesn't have an interrupt then POLL

On 06/14/2016 02:49 PM, Sergei Shtylyov wrote:
> On 06/14/2016 10:27 PM, Sergei Shtylyov wrote:
>
>>>> If the interrupt configuration isn't set and we are using the
>>>> internal phy, then we need to poll the phy to reliably detect
>>>> phy state changes.
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@....com>
>>>> ---
>>>>  drivers/net/ethernet/smsc/smsc911x.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/smsc/smsc911x.c
>>>> b/drivers/net/ethernet/smsc/smsc911x.c
>>>> index 8af2556..369dc7d 100644
>>>> --- a/drivers/net/ethernet/smsc/smsc911x.c
>>>> +++ b/drivers/net/ethernet/smsc/smsc911x.c
>>>> @@ -1020,6 +1020,9 @@ static int smsc911x_mii_probe(struct
>>>> net_device *dev)
>>>>          return -ENODEV;
>>>>      }
>>>>
>>>> +    if ((!phydev->irq) && (!pdata->using_extphy))
>>>
>>>    Inner parens aren't needed at all.
>>
>>    Hm, 'phydev->irq' shouldn't be 0 in the first place. It seems to me we
>> should correctly initialize 'pdata->phy_irq[]' in smsc911x_mii_init()...
>
>     And looking at that array, I doubt it's really useful for
> anything... And the memcpy() there seems buggy as well -- it copies just
> 4 bytes of this array to 'pdata->mii_bus->irq'. I do care about this
> driver, so might be a good idea to clean it up a bit...

	The use of phy_connect_direct() in the driver probe is incorrect, and 
keeps the driver from being unloaded.

	Also, some portion of smsc's can deliver mii state change interrupts 
via the smsc interrupt, but that code is no longer in this driver.

	I suspect a portion of the problem, besides all the strange hardware 
configurations this driver supports are the emulated hardware like QEMU 
that also uses it.