Minki/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2024-11-25 21:51:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

net/ethtool/ioctl: split ethtool_get_phy_stats into multiple helpers

Browse Source 
So that it's easier to follow and make sense of the branching and
various conditions.

Stats retrieval has been split into two separate functions
ethtool_get_phy_stats_phydev & ethtool_get_phy_stats_ethtool.
The former attempts to retrieve the stats using phydev & phy_ops, while
the latter uses ethtool_ops.

Actual n_stats validation & array allocation has been moved into a new
ethtool_vzalloc_stats_array helper.

This also fixes a potential NULL dereference of
ops->get_ethtool_phy_stats where it was getting called in an else branch
unconditionally without making sure it was actually present.

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

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Daniil Tatianin 2022-12-26 14:48:25 +03:00 committed by David S. Miller
parent fd4778581d
commit 201ed315f9
1 changed files with 69 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
net/ethtool/ioctl.c
View File

@ -2078,23 +2078,8 @@ 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)
{
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;
if (!phydev && (!ops->get_ethtool_phy_stats || !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);
if (n_stats < 0)
return n_stats;
if (n_stats > S32_MAX / sizeof(u64))
@ -2102,31 +2087,82 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
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;
int n_stats, ret;
if (!phy_ops || !phy_ops->get_sset_count || !phy_ops->get_stats)
return -EOPNOTSUPP;
n_stats = phy_ops->get_sset_count(phydev);
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);
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);
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)
goto out;
} else {
ops->get_ethtool_phy_stats(dev, &stats, data);
if (ret)
goto out;
if (copy_to_user(useraddr, &stats, sizeof(stats))) {
ret = -EFAULT;
goto out;
}
ret = -EFAULT;
if (copy_to_user(useraddr, &stats, sizeof(stats)))
goto out;
useraddr += sizeof(stats);
if (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);