net/hns3: fix insecure way to query MAC statistics
authorHuisong Li <lihuisong@huawei.com>
Fri, 28 Jan 2022 02:07:05 +0000 (10:07 +0800)
committerFerruh Yigit <ferruh.yigit@intel.com>
Mon, 31 Jan 2022 13:22:21 +0000 (14:22 +0100)
The query way of MAC statistics in HNS3 PF driver is as following:
1) get MAC statistics register number and calculate descriptor number.
2) use above descriptor number to send command to firmware to query all
   MAC statistics and copy to hns3_mac_stats struct in driver.

The preceding way does not verify the validity of the number of obtained
register, which may cause memory out-of-bounds.

Fixes: 8839c5e202f3 ("net/hns3: support device stats")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
drivers/net/hns3/hns3_ethdev.c
drivers/net/hns3/hns3_ethdev.h
drivers/net/hns3/hns3_stats.c
drivers/net/hns3/hns3_stats.h

index 73bf209..57f1572 100644 (file)
@@ -2733,6 +2733,10 @@ hns3_get_capability(struct hns3_hw *hw)
        if (ret)
                return ret;
 
+       ret = hns3_query_mac_stats_reg_num(hw);
+       if (ret)
+               return ret;
+
        if (hw->revision < PCI_REVISION_ID_HIP09_A) {
                hns3_set_default_dev_specifications(hw);
                hw->intr.mapping_mode = HNS3_INTR_MAPPING_VEC_RSV_ONE;
index cf6380e..ef028a7 100644 (file)
@@ -500,6 +500,7 @@ struct hns3_hw {
        struct hns3_tqp_stats tqp_stats;
        /* Include Mac stats | Rx stats | Tx stats */
        struct hns3_mac_stats mac_stats;
+       uint32_t mac_stats_reg_num;
        struct hns3_rx_missed_stats imissed_stats;
        uint64_t oerror_stats;
        uint32_t fw_version;
index 606b725..806720f 100644 (file)
@@ -307,24 +307,21 @@ static const struct hns3_xstats_name_offset hns3_imissed_stats_strings[] = {
 
 static void hns3_tqp_stats_clear(struct hns3_hw *hw);
 
-/*
- * Query all the MAC statistics data of Network ICL command ,opcode id: 0x0034.
- * This command is used before send 'query_mac_stat command', the descriptor
- * number of 'query_mac_stat command' must match with reg_num in this command.
- * @praram hw
- *   Pointer to structure hns3_hw.
- * @return
- *   0 on success.
- */
 static int
-hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
+hns3_update_mac_stats(struct hns3_hw *hw)
 {
+#define HNS3_MAC_STATS_REG_NUM_PER_DESC        4
+
        uint64_t *data = (uint64_t *)(&hw->mac_stats);
        struct hns3_cmd_desc *desc;
+       uint32_t stats_iterms;
        uint64_t *desc_data;
-       uint16_t i, k, n;
+       uint32_t desc_num;
+       uint16_t i;
        int ret;
 
+       /* The first desc has a 64-bit header, so need to consider it. */
+       desc_num = hw->mac_stats_reg_num / HNS3_MAC_STATS_REG_NUM_PER_DESC + 1;
        desc = rte_malloc("hns3_mac_desc",
                          desc_num * sizeof(struct hns3_cmd_desc), 0);
        if (desc == NULL) {
@@ -340,65 +337,71 @@ hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
                return ret;
        }
 
-       for (i = 0; i < desc_num; i++) {
-               /* For special opcode 0034, only the first desc has the head */
-               if (i == 0) {
-                       desc_data = (uint64_t *)(&desc[i].data[0]);
-                       n = HNS3_RD_FIRST_STATS_NUM;
-               } else {
-                       desc_data = (uint64_t *)(&desc[i]);
-                       n = HNS3_RD_OTHER_STATS_NUM;
-               }
-
-               for (k = 0; k < n; k++) {
-                       *data += rte_le_to_cpu_64(*desc_data);
-                       data++;
-                       desc_data++;
-               }
+       stats_iterms = RTE_MIN(sizeof(hw->mac_stats) / sizeof(uint64_t),
+                              hw->mac_stats_reg_num);
+       desc_data = (uint64_t *)(&desc[0].data[0]);
+       for (i = 0; i < stats_iterms; i++) {
+               /*
+                * Data memory is continuous and only the first descriptor has a
+                * header in this command.
+                */
+               *data += rte_le_to_cpu_64(*desc_data);
+               data++;
+               desc_data++;
        }
        rte_free(desc);
 
        return 0;
 }
 
-/*
- * Query Mac stat reg num command ,opcode id: 0x0033.
- * This command is used before send 'query_mac_stat command', the descriptor
- * number of 'query_mac_stat command' must match with reg_num in this command.
- * @praram rte_stats
- *   Pointer to structure rte_eth_stats.
- * @return
- *   0 on success.
- */
 static int
-hns3_mac_query_reg_num(struct rte_eth_dev *dev, uint32_t *desc_num)
+hns3_mac_query_reg_num(struct hns3_hw *hw, uint32_t *reg_num)
 {
-       struct hns3_adapter *hns = dev->data->dev_private;
-       struct hns3_hw *hw = &hns->hw;
+#define HNS3_MAC_STATS_RSV_REG_NUM_ON_HIP08_B  3
        struct hns3_cmd_desc desc;
-       uint32_t *desc_data;
-       uint32_t reg_num;
        int ret;
 
        hns3_cmd_setup_basic_desc(&desc, HNS3_OPC_QUERY_MAC_REG_NUM, true);
        ret = hns3_cmd_send(hw, &desc, 1);
-       if (ret)
+       if (ret) {
+               hns3_err(hw, "failed to query MAC statistic reg number, ret = %d",
+                        ret);
                return ret;
+       }
 
-       /*
-        * The num of MAC statistics registers that are provided by IMP in this
-        * version.
-        */
-       desc_data = (uint32_t *)(&desc.data[0]);
-       reg_num = rte_le_to_cpu_32(*desc_data);
+       /* The number of MAC statistics registers are provided by firmware. */
+       *reg_num = rte_le_to_cpu_32(desc.data[0]);
+       if (*reg_num == 0) {
+               hns3_err(hw, "MAC statistic reg number is invalid!");
+               return -ENODATA;
+       }
 
        /*
-        * The descriptor number of 'query_additional_mac_stat command' is
-        * '1 + (reg_num-3)/4 + ((reg_num-3)%4 !=0)';
-        * This value is 83 in this version
+        * If driver doesn't request the firmware to report more MAC statistics
+        * iterms and the total number of MAC statistics registers by using new
+        * method, firmware will only reports the number of valid statistics
+        * registers. However, structure hns3_mac_stats in driver contains valid
+        * and reserved statistics iterms. In this case, the total register
+        * number must be added to three reserved statistics registers.
         */
-       *desc_num = 1 + ((reg_num - 3) >> 2) +
-                   (uint32_t)(((reg_num - 3) & 0x3) ? 1 : 0);
+       *reg_num += HNS3_MAC_STATS_RSV_REG_NUM_ON_HIP08_B;
+
+       return 0;
+}
+
+int
+hns3_query_mac_stats_reg_num(struct hns3_hw *hw)
+{
+       uint32_t mac_stats_reg_num = 0;
+       int ret;
+
+       ret = hns3_mac_query_reg_num(hw, &mac_stats_reg_num);
+       if (ret)
+               return ret;
+
+       hw->mac_stats_reg_num = mac_stats_reg_num;
+       if (hw->mac_stats_reg_num > sizeof(hw->mac_stats) / sizeof(uint64_t))
+               hns3_warn(hw, "MAC stats reg number from firmware is greater than stats iterms in driver.");
 
        return 0;
 }
@@ -408,15 +411,8 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)
 {
        struct hns3_adapter *hns = dev->data->dev_private;
        struct hns3_hw *hw = &hns->hw;
-       uint32_t desc_num;
-       int ret;
 
-       ret = hns3_mac_query_reg_num(dev, &desc_num);
-       if (ret == 0)
-               ret = hns3_update_mac_stats(hw, desc_num);
-       else
-               hns3_err(hw, "Query mac reg num fail : %d", ret);
-       return ret;
+       return hns3_update_mac_stats(hw);
 }
 
 static int
index d1230f9..c81d351 100644 (file)
@@ -5,11 +5,6 @@
 #ifndef _HNS3_STATS_H_
 #define _HNS3_STATS_H_
 
-/* stats macro */
-#define HNS3_MAC_CMD_NUM               21
-#define HNS3_RD_FIRST_STATS_NUM                2
-#define HNS3_RD_OTHER_STATS_NUM                4
-
 /* TQP stats */
 struct hns3_tqp_stats {
        uint64_t rcb_tx_ring_pktnum_rcd; /* Total num of transmitted packets */
@@ -22,6 +17,7 @@ struct hns3_tqp_stats {
 struct hns3_mac_stats {
        uint64_t mac_tx_mac_pause_num;
        uint64_t mac_rx_mac_pause_num;
+       uint64_t rsv0;
        uint64_t mac_tx_pfc_pri0_pkt_num;
        uint64_t mac_tx_pfc_pri1_pkt_num;
        uint64_t mac_tx_pfc_pri2_pkt_num;
@@ -58,7 +54,7 @@ struct hns3_mac_stats {
        uint64_t mac_tx_1519_2047_oct_pkt_num;
        uint64_t mac_tx_2048_4095_oct_pkt_num;
        uint64_t mac_tx_4096_8191_oct_pkt_num;
-       uint64_t rsv0;
+       uint64_t rsv1;
        uint64_t mac_tx_8192_9216_oct_pkt_num;
        uint64_t mac_tx_9217_12287_oct_pkt_num;
        uint64_t mac_tx_12288_16383_oct_pkt_num;
@@ -85,7 +81,7 @@ struct hns3_mac_stats {
        uint64_t mac_rx_1519_2047_oct_pkt_num;
        uint64_t mac_rx_2048_4095_oct_pkt_num;
        uint64_t mac_rx_4096_8191_oct_pkt_num;
-       uint64_t rsv1;
+       uint64_t rsv2;
        uint64_t mac_rx_8192_9216_oct_pkt_num;
        uint64_t mac_rx_9217_12287_oct_pkt_num;
        uint64_t mac_rx_12288_16383_oct_pkt_num;
@@ -168,5 +164,6 @@ int hns3_stats_reset(struct rte_eth_dev *dev);
 int hns3_tqp_stats_init(struct hns3_hw *hw);
 void hns3_tqp_stats_uninit(struct hns3_hw *hw);
 int hns3_update_imissed_stats(struct hns3_hw *hw, bool is_clear);
+int hns3_query_mac_stats_reg_num(struct hns3_hw *hw);
 
 #endif /* _HNS3_STATS_H_ */