Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ding Hui <dinghui@sangfor.com.cn>
To:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        pengdonglin@sangfor.com.cn, huangcun@sangfor.com.cn,
        Ding Hui <dinghui@sangfor.com.cn>
Subject: [PATCH net-next] net: ethtool: Fix out-of-bounds copy to user
Date:   Thu,  1 Jun 2023 19:28:39 +0800
Message-Id: <20230601112839.13799-1-dinghui@sangfor.com.cn>
Precedence: bulk

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