patch 'net/hns3: fix insecure way to query MAC statistics' has been queued to stable release 20.11.5

[luca.boccassi@gmail.com]

Hi,

FYI, your patch has been queued to stable release 20.11.5

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 02/20/22. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/bluca/dpdk-stable

This queued commit can be viewed at:
https://github.com/bluca/dpdk-stable/commit/b65114d8a356dab639016ecf64345b67e5642982

Thanks.

Luca Boccassi

---
From b65114d8a356dab639016ecf64345b67e5642982 Mon Sep 17 00:00:00 2001
From: Huisong Li <lihuisong@huawei.com>
Date: Fri, 28 Jan 2022 10:07:05 +0800
Subject: [PATCH] net/hns3: fix insecure way to query MAC statistics

[ upstream commit 6ee07e3cb589145d0f0826f918fd44eb3e160a08 ]

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")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c |   4 ++
 drivers/net/hns3/hns3_ethdev.h |   1 +
 drivers/net/hns3/hns3_stats.c  | 120 ++++++++++++++++-----------------
 drivers/net/hns3/hns3_stats.h  |  11 ++-
 4 files changed, 67 insertions(+), 69 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 8514d62617..3c8d41c6c8 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -3102,6 +3102,10 @@ hns3_get_capability(struct hns3_hw *hw)
 	}
 	hw->revision = revision;
 
+	ret = hns3_query_mac_stats_reg_num(hw);
+	if (ret)
+		return ret;
+
 	if (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 d6d9b0504a..5717f54441 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -431,6 +431,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;
 	uint32_t fw_version;
 
 	uint16_t num_msi;
diff --git a/drivers/net/hns3/hns3_stats.c b/drivers/net/hns3/hns3_stats.c
index fc577ddf45..04702f17d1 100644
--- a/drivers/net/hns3/hns3_stats.c
+++ b/drivers/net/hns3/hns3_stats.c
@@ -328,24 +328,21 @@ static const struct hns3_xstats_name_offset hns3_tx_queue_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) {
@@ -361,65 +358,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) {
+		hns3_err(hw, "failed to query MAC statistic reg number, ret = %d",
+			 ret);
+		return ret;
+	}
+
+	/* 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;
+	}
+
+	/*
+	 * 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.
+	 */
+	*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;
 
-	/*
-	 * 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 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
-	 */
-	*desc_num = 1 + ((reg_num - 3) >> 2) +
-		    (uint32_t)(((reg_num - 3) & 0x3) ? 1 : 0);
+	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;
 }
@@ -429,15 +432,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);
 }
 
 /* Get tqp stats from register */
diff --git a/drivers/net/hns3/hns3_stats.h b/drivers/net/hns3/hns3_stats.h
index 7b9b502a6b..436fac3b31 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;
@@ -151,5 +147,6 @@ int hns3_stats_reset(struct rte_eth_dev *dev);
 void hns3_error_int_stats_add(struct hns3_adapter *hns, const char *err);
 int hns3_tqp_stats_init(struct hns3_hw *hw);
 void hns3_tqp_stats_uninit(struct hns3_hw *hw);
+int hns3_query_mac_stats_reg_num(struct hns3_hw *hw);
 
 #endif /* _HNS3_STATS_H_ */
-- 
2.30.2

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2022-02-18 12:37:41.150237151 +0000
+++ 0085-net-hns3-fix-insecure-way-to-query-MAC-statistics.patch	2022-02-18 12:37:37.774793796 +0000
@@ -1 +1 @@
-From 6ee07e3cb589145d0f0826f918fd44eb3e160a08 Mon Sep 17 00:00:00 2001
+From b65114d8a356dab639016ecf64345b67e5642982 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 6ee07e3cb589145d0f0826f918fd44eb3e160a08 ]
+
@@ -15 +16,0 @@
-Cc: stable@dpdk.org
@@ -27 +28 @@
-index 73bf209717..57f1572340 100644
+index 8514d62617..3c8d41c6c8 100644
@@ -30,3 +31,3 @@
-@@ -2733,6 +2733,10 @@ hns3_get_capability(struct hns3_hw *hw)
- 	if (ret)
- 		return ret;
+@@ -3102,6 +3102,10 @@ hns3_get_capability(struct hns3_hw *hw)
+ 	}
+ 	hw->revision = revision;
@@ -38 +39 @@
- 	if (hw->revision < PCI_REVISION_ID_HIP09_A) {
+ 	if (revision < PCI_REVISION_ID_HIP09_A) {
@@ -42 +43 @@
-index cf6380ebb2..ef028a7b2c 100644
+index d6d9b0504a..5717f54441 100644
@@ -45 +46 @@
-@@ -500,6 +500,7 @@ struct hns3_hw {
+@@ -431,6 +431,7 @@ struct hns3_hw {
@@ -50,2 +50,0 @@
- 	struct hns3_rx_missed_stats imissed_stats;
- 	uint64_t oerror_stats;
@@ -52,0 +52,2 @@
+ 
+ 	uint16_t num_msi;
@@ -54 +55 @@
-index 606b72509a..806720faff 100644
+index fc577ddf45..04702f17d1 100644
@@ -57 +58 @@
-@@ -307,24 +307,21 @@ static const struct hns3_xstats_name_offset hns3_imissed_stats_strings[] = {
+@@ -328,24 +328,21 @@ static const struct hns3_xstats_name_offset hns3_tx_queue_strings[] = {
@@ -90 +91 @@
-@@ -340,65 +337,71 @@ hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
+@@ -361,65 +358,71 @@ hns3_update_mac_stats(struct hns3_hw *hw, const uint32_t desc_num)
@@ -205 +206 @@
-@@ -408,15 +411,8 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)
+@@ -429,15 +432,8 @@ hns3_query_update_mac_stats(struct rte_eth_dev *dev)
@@ -221 +222 @@
- static int
+ /* Get tqp stats from register */
@@ -223 +224 @@
-index d1230f94cb..c81d351082 100644
+index 7b9b502a6b..436fac3b31 100644
@@ -264 +265,2 @@
-@@ -168,5 +164,6 @@ int hns3_stats_reset(struct rte_eth_dev *dev);
+@@ -151,5 +147,6 @@ int hns3_stats_reset(struct rte_eth_dev *dev);
+ void hns3_error_int_stats_add(struct hns3_adapter *hns, const char *err);
@@ -267 +268,0 @@
- int hns3_update_imissed_stats(struct hns3_hw *hw, bool is_clear);