Commit e71460d4 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'ethtool_gert_phy_stats-fixes' 



Daniil Tatianin says:

====================
net/ethtool/ioctl: split ethtool_get_phy_stats into multiple helpers

This series fixes a potential NULL dereference in ethtool_get_phy_stats
while also attempting to refactor/split said function into multiple
helpers so that it's easier to reason about what's going on.

I've taken Andrew Lunn's suggestions on the previous version of this
patch and added a bit of my own.

Changes since v1:
- Remove an extra newline in the first patch
- Move WARN_ON_ONCE into the if check as it already returns the
  result of the comparison
- Actually split ethtool_get_phy_stats instead of attempting to
  refactor it
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 8ac718cc 201ed315

net/ethtool/ioctl.c

+70 −37
Original line number Diff line number Diff line
@@ -2078,58 +2078,91 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr) 
	return ret;

}



static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)

static int ethtool_vzalloc_stats_array(int n_stats, u64 **data)

{

	if (n_stats < 0)

		return n_stats;

	if (n_stats > S32_MAX / sizeof(u64))

		return -ENOMEM;

	if (WARN_ON_ONCE(!n_stats))

		return -EOPNOTSUPP;



	*data = vzalloc(array_size(n_stats, sizeof(u64)));

	if (!*data)

		return -ENOMEM;



	return 0;

}



static int ethtool_get_phy_stats_phydev(struct phy_device *phydev,

					 struct ethtool_stats *stats,

					 u64 **data)

 {

	const struct ethtool_phy_ops *phy_ops = ethtool_phy_ops;

	const struct ethtool_ops *ops = dev->ethtool_ops;

	struct phy_device *phydev = dev->phydev;

	struct ethtool_stats stats;

	u64 *data;

	int ret, n_stats;

	int n_stats, ret;



	if (!phydev && (!ops->get_ethtool_phy_stats || !ops->get_sset_count))

	if (!phy_ops || !phy_ops->get_sset_count || !phy_ops->get_stats)

		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



	ret = ethtool_vzalloc_stats_array(n_stats, data);

	if (ret)

		return ret;



	stats->n_stats = n_stats;

	return phy_ops->get_stats(phydev, stats, *data);

}



static int ethtool_get_phy_stats_ethtool(struct net_device *dev,

					  struct ethtool_stats *stats,

					  u64 **data)

{

	const struct ethtool_ops *ops = dev->ethtool_ops;

	int n_stats, ret;



	if (!ops || !ops->get_sset_count || ops->get_ethtool_phy_stats)

		return -EOPNOTSUPP;



	n_stats = ops->get_sset_count(dev, ETH_SS_PHY_STATS);

	if (n_stats < 0)

		return n_stats;

	if (n_stats > S32_MAX / sizeof(u64))

		return -ENOMEM;

	WARN_ON_ONCE(!n_stats);



	ret = ethtool_vzalloc_stats_array(n_stats, data);

	if (ret)

		return ret;



	stats->n_stats = n_stats;

	ops->get_ethtool_phy_stats(dev, stats, *data);



	return 0;

}



static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)

{

	struct phy_device *phydev = dev->phydev;

	struct ethtool_stats stats;

	u64 *data = NULL;

	int ret = -EOPNOTSUPP;



	if (copy_from_user(&stats, useraddr, sizeof(stats)))

		return -EFAULT;



	stats.n_stats = n_stats;

	if (phydev)

		ret = ethtool_get_phy_stats_phydev(phydev, &stats, &data);



	if (n_stats) {

		data = vzalloc(array_size(n_stats, sizeof(u64)));

		if (!data)

			return -ENOMEM;

	if (ret == -EOPNOTSUPP)

		ret = ethtool_get_phy_stats_ethtool(dev, &stats, &data);



		if (phydev && !ops->get_ethtool_phy_stats &&

		    phy_ops && phy_ops->get_stats) {

			ret = phy_ops->get_stats(phydev, &stats, data);

			if (ret < 0)

	if (ret)

		goto out;

		} else {

			ops->get_ethtool_phy_stats(dev, &stats, data);

		}

	} else {

		data = NULL;

	}



	if (copy_to_user(useraddr, &stats, sizeof(stats))) {

		ret = -EFAULT;

	if (copy_to_user(useraddr, &stats, sizeof(stats)))

		goto out;

	}



	useraddr += sizeof(stats);

	if (n_stats && copy_to_user(useraddr, data, array_size(n_stats, sizeof(u64))))

		goto out;

	ret = 0;

	if (copy_to_user(useraddr, data, array_size(stats.n_stats, sizeof(u64))))

		ret = -EFAULT;



 out:

	vfree(data);