Skip to content

Commit

Permalink
net/hns3: fix insecure way to query MAC statistics
Browse files Browse the repository at this point in the history 
[ upstream commit 6ee07e3 ]

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: 8839c5e ("net/hns3: support device stats")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
  • Loading branch information
@LiHuiSong1 @bluca
LiHuiSong1 authored and bluca committed Feb 17, 2022
1 parent 73d4297 commit b65114d
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 67 deletions.
4 changes: 4 additions & 0 deletions drivers/net/hns3/hns3_ethdev.c
View file
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/hns3/hns3_ethdev.h
View file
Expand Up @@ -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;
Expand Down
116 changes: 56 additions & 60 deletions drivers/net/hns3/hns3_stats.c
View file
Expand Up @@ -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) {
Expand All @@ -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)
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;
}
Expand All @@ -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 */
Expand Down
11 changes: 4 additions & 7 deletions drivers/net/hns3/hns3_stats.h
View file
Expand Up @@ -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 */
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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_ */

0 comments on commit b65114d

Please sign in to comment.