netdev - Re: [PATCH V3 net] net: stmmac: fix MAC WoL unwork if PHY doesn't support WoL

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: <ab8152e4-c462-def4-c2e8-0ec2bec5d638@gmail.com>
Date:   Wed, 28 Apr 2021 09:22:00 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Joakim Zhang <qiangqing.zhang@....com>
Cc:     peppe.cavallaro@...com, alexandre.torgue@...com,
        joabreu@...opsys.com, davem@...emloft.net, kuba@...nel.org,
        Jisheng.Zhang@...aptics.com, netdev@...r.kernel.org,
        linux-imx@....com
Subject: Re: [PATCH V3 net] net: stmmac: fix MAC WoL unwork if PHY doesn't
 support WoL



On 4/28/2021 5:26 AM, Andrew Lunn wrote:
>>  static int stmmac_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
>>  {
>>  	struct stmmac_priv *priv = netdev_priv(dev);
>> -	u32 support = WAKE_MAGIC | WAKE_UCAST;
>> +	struct ethtool_wolinfo wol_phy = { .cmd = ETHTOOL_GWOL };
>> +	u32 support = WAKE_MAGIC | WAKE_UCAST | WAKE_MAGICSECURE | WAKE_BCAST;
> 
> Reverse christmass tree please.
> 
>>  
>> -	if (!device_can_wakeup(priv->device))
>> -		return -EOPNOTSUPP;
>> +	if (wol->wolopts & ~support)
>> +		return -EINVAL;
> 
> Maybe -EOPNOTSUPP would be better.
> 
>>  
>> -	if (!priv->plat->pmt) {
>> +	/* First check if can WoL from PHY */
>> +	phylink_ethtool_get_wol(priv->phylink, &wol_phy);
> 
> This could return an error. In which case, you probably should not
> trust wol_phy.
> 
>> +	if (wol->wolopts & wol_phy.supported) {
> 
> This returns true if the PHY supports one or more of the requested WoL
> sources.
> 
>>  		int ret = phylink_ethtool_set_wol(priv->phylink, wol);
> 
> and here you request the PHY to enable all the requested WoL
> sources. If it only supports a subset, it is likely to return
> -EOPNOTSUPP, or -EINVAL, and do nothing. So here you only want to
> enable those sources the PHY actually supports. And let the MAC
> implement the rest.

And when your resubmit, I do not believe that unwork is a word, you
could provide the following subject:

net: stmmac: Fix MAC WoL not working if PHY does not support WoL

or something like that.

Thanks!
-- 
Florian