Messages in this thread Patch in this message | | | From | Ding Hui <> | Subject | [PATCH net-next] net: ethtool: Fix out-of-bounds copy to user | Date | Thu, 1 Jun 2023 19:28:39 +0800 |
| |
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@sangfor.com.cn> --- 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
| |