[PATCH net-next] net: ethtool: Fix out-of-bounds copy to user

From: Ding Hui
Date: Thu Jun 01 2023 - 07:29:03 EST


When we get statistics by ethtool during changing the number of NIC
channels greater, the utility may crash due to memory corruption.

The NIC drivers callback get_sset_count() could return a calculated
length depends on current number of channels (e.g. i40e, igb).

The ethtool allocates a user buffer with the first ioctl returned
length and invokes the second ioctl to get data. The kernel copies
data to the user buffer but without checking its length. If the length
returned by the second get_sset_count() is greater than the length
allocated by the user, it will lead to an out-of-bounds copy.

Fix it by restricting the copy length not exceed the buffer length
specified by userspace.

Signed-off-by: Ding Hui <dinghui@xxxxxxxxxxxxxx>
---
net/ethtool/ioctl.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 6bb778e10461..82a975a9c895 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1902,7 +1902,7 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
if (copy_from_user(&test, useraddr, sizeof(test)))
return -EFAULT;

- test.len = test_len;
+ test.len = min_t(u32, test.len, test_len);
data = kcalloc(test_len, sizeof(u64), GFP_USER);
if (!data)
return -ENOMEM;
@@ -1915,7 +1915,8 @@ static int ethtool_self_test(struct net_device *dev, char __user *useraddr)
if (copy_to_user(useraddr, &test, sizeof(test)))
goto out;
useraddr += sizeof(test);
- if (copy_to_user(useraddr, data, array_size(test.len, sizeof(u64))))
+ if (test.len &&
+ copy_to_user(useraddr, data, array_size(test.len, sizeof(u64))))
goto out;
ret = 0;

@@ -1940,10 +1941,10 @@ static int ethtool_get_strings(struct net_device *dev, void __user *useraddr)
return -ENOMEM;
WARN_ON_ONCE(!ret);

- gstrings.len = ret;
+ gstrings.len = min_t(u32, gstrings.len, ret);

if (gstrings.len) {
- data = vzalloc(array_size(gstrings.len, ETH_GSTRING_LEN));
+ data = vzalloc(array_size(ret, ETH_GSTRING_LEN));
if (!data)
return -ENOMEM;

@@ -2055,9 +2056,9 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
if (copy_from_user(&stats, useraddr, sizeof(stats)))
return -EFAULT;

- stats.n_stats = n_stats;
+ stats.n_stats = min_t(u32, stats.n_stats, n_stats);

- if (n_stats) {
+ if (stats.n_stats) {
data = vzalloc(array_size(n_stats, sizeof(u64)));
if (!data)
return -ENOMEM;
@@ -2070,7 +2071,8 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
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))))
+ if (stats.n_stats &&
+ copy_to_user(useraddr, data, array_size(stats.n_stats, sizeof(u64))))
goto out;
ret = 0;

--
2.17.1