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

[Ben Hutchings]

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

signature.asc
Description: Digital signature