All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2] hns: Fix string set validation in ethtool string operations
@ 2018-03-13 23:39 Ben Hutchings
  2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2018-03-13 23:39 UTC (permalink / raw)
  To: Yisen Zhuang, Salil Mehta; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 3201 bytes --]

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@decadent.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;
 	}
 }
 


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-15 10:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-13 23:39 [PATCH net 1/2] hns: Fix string set validation in ethtool string operations Ben Hutchings
2018-03-13 23:41 ` [PATCH 2/2] hns: Clean up " Ben Hutchings
2018-03-15 10:10   ` kbuild test robot
2018-03-15 10:10   ` [RFC PATCH] hns: hns_ae_get_stats_strings() can be static kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.