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

Thu Nov 17 10:04:42 MSK 2022

On 11/15/22 8:07 AM, Jakub Kicinski wrote:
> On 2022年11月14日 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.