From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C79A6A034E; Fri, 28 Jan 2022 03:07:26 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3A0094284E; Fri, 28 Jan 2022 03:07:03 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 1F18A4277B for ; Fri, 28 Jan 2022 03:06:57 +0100 (CET) Received: from dggeme756-chm.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4JlLQP6PGCz9sHY; Fri, 28 Jan 2022 10:05:33 +0800 (CST) Received: from localhost.localdomain (10.69.192.56) by dggeme756-chm.china.huawei.com (10.3.19.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2308.21; Fri, 28 Jan 2022 10:06:54 +0800 From: "Min Hu (Connor)" To: CC: , Subject: [PATCH 3/6] net/hns3: fix insecure way to query MAC statistics Date: Fri, 28 Jan 2022 10:07:05 +0800 Message-ID: <20220128020708.62787-4-humin29@huawei.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20220128020708.62787-1-humin29@huawei.com> References: <20220128020708.62787-1-humin29@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.69.192.56] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To dggeme756-chm.china.huawei.com (10.3.19.102) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org From: Huisong Li 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 Signed-off-by: Min Hu (Connor) --- drivers/net/hns3/hns3_ethdev.c | 4 ++ drivers/net/hns3/hns3_ethdev.h | 1 + drivers/net/hns3/hns3_stats.c | 116 ++++++++++++++++----------------- drivers/net/hns3/hns3_stats.h | 11 ++-- 4 files changed, 65 insertions(+), 67 deletions(-) diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c index 73bf209717..57f1572340 100644 --- a/drivers/net/hns3/hns3_ethdev.c +++ b/drivers/net/hns3/hns3_ethdev.c @@ -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; diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h index cf6380ebb2..ef028a7b2c 100644 --- a/drivers/net/hns3/hns3_ethdev.h +++ b/drivers/net/hns3/hns3_ethdev.h @@ -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; diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c index 606b72509a..806720faff 100644 --- a/drivers/net/hns3/hns3_stats.c +++ b/drivers/net/hns3/hns3_stats.c @@ -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 diff --git a/drivers/net/hns3/hns3_stats.h b/drivers/net/hns3/hns3_stats.h index d1230f94cb..c81d351082 100644 --- a/drivers/net/hns3/hns3_stats.h +++ b/drivers/net/hns3/hns3_stats.h @@ -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_ */ -- 2.33.0