From 2994a8d169adc6ba5b543a2f35b579e437044beb Mon Sep 17 00:00:00 2001 From: Jie Hai Date: Tue, 6 Aug 2024 16:43:24 +0800 Subject: [PATCH 08/27] hikptool/nic: Fix the Code review comments The value got from the firmware may cause the following problem. 1. Out-of-bounds access may occur. 2. Apply for memory of size 0. 3. Integer overflow may happen. This patch fixes it. Signed-off-by: Jie Hai --- net/nic/nic_ppp/hikp_nic_ppp.c | 142 +++++++++++++++++++++++++++++++-- 1 file changed, 137 insertions(+), 5 deletions(-) diff --git a/net/nic/nic_ppp/hikp_nic_ppp.c b/net/nic/nic_ppp/hikp_nic_ppp.c index 4dbe8d6..c070666 100644 --- a/net/nic/nic_ppp/hikp_nic_ppp.c +++ b/net/nic/nic_ppp/hikp_nic_ppp.c @@ -42,6 +42,8 @@ static int hikp_nic_query_ppp_by_entryid(struct hikp_cmd_header *req_header, #define NIC_PPP_VLAN_TBL_NAME "vlan" #define NIC_PPP_MNG_TBL_NAME "mng" +#define HIKP_PPP_MAX_MAC_ID_NUM 8 + static const struct ppp_feature_cmd g_ppp_feature_cmd[] = { {NIC_PPP_MAC_TBL_NAME, NIC_MAC_TBL_DUMP, true, hikp_nic_query_ppp_by_entryid, hikp_nic_ppp_show_mac_tbl}, @@ -259,6 +261,9 @@ static void hikp_nic_ppp_show_mac_tbl(const void *data) hikp_nic_ppp_show_key_mem((struct nic_mac_tbl *)data, true); + if (g_ppp_hw_res.overflow_cam_size == 0) + return; + of_uc_entry = (struct mac_vlan_uc_entry *)calloc(g_ppp_hw_res.overflow_cam_size, sizeof(struct mac_vlan_uc_entry)); if (of_uc_entry == NULL) { @@ -326,8 +331,9 @@ static void hikp_nic_ppp_show_vf_vlan_info(const struct vf_vlan_tbl *vf_tbl, uin offset = abs_func_id % HIKP_NIC_PPP_FUNC_BITMAP_SIZE; hikp_nic_ppp_get_func_name(func_name, sizeof(func_name), func_id); printf("%s_abs_func_id: %u\n", func_name, - (uint32_t)(g_ppp_hw_res.abs_func_id_base + func_id - 1)); + (uint32_t)(hw_res->abs_func_id_base + func_id - 1)); printf("%s VLAN id:\n\t", func_name); + for (i = 0; i < vf_tbl->entry_size; i++) { vf_entry = &vf_tbl->entry[i]; if (hikp_get_bit(vf_entry->func_bitmap[idx], offset) != 0) { @@ -490,10 +496,11 @@ static int hikp_nic_ppp_get_blk(struct hikp_cmd_header *req_header, goto out; rsp = (struct nic_ppp_rsp *)cmd_ret->rsp_data; - if (rsp->rsp_head.cur_blk_size > buf_len) { + if (rsp->rsp_head.cur_blk_size > buf_len || + rsp->rsp_head.cur_blk_size > sizeof(rsp->rsp_data)) { HIKP_ERROR_PRINT("nic_ppp block context copy size error, " - "buffer size=%zu, data size=%u.\n", - buf_len, rsp->rsp_head.cur_blk_size); + "dst buffer size=%zu, src buffer size=%zu, data size=%u.\n", + buf_len, sizeof(rsp->rsp_data), rsp->rsp_head.cur_blk_size); ret = -EINVAL; goto out; } @@ -532,6 +539,17 @@ static int hikp_nic_ppp_query_uc_mac_addr(struct hikp_cmd_header *req_header, idx, ret); return ret; } + if (rsp_head.next_entry_idx <= idx) { + HIKP_ERROR_PRINT("Next entry index (%u) should be greater than current (%u).\n", + rsp_head.next_entry_idx, idx); + return -EINVAL; + } + if (entry_size + rsp_head.cur_blk_entry_cnt > max_hw_entry_size) { + HIKP_ERROR_PRINT("The sum of entry number (%lu) after block-%u " + "is over the maximum entry nubmer (%u) of unicast MAC table.\n", + entry_size + rsp_head.cur_blk_entry_cnt, idx, max_hw_entry_size); + return -EINVAL; + } entry_size += rsp_head.cur_blk_entry_cnt; idx = rsp_head.next_entry_idx; } @@ -563,6 +581,17 @@ static int hikp_nic_ppp_query_mc_mac_addr(struct hikp_cmd_header *req_header, idx, ret); return ret; } + if (rsp_head.next_entry_idx <= idx) { + HIKP_ERROR_PRINT("Next entry index (%u) should be greater than current (%u).\n", + rsp_head.next_entry_idx, idx); + return -EINVAL; + } + if (entry_size + rsp_head.cur_blk_entry_cnt > max_hw_entry_size) { + HIKP_ERROR_PRINT("The sum of entry number (%lu) after block-%u " + "is over the maximum entry nubmer (%u) of multicast MAC table.\n", + entry_size + rsp_head.cur_blk_entry_cnt, idx, max_hw_entry_size); + return -EINVAL; + } entry_size += rsp_head.cur_blk_entry_cnt; idx = rsp_head.next_entry_idx; } @@ -623,6 +652,17 @@ static int hikp_nic_ppp_query_vf_vlan_tbl(struct hikp_cmd_header *req_header, idx, ret); return ret; } + if (rsp_head.next_entry_idx <= idx) { + HIKP_ERROR_PRINT("Next entry index (%u) should be greater than current (%u).\n", + rsp_head.next_entry_idx, idx); + return -EINVAL; + } + if (entry_size + rsp_head.cur_blk_entry_cnt > hw_entry_size) { + HIKP_ERROR_PRINT("The sum of entry number (%lu) after block-%u " + "is over the maximum entry nubmer (%u) of VF VLAN table.\n", + entry_size + rsp_head.cur_blk_entry_cnt, idx, hw_entry_size); + return -EINVAL; + } entry_size += rsp_head.cur_blk_entry_cnt; idx = rsp_head.next_entry_idx; } @@ -653,6 +693,17 @@ static int hikp_nic_ppp_query_port_vlan_tbl(struct hikp_cmd_header *req_header, idx, ret); return ret; } + if (rsp_head.next_entry_idx <= idx) { + HIKP_ERROR_PRINT("Next entry index (%u) should be greater than current (%u).\n", + rsp_head.next_entry_idx, idx); + return -EINVAL; + } + if (entry_size + rsp_head.cur_blk_entry_cnt > hw_entry_size) { + HIKP_ERROR_PRINT("The sum of entry number (%lu) after block-%u " + "is over the maximum entry nubmer (%u) of port VLAN table.\n", + entry_size + rsp_head.cur_blk_entry_cnt, idx, hw_entry_size); + return -EINVAL; + } entry_size += rsp_head.cur_blk_entry_cnt; idx = rsp_head.next_entry_idx; } @@ -704,6 +755,17 @@ static int hikp_nic_query_mng_tbl(struct hikp_cmd_header *req_header, idx, ret); return ret; } + if (rsp_head.next_entry_idx <= idx) { + HIKP_ERROR_PRINT("Next entry index (%u) should be greater than current (%u).\n", + rsp_head.next_entry_idx, idx); + return -EINVAL; + } + if (entry_size + rsp_head.cur_blk_entry_cnt > g_ppp_hw_res.mng_tbl_size) { + HIKP_ERROR_PRINT("The sum of entry number (%lu) after block-%u " + "is over the maximum entry nubmer (%u) of manager table.\n", + entry_size + rsp_head.cur_blk_entry_cnt, idx, g_ppp_hw_res.mng_tbl_size); + return -EINVAL; + } entry_size += rsp_head.cur_blk_entry_cnt; idx = rsp_head.next_entry_idx; } @@ -729,6 +791,24 @@ static int hikp_nic_query_ppp_by_entryid(struct hikp_cmd_header *req_header, return hikp_nic_query_mng_tbl(req_header, &req_data, (struct nic_mng_tbl *)data); } +static int hikp_nic_ppp_check_func_num(void *data) +{ + const struct ppp_feature_cmd *ppp_cmd; + uint16_t func_num = 0; + + ppp_cmd = &g_ppp_feature_cmd[g_ppp_param.feature_idx]; + if (ppp_cmd->sub_cmd_code == NIC_PROMISCUOUS_TBL_DUMP) + func_num = ((struct nic_promisc_tbl *)data)->func_num; + else if (ppp_cmd->sub_cmd_code == NIC_VLAN_OFFLOAD_DUMP) + func_num = ((struct nic_vlan_offload_cfg *)data)->func_num; + + if (func_num > HIKP_NIC_MAX_FUNC_NUM) { + HIKP_ERROR_PRINT("Illegal function num(%u) from firmware.\n", func_num); + return -EINVAL; + } + return 0; +} + static int hikp_nic_query_ppp_by_blkid(struct hikp_cmd_header *req_header, const struct bdf_t *bdf, void *data, size_t len) { @@ -752,6 +832,10 @@ static int hikp_nic_query_ppp_by_blkid(struct hikp_cmd_header *req_header, const /* Copy the remaining block content if total block number is greater than 1. */ for (blk_id = 1; blk_id < total_blk_num; blk_id++) { req_data.block_id = blk_id; + if (len <= total_blk_size) { + HIKP_ERROR_PRINT("No enough buffer to get block-%u context.\n", blk_id); + return -ENOMEM; + } ret = hikp_nic_ppp_get_blk(req_header, &req_data, (uint8_t *)data + total_blk_size, len - total_blk_size, &rsp_head); if (ret != 0) { @@ -761,7 +845,7 @@ static int hikp_nic_query_ppp_by_blkid(struct hikp_cmd_header *req_header, const total_blk_size += rsp_head.cur_blk_size; } - return ret; + return hikp_nic_ppp_check_func_num(data); } static int hikp_nic_ppp_get_hw_resources(const struct bdf_t *bdf, @@ -975,6 +1059,46 @@ static int hikp_nic_check_func_id_valid(struct major_cmd_ctrl *self, return 0; } +static int hikp_nic_check_hw_res(struct hikp_nic_ppp_hw_resources *hw_res) +{ + if (!g_ppp_feature_cmd[g_ppp_param.feature_idx].need_query_hw_res) + return 0; + + if (hw_res->max_key_mem_size == 0) { + HIKP_ERROR_PRINT("Max key memory size should not be zero!\n"); + return -EINVAL; + } + if (hw_res->port_vlan_tbl_size == 0) { + HIKP_ERROR_PRINT("PORT VLAN Table size should not be zero!\n"); + return -EINVAL; + } + if (hw_res->vf_vlan_tbl_size == 0) { + HIKP_ERROR_PRINT("VF VLAN Table size should not be zero!\n"); + return -EINVAL; + } + if (hw_res->mng_tbl_size == 0) { + HIKP_ERROR_PRINT("VF VLAN Table size (%u）is zero!\n"); + return -EINVAL; + } + if (hw_res->mac_id >= HIKP_PPP_MAX_MAC_ID_NUM) { + HIKP_ERROR_PRINT("MAC ID (%u) should be less than %u.\n", + hw_res->mac_id, HIKP_PPP_MAX_MAC_ID_NUM); + return -EINVAL; + } + if (hw_res->total_func_num == 0 || hw_res->total_func_num > HIKP_NIC_MAX_FUNC_NUM) { + HIKP_ERROR_PRINT("Total_func_num (%u）should be in [1, %u].\n", + hw_res->total_func_num, HIKP_NIC_MAX_FUNC_NUM); + return -EINVAL; + } + if (hw_res->abs_func_id_base >= HIKP_NIC_MAX_FUNC_NUM) { + HIKP_ERROR_PRINT("Function ID base (%u) should be less than %u.\n", + hw_res->abs_func_id_base, HIKP_NIC_MAX_FUNC_NUM); + return -EINVAL; + } + + return 0; +} + static void hikp_nic_ppp_cmd_execute(struct major_cmd_ctrl *self) { struct bdf_t *bdf = &g_ppp_param.target.bdf; @@ -995,6 +1119,14 @@ static void hikp_nic_ppp_cmd_execute(struct major_cmd_ctrl *self) return; } + ret = hikp_nic_check_hw_res(&g_ppp_hw_res); + if (ret != 0) { + snprintf(self->err_str, sizeof(self->err_str), + "ppp hardware resources obtained is invalid."); + self->err_no = ret; + return; + } + ppp_cmd = &g_ppp_feature_cmd[g_ppp_param.feature_idx]; ret = hikp_nic_check_func_id_valid(self, ppp_cmd, &g_ppp_param, &g_ppp_hw_res); if (ret != 0) -- 2.45.0.windows.1