netdev - [PATCH net 1/2] hns: Fix string set validation in ethtool string operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 13 Mar 2018 23:39:48 +0000
From:   Ben Hutchings <ben@...adent.org.uk>
To:     Yisen Zhuang <yisen.zhuang@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>
Cc:     netdev@...r.kernel.org
Subject: [PATCH net 1/2] hns: Fix string set validation in ethtool string
 operations

The hns driver used to assume that it would only ever be asked
about ETH_SS_TEST or ETH_SS_STATS, and for any other stringset
it would claim a count of 0 but if asked for names would copy
over all the statistic names.  This resulted in a potential
heap buffer overflow.

This was "fixed" some time ago by making it report the number of
statistics when asked about the ETH_SS_PRIV_FLAGS stringset.  This
doesn't make any sense, and it left the bug still exploitable with
other stringset numbers.

As a minimal but complete fix, stop calling the driver-internal
string operations for any stringset other than ETH_SS_STATS.

Fixes: 511e6bc071db ("net: add Hisilicon Network Subsystem DSAF support")
Fixes: 412b65d15a7f ("net: hns: fix ethtool_get_strings overflow ...")
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 7ea7f8a4aa2a..b836742f7ab6 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -889,11 +889,6 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	struct hnae_handle *h = priv->ae_handle;
 	char *buff = (char *)data;
 
-	if (!h->dev->ops->get_strings) {
-		netdev_err(netdev, "h->dev->ops->get_strings is null!\n");
-		return;
-	}
-
 	if (stringset == ETH_SS_TEST) {
 		if (priv->ae_handle->phy_if != PHY_INTERFACE_MODE_XGMII) {
 			memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_MAC],
@@ -907,7 +902,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 			memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_PHY],
 			       ETH_GSTRING_LEN);
 
-	} else {
+	} else if (stringset == ETH_SS_STATS && h->dev->ops->get_strings) {
 		snprintf(buff, ETH_GSTRING_LEN, "rx_packets");
 		buff = buff + ETH_GSTRING_LEN;
 		snprintf(buff, ETH_GSTRING_LEN, "tx_packets");
@@ -969,7 +964,7 @@ void hns_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 /**
  * nic_get_sset_count - get string set count witch returned by nic_get_strings.
  * @dev: net device
- * @stringset: string set index, 0: self test string; 1: statistics string.
+ * @stringset: string set index
  *
  * Return string set count.
  */
@@ -979,10 +974,6 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
 	struct hnae_handle *h = priv->ae_handle;
 	struct hnae_ae_ops *ops = h->dev->ops;
 
-	if (!ops->get_sset_count) {
-		netdev_err(netdev, "get_sset_count is null!\n");
-		return -EOPNOTSUPP;
-	}
 	if (stringset == ETH_SS_TEST) {
 		u32 cnt = (sizeof(hns_nic_test_strs) / ETH_GSTRING_LEN);
 
@@ -993,8 +984,10 @@ int hns_get_sset_count(struct net_device *netdev, int stringset)
 			cnt--;
 
 		return cnt;
+	} else if (stringset == ETH_SS_STATS && ops->get_sset_count) {
+		return HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset);
 	} else {
-		return (HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset));
+		return -EOPNOTSUPP;
 	}
 }
 


Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)