[lvc-project] [PATCH v1] net/ethtool/ioctl: ensure that we have phy ops before using them

[Daniil Tatianin]

On 11/15/22 8:07 AM, Jakub Kicinski wrote:
> On Mon, 14 Nov 2022 11:15:32 +0300 Daniil Tatianin wrote:
>> +	if (!(phydev && phy_ops && phy_ops->get_stats) &&
>> +	    !ops->get_ethtool_phy_stats)
> 
> This condition is still complicated.
> 
>> +		return -EOPNOTSUPP;
> 
> The only way this crash can happen is if driver incorrectly returns
> non-zero stats count but doesn't have a callback to read the stats.
> So WARN_ON() would be in order here.
> 
>>   	if (!phydev && (!ops->get_ethtool_phy_stats || !ops->get_sset_count))
>>   		return -EOPNOTSUPP;
>>   
>> @@ -2063,13 +2066,12 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
>>   		if (!data)
>>   			return -ENOMEM;
>>   
>> -		if (dev->phydev && !ops->get_ethtool_phy_stats &&
>> -		    phy_ops && phy_ops->get_stats) {
>> -			ret = phy_ops->get_stats(dev->phydev, &stats, data);
>> +		if (ops->get_ethtool_phy_stats) {
>> +			ops->get_ethtool_phy_stats(dev, &stats, data);
>> +		} else {
>> +			ret = phy_ops->get_stats(phydev, &stats, data);
>>   			if (ret < 0)
>>   				goto out;
>> -		} else {
>> -			ops->get_ethtool_phy_stats(dev, &stats, data);
>>   		}
> 
> We can also clean up the pointless indentation of this code while at it.
> 
> How about something along these lines (completely untested, please
> review, test and make your own):


Thanks, I'll do something like this and resend.