[lvc-project] [PATCH v1 3/3] net/ethtool/ioctl: correct & simplify ethtool_get_phy_stats if checks

[Daniil Tatianin]

ops->get_ethtool_phy_stats was getting called in an else branch
of ethtool_get_phy_stats() unconditionally without making sure
it was actually present.

Refactor the if checks so that it's more obvious what's going on and
avoid accidental NULL derefs.

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.

Signed-off-by: Daniil Tatianin <d-tatianin at yandex-team.ru>
---
 net/ethtool/ioctl.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index f83118c68e20..2b01e0042e6e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -2076,25 +2076,27 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
 {
 	const struct ethtool_phy_ops *phy_ops = ethtool_phy_ops;
 	const struct ethtool_ops *ops = dev->ethtool_ops;
+	bool has_phy_stats_ops = ops->get_ethtool_phy_stats != NULL;
 	struct phy_device *phydev = dev->phydev;
 	struct ethtool_stats stats;
 	u64 *data;
 	int ret, n_stats;
 
-	if (!phydev && (!ops->get_ethtool_phy_stats || !ops->get_sset_count))
-		return -EOPNOTSUPP;
+	if (!phydev || !phy_ops) {
+		if (!ops->get_sset_count)
+			return -EOPNOTSUPP;
 
-	if (phydev && !ops->get_ethtool_phy_stats &&
-	    phy_ops && phy_ops->get_sset_count)
-		n_stats = phy_ops->get_sset_count(phydev);
-	else
 		n_stats = ops->get_sset_count(dev, ETH_SS_PHY_STATS);
+	} else {
+		n_stats = phy_ops->get_sset_count(phydev);
+		has_phy_stats_ops |= phy_ops->get_stats != NULL;
+	}
 
 	if (n_stats < 0)
 		return n_stats;
 	if (n_stats > S32_MAX / sizeof(u64))
 		return -ENOMEM;
-	if (!n_stats) {
+	if (!n_stats || !has_phy_stats_ops) {
 		WARN_ON_ONCE(1);
 		return -EOPNOTSUPP;
 	}
@@ -2108,13 +2110,12 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
 	if (!data)
 		return -ENOMEM;
 
-	if (phydev && !ops->get_ethtool_phy_stats &&
-		phy_ops && phy_ops->get_stats) {
+	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);
 	}
 
 	ret = -EFAULT;
-- 
2.25.1