Мы используем куки, чтобы пользоваться сайтом
было удобно.
Мы ответим вам на
Если вы так и не получили ответ, пожалуйста, проверьте, отфильтровано ли письмо в одну из следующих стандартных папок:
Вебинар: C# разработка и статический анализ: в чем практическая польза? - 18.11
В своей обители в Р'лайхе мёртвый Ктулху спит в ожидании своего часа. А в C коде проекта DPDK спит множество ошибок, и тоже в ожидании своего часа. Давайте посмотрим, какие из них может выявить анализатор PVS-Studio.
Data Plane Development Kit (DPDK) — открытый проект, представляющий собой набор библиотек для работы с сетью. Создан в целях переноса обработки TCP/IP пакетов с уровня ядра операционной системы в пользовательское пространство. Подход практически исключает использование системных прерываний для обращения к сетевым данным, что обеспечивает более высокий КПД используемых ресурсов.
Не будем задерживаться и сразу приступим к ошибкам. Их много.
Десять первых багов (точнее даже чуть больше) относятся к юнит-тестам и были рассмотрены в предыдущей статье "Поиск ошибок в юнит-тестах".
void nthw_hif_delete(nthw_hif_t *p)
{
if (p) {
memset(p, 0, sizeof(nthw_hif_t));
free(p);
}
}Предупреждение PVS-Studio:
V597 The compiler could delete the 'memset' function call, which is used to flush 'p' object. The memset_s() function should be used to erase the private data. nthw_hif.c 27
Классика: неправильное заполнение буфера, содержащего приватные данные. Годы идут, но дефекты безопасности этого типа встречаются всё так же часто. Количество таких ошибок, которое мы собрали при проверке открытых проектов, уже перевалило за 300.
Компилятор видит, что занулённый буфер затем сразу освобождается. Благодаря правилу as-if, он может удалить вызов memset, т.к. такая оптимизация не поменяет наблюдаемого поведения программы.
В памяти остаются приватные данные, которые разработчик на всякий случай хотел уничтожить.
Если вы впервые столкнулись с этим видом дефекта, то рекомендую следующие материалы:
Встретилась функция, связанная с криптографией. В ней есть буфер, который хотят предварительно обнулить перед использованием, но из-за простейшей ошибки этого не произойдёт. В буфере останутся случайные мусорные данные.
Дело в том, что в момент вызова функции memset переменная state1_size, задающая количество заполняемых байт, остаётся равна нулю. Соответственно, memset заполняет ноль байт.
int qat_sym_cd_auth_set(....)
{
....
uint16_t state1_size = 0, state2_size = 0, cd_extra_size = 0;
....
switch (cdesc->qat_hash_alg) {
....
case ICP_QAT_HW_AUTH_ALGO_SHA3_224:
/* Plain SHA3-224 */
memset(cdesc->cd_cur_ptr, 0, state1_size); // <= BUG N1
state1_size = qat_hash_get_state1_size(
cdesc->qat_hash_alg);
break;
case ICP_QAT_HW_AUTH_ALGO_SHA3_256:
/* Plain SHA3-256 */
memset(cdesc->cd_cur_ptr, 0, state1_size); // <= BUG N2
state1_size = qat_hash_get_state1_size(
cdesc->qat_hash_alg);
break;
case ICP_QAT_HW_AUTH_ALGO_SHA3_384:
/* Plain SHA3-384 */
memset(cdesc->cd_cur_ptr, 0, state1_size); // <= BUG N3
state1_size = qat_hash_get_state1_size(
cdesc->qat_hash_alg);
break;
case ICP_QAT_HW_AUTH_ALGO_SHA3_512:
/* Plain SHA3-512 */
memset(cdesc->cd_cur_ptr, 0, state1_size); // <= BUG N4
state1_size = qat_hash_get_state1_size(
cdesc->qat_hash_alg);
break;
....
}
....
}Предупреждения PVS-Studio:
Все четыре фрагмента однотипны. Посмотрим ещё раз на первый из них:
memset(cdesc->cd_cur_ptr, 0, state1_size);
state1_size = qat_hash_get_state1_size(cdesc->qat_hash_alg);У меня есть большое подозрение, что эти две строки просто перепутаны местами. В начале нужно вычислить значение переменной state1_size, а уже затем использовать её при вызове memset:
state1_size = qat_hash_get_state1_size(cdesc->qat_hash_alg);
memset(cdesc->cd_cur_ptr, 0, state1_size);static int
parse_repr(const char *key __rte_unused, const char *value, void *args)
{
....
const char *str = value;
....
if (str == NULL) {
PMD_DRV_LOG(ERR, "wrong representor format: %s", str);
return -1;
}
....
}Предупреждение PVS-Studio:
V576 Incorrect format. Consider checking the fifth actual argument of the 'rte_log' function. A null pointer is used. cpfl_ethdev.c 1583
Если указатель на строку оказался нулевым, то его попробуют распечатать при формировании сообщения об ошибке. Так себе идея.
s32 e1000_set_mac_type(struct e1000_hw *hw)
{
....
switch (hw->device_id) {
....
case E1000_DEV_ID_PCH_ADL_I219_LM16:
case E1000_DEV_ID_PCH_ADL_I219_V16:
case E1000_DEV_ID_PCH_RPL_I219_LM23:
case E1000_DEV_ID_PCH_RPL_I219_V23:
mac->type = e1000_pch_tgp; // <=
case E1000_DEV_ID_PCH_ADL_I219_LM17:
case E1000_DEV_ID_PCH_ADL_I219_V17:
case E1000_DEV_ID_PCH_RPL_I219_LM22:
case E1000_DEV_ID_PCH_RPL_I219_V22:
mac->type = e1000_pch_adp; // <=
break;
case E1000_DEV_ID_82575EB_COPPER:
case E1000_DEV_ID_82575EB_FIBER_SERDES:
case E1000_DEV_ID_82575GB_QUAD_COPPER:
mac->type = e1000_82575;
break;
....
}
....
}Предупреждение PVS-Studio:
V796 It is possible that 'break' statement is missing in switch statement. e1000_api.c 297
Пояснения избыточны. Одна из любимых ошибок C и C++ программистов.
static int
cnxk_gpio_read_attr(char *attr, char *val)
{
FILE *fp;
int ret;
fp = fopen(attr, "r");
if (!fp)
return -errno;
ret = fscanf(fp, "%s", val);
if (ret < 0)
return -errno; // <=
....
}Предупреждение PVS-Studio:
V773 The function was exited without closing the file referenced by the 'fp' handle. A resource leak is possible. cnxk_gpio_selftest.c 46
Если удастся открыть файл, но не удастся прочитать из него строку текста, то произойдёт выход из функции без закрытия файлового дескриптора. Итог:
static int
ot_ipsec_sa_common_param_fill(....)
{
....
if (w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_CBC ||
w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_CCM ||
w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_CTR ||
w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_GCM ||
w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_CCM ||
w2->s.auth_type == ROC_IE_OT_SA_AUTH_AES_GMAC) {
....
}Предупреждение PVS-Studio:
V501 There are identical sub-expressions 'w2->s.enc_type == ROC_IE_OT_SA_ENC_AES_CCM' to the left and to the right of the '||' operator. cnxk_security.c 177
Скорее всего, код писался копированием, поэтому случайно происходит двойное сравнение с константой ROC_IE_OT_SA_ENC_AES_CCM. Одна проверка или просто лишняя, и её можно удалить, или должно быть сравнение с какой-то другой константой.
Что интересно, Copy-Paste проявил себя ещё раз. Этот код размножен, и точно такое же дублирование проверки встречается ниже:
static int
nthw_pci_dev_init(struct rte_pci_device *pci_dev)
{
int num_port_speeds = 0;
....
// Значение переменной num_port_speeds не изменяется.
....
for (int i = 0; i < num_port_speeds; ++i) {
struct adapter_info_s *p_adapter_info = &p_nt_drv->adapter_info;
nt_link_speed_t link_speed = convert_link_speed(pls_mbps[i].link_speed);
port_ops->set_link_speed(p_adapter_info, i, link_speed);
}
....
}Предупреждение PVS-Studio:
V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. ntnic_ethdev.c 537
Где-то в коде забыли изменить значение переменной num_port_speeds, в результате чего цикл выполнит ноль итераций.
static inline int
i40e_flow_fdir_fill_eth_ip_head(....)
{
....
} else if (cus_pctype->index == I40E_CUSTOMIZED_IPV4_L2TPV3) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_L2TP,
len, ether_type);
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_ESP,
len, ether_type);
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
len, ether_type);
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP,
len, ether_type);
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6)
len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_ESP,
len, ether_type);
else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV6_UDP)
len = fill_ip6_head(fdir_input, raw_pkt, IPPROTO_UDP,
len, ether_type);
....
}Видите ошибку? Думаю, вам лень будет её искать. Скучно читать подобный код, тем более что это только часть кода с проверками. Поэтому ошибка и остаётся незамеченной.
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP, len, ether_type);
} else if (cus_pctype->index == I40E_CUSTOMIZED_ESP_IPV4_UDP) {
len = fill_ip4_head(fdir_input, raw_pkt, IPPROTO_UDP, len, ether_type);Два раза написано одно и то же. Кстати, тут даже и непонятно, что посоветовать, чтобы избегать подобных ошибок. Хорошо, что есть анализатор PVS-Studio, который помогает находить ошибки даже в скучном коде.
Предупреждение PVS-Studio:
V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 599, 602. i40e_fdir.c 599
static const char *const sa_nthw_fpga_bus_type_str[] = {
"ERR", /* NTHW_FPGA_BUS_TYPE_UNKNOWN, */
"BAR", /* NTHW_FPGA_BUS_TYPE_BAR, */
"PCI", /* NTHW_FPGA_BUS_TYPE_PCI, */
"CCIP", /* NTHW_FPGA_BUS_TYPE_CCIP, */
"RAB0", /* NTHW_FPGA_BUS_TYPE_RAB0, */
"RAB1", /* NTHW_FPGA_BUS_TYPE_RAB1, */
"RAB2", /* NTHW_FPGA_BUS_TYPE_RAB2, */
"NMB", /* NTHW_FPGA_BUS_TYPE_NMB, */
"NDM", /* NTHW_FPGA_BUS_TYPE_NDM, */
};
static const char *get_bus_name(int n_bus_type_id)
{
if (n_bus_type_id >= 1 &&
n_bus_type_id <= (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
return sa_nthw_fpga_bus_type_str[n_bus_type_id];
else
return "ERR";
}Предупреждение PVS-Studio:
V557 Array overrun is possible. The value of 'n_bus_type_id' index could reach 9. nthw_fpga_model.c 32
Прежде чем извлечь строчку из массива, выполняется проверка индекса n_bus_type_id. К этой проверке есть два вопроса:
Рискну предположить, что значения идентификаторов в переменной n_bus_type_id начинаются с 1. Тогда ошибка заключается в том, что забыли вычесть 1 перед извлечением элемента из массива. В этом случае корректным будет код:
static const char *get_bus_name(int n_bus_type_id)
{
if (n_bus_type_id >= 1 &&
n_bus_type_id <= (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
return sa_nthw_fpga_bus_type_str[n_bus_type_id - 1];
else
return "ERR";
}Впрочем, я не уверен. Странно, что никто не заметил, что функция возвращает не те строчки, которые должна. Возможно, всё-таки индексы нумеруются с 0. Тогда следует переписать проверку:
static const char *get_bus_name(int n_bus_type_id)
{
if (n_bus_type_id >= 0 &&
n_bus_type_id < (int)ARRAY_SIZE(sa_nthw_fpga_bus_type_str))
return sa_nthw_fpga_bus_type_str[n_bus_type_id];
else
return "ERR";
}Прошу простить мою неуверенность. Я первый раз вижу этот код. Мне очевидно, что код ошибочен, но, к сожалению, я ограничен во времени, чтобы подробнее изучать каждую найденную ошибку более глубоко. Их здесь десятки, а я один :)
/** Number of elements in the array. */
#define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0]))
int
rte_devargs_layers_parse(struct rte_devargs *devargs,
const char *devstr)
{
struct {
const char *key;
const char *str;
struct rte_kvargs *kvlist;
} layers[] = {
{ RTE_DEVARGS_KEY_BUS "=", NULL, NULL, },
{ RTE_DEVARGS_KEY_CLASS "=", NULL, NULL, },
{ RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, },
};
....
if (nblayer > RTE_DIM(layers)) {
ret = -E2BIG;
goto get_out;
}
layers[nblayer].str = s;
....
}Предупреждение PVS-Studio:
V557 Array overrun is possible. The value of 'nblayer' index could reach 3. eal_common_devargs.c 95
Опять неправильная проверка:
if (nblayer > RTE_DIM(layers)) {Если nblayer == RTE_DIM(layers), то произойдёт выход за границу массива. Правильный вариант кода:
if (nblayer >= RTE_DIM(layers)) {
ret = -E2BIG;
goto get_out;
}Аналогичные случаи:
Последний выход за границу массива настолько прекрасен сложностью обнаружения при обзоре кода, что я вынес его в отдельную статью. Надеюсь, вам тоже понравится: "Самая красивая ошибка, которую я нашёл с помощью PVS-Studio в 2024 году".
Собрал здесь четыре похожих случая. Независимо от условия, происходят одинаковые присваивания.
static uint32_t bnx2x_send_unload_req(struct bnx2x_softc *sc,
int unload_mode)
{
uint32_t reset_code = 0;
/* Select the UNLOAD request mode */
if (unload_mode == UNLOAD_NORMAL) {
reset_code = DRV_MSG_CODE_UNLOAD_REQ_WOL_DIS;
} else {
reset_code = DRV_MSG_CODE_UNLOAD_REQ_WOL_DIS;
}
....
}Предупреждение PVS-Studio:
V523 The 'then' statement is equivalent to the 'else' statement. bnx2x.c 1633
static s32 txgbe_read_phy_if(struct txgbe_hw *hw)
{
....
if (!hw->phy.phy_semaphore_mask) {
if (hw->bus.lan_id)
hw->phy.phy_semaphore_mask = TXGBE_MNGSEM_SWPHY;
else
hw->phy.phy_semaphore_mask = TXGBE_MNGSEM_SWPHY;
}
return 0;
}Предупреждение PVS-Studio:
V523 The 'then' statement is equivalent to the 'else' statement. txgbe_phy.c 87
int nthw_hif_init(nthw_hif_t *p, nthw_fpga_t *p_fpga, int n_instance)
{
....
p->mp_reg_build_seed = NULL; /* Reg/Fld not present on HIF */
if (p->mp_reg_build_seed)
p->mp_fld_build_seed = NULL; /* Reg/Fld not present on HIF */
else
p->mp_fld_build_seed = NULL;
....
}Предупреждение PVS-Studio:
V523 The 'then' statement is equivalent to the 'else' statement. nthw_hif.c 90
Подозреваю, что должны использоваться различные константы, но подвела внимательность. Если же написано именно так, как и задумывалось, то в коде явно не хватает поясняющих комментариев.
В завершение самый facepalm вариант.
Примечание. Это юбилейный баг. Он стал 16000-м в коллекции багов на нашем сайте.
static int
otx_ep_eth_dev_uninit(struct rte_eth_dev *eth_dev)
{
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
return 0;
}
eth_dev->dev_ops = NULL;
eth_dev->rx_pkt_burst = NULL;
eth_dev->tx_pkt_burst = NULL;
return 0;
}Предупреждение PVS-Studio:
V523 The 'then' statement is equivalent to the subsequent code fragment. otx_ep_ethdev.c 728
static void
parse_sync(struct ptpv2_data_slave_ordinary *ptp_data,
uint16_t rx_tstamp_idx)
{
....
if (memcmp(&ptp_hdr->source_port_id.clock_id,
&ptp_hdr->source_port_id.clock_id,
sizeof(struct clock_id)) == 0) {
....
}Предупреждение PVS-Studio:
V549 The first argument of 'memcmp' function is equal to the second argument. ptpclient.c 371
Программист опечатался в имени переменной, в результате чего вызов функции memcmp не имеет смысла.
#define HINIC_MAX_Q_FILTERS 64 /* hinic just support 64 filter types */
/* Structure to store filters' info. */
struct hinic_filter_info {
....
uint64_t type_mask; /* Bit mask for every used filter */
....
};
static inline void
hinic_ethertype_filter_remove(struct hinic_filter_info *filter_info,
uint8_t idx)
{
if (idx >= HINIC_MAX_Q_FILTERS)
return;
filter_info->pkt_type = 0;
filter_info->type_mask &= ~(1 << idx);
filter_info->pkt_filters[idx].pkt_proto = (uint16_t)0;
filter_info->pkt_filters[idx].enable = FALSE;
filter_info->pkt_filters[idx].qid = 0;
}На первый взгляд кажется, что всё хорошо. Есть 64-битная маска, в которой сбрасываются различные биты. Можете, кстати, сами в начале попробовать понять, что здесь не так.
Давайте соберём рядом все нужные элементы.
uint64_t type_mask; // 64-битная маска
uint8_t idx; // Номер бита, который нужно сбросить
// Есть защита от слишком большого значения
#define HINIC_MAX_Q_FILTERS 64
if (idx >= HINIC_MAX_Q_FILTERS) return;
filter_info->type_mask &= ~(1 << idx); // Сброс битаПредупреждение PVS-Studio:
V784 The size of the bit mask is less than the size of the first operand. This will cause the loss of higher bits. hinic_pmd_flow.c 2292
Дело в том, что символьный литерал 1 имеет 32-битный тип int! Результат сдвига также будет иметь тип int. Соответственно, невозможно получить маску, например, для 40-ого бита. Строго говоря, поведение такой операции не определено, если idx >= 32.
Но давайте пока остановимся на варианте, когда idx < 32. Хоть теперь поведение операции сдвига определено, всё по-прежнему плохо — код иногда портит старшие разряды в 64-битной маске. Представим, что idx будет равен 1. Что ожидает программист:
Что на самом деле получится:
Кажется, что всё то же самое... Только чуть длиннее.
Нет, следите за руками! Вернее, обратите внимание на нюанс, что знаковый бит оказался равен единице. А если он будет нулевым? Тогда сбросятся старшие биты маски. Смотрите, что будет, если мы захотим сбросить 32-ой бит:
Будут зря сброшены все старшие 32 разряда в 64-битной маске. Особенно баг подл тем, что редко проявляет себя.
Чтобы исправить ошибку, необходимо сдвигать единицу, имеющую тип uint64_t:
filter_info->type_mask &= ~(1ULL << idx); // OK
filter_info->type_mask &= ~(uint64_t(1) << idx); // OKВ начале будет много кода, чтобы можно было убедится, что это именно баг, а не хитрая задумка.
#define MAX_TLV_BUFFER 128 /* In dwords. 512 in bytes*/
typedef enum _lldp_agent_e {
LLDP_NEAREST_BRIDGE = 0,
LLDP_NEAREST_NON_TPMR_BRIDGE,
LLDP_NEAREST_CUSTOMER_BRIDGE,
LLDP_MAX_LLDP_AGENTS
} lldp_agent_e;
enum _ecore_status_t
ecore_lldp_mib_update_event(struct ecore_hwfn *p_hwfn,
struct ecore_ptt *p_ptt)
{
....
struct ecore_dcbx_mib_meta_data data;
enum _ecore_status_t rc = ECORE_SUCCESS;
struct lldp_received_tlvs_s tlvs;
int i;
for (i = 0; i < LLDP_MAX_LLDP_AGENTS; i++) {
OSAL_MEM_ZERO(&data, sizeof(data));
data.addr = p_hwfn->mcp_info->port_addr +
offsetof(struct public_port, lldp_received_tlvs[i]);
data.lldp_tlvs = &tlvs;
data.size = sizeof(tlvs);
rc = ecore_dcbx_copy_mib(p_hwfn, p_ptt, &data,
ECORE_DCBX_LLDP_TLVS);
if (rc != ECORE_SUCCESS) {
DP_NOTICE(p_hwfn, false, "Failed to read lldp TLVs\n");
return rc;
}
if (!tlvs.length)
continue;
for (i = 0; i < MAX_TLV_BUFFER; i++)
tlvs.tlvs_buffer[i] =
OSAL_CPU_TO_BE32(tlvs.tlvs_buffer[i]);
OSAL_LLDP_RX_TLVS(p_hwfn, tlvs.tlvs_buffer, tlvs.length);
}
return rc;
....
}Предупреждение PVS-Studio:
V535 The variable 'i' is being used for this loop and for the outer loop. Check lines: 1368, 1384. ecore_dcbx.c 1384
Не хочется разбираться в коде? Видимо, не хотелось и разработчикам при его проверке. Я сейчас сокращу код — подставлю значение констант:
for (i = 0; i < 3; i++) {
....
for (i = 0; i < 128; i++) { .... }
....
}Когда одна переменная используется для двух циклов, внешний цикл может стать вечным. Здесь разработчикам в каком-то смысле "повезло". А может и нет — будь цикл вечным, это бы заметили.
После окончания внутреннего цикла переменная i равна 128. Соответственно, внешний цикл после этого тоже сразу завершается, выполнив только одну итерацию.
Идентичный баг живёт здесь:
static int
check_lcore_params(void)
{
....
if ((socketid = rte_lcore_to_socket_id(lcore) != 0) &&
(numa_on == 0)) {
printf("warning: lcore %u is on socket %d with numa off\n",
lcore, socketid);
}
....
}Предупреждение PVS-Studio:
V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. main.c 1415
Программист хотел присвоить значение переменной, а затем проверить его:
((socketid = rte_lcore_to_socket_id(lcore)) != 0)Но приоритет операций таков, что вначале выполняется проверка, а затем присваивание:
(socketid = (rte_lcore_to_socket_id(lcore) != 0))Значение переменной socketid окажется испорченным. Оно всегда будет 0 или 1. Это некорректное значение может быть использовано как для формирования сообщения (printf), так и далее по коду.
Аналогичная ошибка:
static void
bnx2x_storm_stats_post(struct bnx2x_softc *sc)
{
int rc;
if (!sc->stats_pending) {
if (sc->stats_pending)
return;
sc->fw_stats_req->hdr.drv_stats_counter =
htole16(sc->stats_counter++);
....
}Это настолько странный код, что PVS-Studio выдаёт на него два предупреждения:
Что хотел разработчик? Что это такое?
if (!sc->stats_pending) {
if (sc->stats_pending)
return;Возможно, stats_pending — это что-то хитрое? Нет, это просто обыкновенная переменная:
/* tracking a pending STAT_QUERY ramrod */
uint16_t stats_pending;Понятно, что ничего непонятно. То ли опечатка, то ли недописанный код.
А где же хтонические неописуемый ужас? Он ждал меня в структуре bnx2x_softc, когда я заглянул в её пучину, чтобы посмотреть, как объявлена переменная stats_pending.
struct bnx2x_softc {
void **rx_queues;
void **tx_queues;
uint32_t max_tx_queues;
uint32_t max_rx_queues;
const struct rte_pci_device *pci_dev;
uint32_t pci_val;
struct bnx2x_pci_cap *pci_caps;
#define BNX2X_INTRS_POLL_PERIOD 1
void *firmware;
uint64_t fw_len;
/* MAC address operations */
struct bnx2x_mac_ops mac_ops;
/* structures for VF mbox/response/bulletin */
struct bnx2x_vf_mbx_msg *vf2pf_mbox;
struct bnx2x_dma vf2pf_mbox_mapping;
struct vf_acquire_resp_tlv acquire_resp;
struct bnx2x_vf_bulletin *pf2vf_bulletin;
struct bnx2x_dma pf2vf_bulletin_mapping;
struct bnx2x_vf_bulletin old_bulletin;
rte_spinlock_t vf2pf_lock;
int media;
int state; /* device state */
#define BNX2X_STATE_CLOSED 0x0000
#define BNX2X_STATE_OPENING_WAITING_LOAD 0x1000
#define BNX2X_STATE_OPENING_WAITING_PORT 0x2000
#define BNX2X_STATE_OPEN 0x3000
#define BNX2X_STATE_CLOSING_WAITING_HALT 0x4000
#define BNX2X_STATE_CLOSING_WAITING_DELETE 0x5000
#define BNX2X_STATE_CLOSING_WAITING_UNLOAD 0x6000
#define BNX2X_STATE_DISABLED 0xD000
#define BNX2X_STATE_DIAG 0xE000
#define BNX2X_STATE_ERROR 0xF000
int flags;
#define BNX2X_ONE_PORT_FLAG 0x1
#define BNX2X_NO_FCOE_FLAG 0x2
#define BNX2X_NO_WOL_FLAG 0x4
#define BNX2X_NO_MCP_FLAG 0x8
#define BNX2X_NO_ISCSI_OOO_FLAG 0x10
#define BNX2X_NO_ISCSI_FLAG 0x20
#define BNX2X_MF_FUNC_DIS 0x40
#define BNX2X_TX_SWITCHING 0x80
#define BNX2X_IS_VF_FLAG 0x100
#define BNX2X_ONE_PORT(sc) (sc->flags & BNX2X_ONE_PORT_FLAG)
#define BNX2X_NOFCOE(sc) (sc->flags & BNX2X_NO_FCOE_FLAG)
#define BNX2X_NOMCP(sc) (sc->flags & BNX2X_NO_MCP_FLAG)
#define MAX_BARS 5
struct bnx2x_bar bar[MAX_BARS]; /* map BARs 0, 2, 4 */
uint16_t doorbell_size;
/* periodic timer callout */
#define PERIODIC_STOP 0
#define PERIODIC_GO 1
volatile unsigned long periodic_flags;
rte_atomic32_t scan_fp;
struct bnx2x_fastpath fp[MAX_RSS_CHAINS];
struct bnx2x_sp_objs sp_objs[MAX_RSS_CHAINS];
uint8_t unit; /* driver instance number */
int pcie_bus; /* PCIe bus number */
int pcie_device; /* PCIe device/slot number */
int pcie_func; /* PCIe function number */
uint8_t pfunc_rel; /* function relative */
uint8_t pfunc_abs; /* function absolute */
uint8_t path_id; /* function absolute */
#define SC_PATH(sc) (sc->path_id)
#define SC_PORT(sc) (sc->pfunc_rel & 1)
#define SC_FUNC(sc) (sc->pfunc_rel)
#define SC_ABS_FUNC(sc) (sc->pfunc_abs)
#define SC_VN(sc) (sc->pfunc_rel >> 1)
#define SC_L_ID(sc) (SC_VN(sc) << 2)
#define PORT_ID(sc) SC_PORT(sc)
#define PATH_ID(sc) SC_PATH(sc)
#define VNIC_ID(sc) SC_VN(sc)
#define FUNC_ID(sc) SC_FUNC(sc)
#define ABS_FUNC_ID(sc) SC_ABS_FUNC(sc)
#define SC_FW_MB_IDX_VN(sc, vn) \
(SC_PORT(sc) + (vn) * \
((CHIP_IS_E1x(sc) || (CHIP_IS_MODE_4_PORT(sc))) ? 2 : 1))
#define SC_FW_MB_IDX(sc) SC_FW_MB_IDX_VN(sc, SC_VN(sc))
int if_capen; /* enabled interface capabilities */
struct bnx2x_devinfo devinfo;
char fw_ver_str[32];
char mf_mode_str[32];
char pci_link_str[32];
struct iro *iro_array;
int dmae_ready;
#define DMAE_READY(sc) (sc->dmae_ready)
struct ecore_credit_pool_obj vlans_pool;
struct ecore_credit_pool_obj macs_pool;
struct ecore_rx_mode_obj rx_mode_obj;
struct ecore_mcast_obj mcast_obj;
struct ecore_rss_config_obj rss_conf_obj;
struct ecore_func_sp_obj func_obj;
uint16_t fw_seq;
uint16_t fw_drv_pulse_wr_seq;
uint32_t func_stx;
struct elink_params link_params;
struct elink_vars link_vars;
uint32_t link_cnt;
struct bnx2x_link_report_data last_reported_link;
char mac_addr_str[32];
uint32_t tx_ring_size;
uint32_t rx_ring_size;
int wol;
int is_leader;
int recovery_state;
#define BNX2X_RECOVERY_DONE 1
#define BNX2X_RECOVERY_INIT 2
#define BNX2X_RECOVERY_WAIT 3
#define BNX2X_RECOVERY_FAILED 4
#define BNX2X_RECOVERY_NIC_LOADING 5
uint32_t rx_mode;
#define BNX2X_RX_MODE_NONE 0
#define BNX2X_RX_MODE_NORMAL 1
#define BNX2X_RX_MODE_ALLMULTI 2
#define BNX2X_RX_MODE_ALLMULTI_PROMISC 3
#define BNX2X_RX_MODE_PROMISC 4
#define BNX2X_MAX_MULTICAST 64
struct bnx2x_port port;
struct cmng_init cmng;
/* user configs */
uint8_t num_queues;
int hc_rx_ticks;
int hc_tx_ticks;
uint32_t rx_budget;
int interrupt_mode;
#define INTR_MODE_INTX 0
#define INTR_MODE_MSI 1
#define INTR_MODE_MSIX 2
#define INTR_MODE_SINGLE_MSIX 3
int udp_rss;
uint8_t igu_dsb_id;
uint8_t igu_base_sb;
uint8_t igu_sb_cnt;
uint32_t igu_base_addr;
uint8_t base_fw_ndsb;
#define DEF_SB_IGU_ID 16
#define DEF_SB_ID HC_SP_SB_ID
/* default status block */
struct bnx2x_dma def_sb_dma;
struct host_sp_status_block *def_sb;
uint16_t def_idx;
uint16_t def_att_idx;
uint32_t attn_state;
struct attn_route attn_group[MAX_DYNAMIC_ATTN_GRPS];
/* general SP events - stats query, cfc delete, etc */
#define HC_SP_INDEX_ETH_DEF_CONS 3
/* EQ completions */
#define HC_SP_INDEX_EQ_CONS 7
/* FCoE L2 connection completions */
#define HC_SP_INDEX_ETH_FCOE_TX_CQ_CONS 6
#define HC_SP_INDEX_ETH_FCOE_RX_CQ_CONS 4
/* iSCSI L2 */
#define HC_SP_INDEX_ETH_ISCSI_CQ_CONS 5
#define HC_SP_INDEX_ETH_ISCSI_RX_CQ_CONS 1
/* event queue */
struct bnx2x_dma eq_dma;
union event_ring_elem *eq;
uint16_t eq_prod;
uint16_t eq_cons;
uint16_t *eq_cons_sb;
#define NUM_EQ_PAGES 1 /* must be a power of 2 */
#define EQ_DESC_CNT_PAGE (BNX2X_PAGE_SIZE / sizeof(union event_ring_elem))
#define EQ_DESC_MAX_PAGE (EQ_DESC_CNT_PAGE - 1)
#define NUM_EQ_DESC (EQ_DESC_CNT_PAGE * NUM_EQ_PAGES)
#define EQ_DESC_MASK (NUM_EQ_DESC - 1)
#define MAX_EQ_AVAIL (EQ_DESC_MAX_PAGE * NUM_EQ_PAGES - 2)
/* depends on EQ_DESC_CNT_PAGE being a power of 2 */
#define NEXT_EQ_IDX(x) \
((((x) & EQ_DESC_MAX_PAGE) == (EQ_DESC_MAX_PAGE - 1)) ? \
((x) + 2) : ((x) + 1))
/* depends on the above and on NUM_EQ_PAGES being a power of 2 */
#define EQ_DESC(x) ((x) & EQ_DESC_MASK)
/* slow path */
struct bnx2x_dma sp_dma;
struct bnx2x_slowpath *sp;
uint32_t sp_state;
/* slow path queue */
struct bnx2x_dma spq_dma;
struct eth_spe *spq;
#define SP_DESC_CNT (BNX2X_PAGE_SIZE / sizeof(struct eth_spe))
#define MAX_SP_DESC_CNT (SP_DESC_CNT - 1)
#define MAX_SPQ_PENDING 8
uint16_t spq_prod_idx;
struct eth_spe *spq_prod_bd;
struct eth_spe *spq_last_bd;
uint16_t *dsb_sp_prod;
volatile unsigned long eq_spq_left; /* COMMON_xxx ramrod credit */
volatile unsigned long cq_spq_left; /* ETH_xxx ramrod credit */
/* fw decompression buffer */
struct bnx2x_dma gz_buf_dma;
void *gz_buf;
uint32_t gz_outlen;
#define GUNZIP_BUF(sc) (sc->gz_buf)
#define GUNZIP_OUTLEN(sc) (sc->gz_outlen)
#define GUNZIP_PHYS(sc) (rte_iova_t)(sc->gz_buf_dma.paddr)
#define FW_BUF_SIZE 0x40000
struct raw_op *init_ops;
uint16_t *init_ops_offsets; /* init block offsets inside init_ops */
uint32_t *init_data; /* data blob, 32 bit granularity */
uint32_t init_mode_flags;
#define INIT_MODE_FLAGS(sc) (sc->init_mode_flags)
/* PRAM blobs - raw data */
const uint8_t *tsem_int_table_data;
const uint8_t *tsem_pram_data;
const uint8_t *usem_int_table_data;
const uint8_t *usem_pram_data;
const uint8_t *xsem_int_table_data;
const uint8_t *xsem_pram_data;
const uint8_t *csem_int_table_data;
const uint8_t *csem_pram_data;
#define INIT_OPS(sc) (sc->init_ops)
#define INIT_OPS_OFFSETS(sc) (sc->init_ops_offsets)
#define INIT_DATA(sc) (sc->init_data)
#define INIT_TSEM_INT_TABLE_DATA(sc) (sc->tsem_int_table_data)
#define INIT_TSEM_PRAM_DATA(sc) (sc->tsem_pram_data)
#define INIT_USEM_INT_TABLE_DATA(sc) (sc->usem_int_table_data)
#define INIT_USEM_PRAM_DATA(sc) (sc->usem_pram_data)
#define INIT_XSEM_INT_TABLE_DATA(sc) (sc->xsem_int_table_data)
#define INIT_XSEM_PRAM_DATA(sc) (sc->xsem_pram_data)
#define INIT_CSEM_INT_TABLE_DATA(sc) (sc->csem_int_table_data)
#define INIT_CSEM_PRAM_DATA(sc) (sc->csem_pram_data)
#define PHY_FW_VER_LEN 20
char fw_ver[32];
/* ILT
* For max 196 cids (64*3 + non-eth), 32KB ILT page size and 1KB
* context size we need 8 ILT entries.
*/
#define ILT_MAX_L2_LINES 8
struct hw_context context[ILT_MAX_L2_LINES];
struct ecore_ilt *ilt;
#define ILT_MAX_LINES 256
/* max supported number of RSS queues: IGU SBs minus one for CNIC */
#define BNX2X_MAX_RSS_COUNT(sc) ((sc)->igu_sb_cnt - CNIC_SUPPORT(sc))
/* max CID count: Max RSS * Max_Tx_Multi_Cos + FCoE + iSCSI */
#define BNX2X_L2_MAX_CID(sc) \
(BNX2X_MAX_RSS_COUNT(sc) * ECORE_MULTI_TX_COS + 2 * CNIC_SUPPORT(sc))
#define BNX2X_L2_CID_COUNT(sc) \
(BNX2X_NUM_ETH_QUEUES(sc) * ECORE_MULTI_TX_COS + 2 * CNIC_SUPPORT(sc))
#define L2_ILT_LINES(sc) \
(DIV_ROUND_UP(BNX2X_L2_CID_COUNT(sc), ILT_PAGE_CIDS))
int qm_cid_count;
uint8_t dropless_fc;
/* total number of FW statistics requests */
uint8_t fw_stats_num;
/*
* This is a memory buffer that will contain both statistics ramrod
* request and data.
*/
struct bnx2x_dma fw_stats_dma;
/*
* FW statistics request shortcut (points at the beginning of fw_stats
* buffer).
*/
int fw_stats_req_size;
struct bnx2x_fw_stats_req *fw_stats_req;
rte_iova_t fw_stats_req_mapping;
/*
* FW statistics data shortcut (points at the beginning of fw_stats
* buffer + fw_stats_req_size).
*/
int fw_stats_data_size;
struct bnx2x_fw_stats_data *fw_stats_data;
rte_iova_t fw_stats_data_mapping;
/* tracking a pending STAT_QUERY ramrod */
uint16_t stats_pending;
/* number of completed statistics ramrods */
uint16_t stats_comp;
uint16_t stats_counter;
uint8_t stats_init;
int stats_state;
struct bnx2x_eth_stats eth_stats;
struct host_func_stats func_stats;
struct bnx2x_eth_stats_old eth_stats_old;
struct bnx2x_net_stats_old net_stats_old;
struct bnx2x_fw_port_stats_old fw_stats_old;
struct dmae_command stats_dmae; /* used by dmae command loader */
int executer_idx;
int mtu;
/* DCB support on/off */
int dcb_state;
#define BNX2X_DCB_STATE_OFF 0
#define BNX2X_DCB_STATE_ON 1
/* DCBX engine mode */
int dcbx_enabled;
#define BNX2X_DCBX_ENABLED_OFF 0
#define BNX2X_DCBX_ENABLED_ON_NEG_OFF 1
#define BNX2X_DCBX_ENABLED_ON_NEG_ON 2
#define BNX2X_DCBX_ENABLED_INVALID -1
uint8_t cnic_support;
uint8_t cnic_enabled;
uint8_t cnic_loaded;
#define CNIC_SUPPORT(sc) 0 /* ((sc)->cnic_support) */
#define CNIC_ENABLED(sc) 0 /* ((sc)->cnic_enabled) */
#define CNIC_LOADED(sc) 0 /* ((sc)->cnic_loaded) */
/* multiple tx classes of service */
uint8_t max_cos;
#define BNX2X_MAX_PRIORITY 8
/* priority to cos mapping */
uint8_t prio_to_cos[BNX2X_MAX_PRIORITY];
int panic;
/* Array of Multicast addrs */
struct rte_ether_addr mc_addrs[VF_MAX_MULTICAST_PER_VF];
/* Multicast mac addresses number */
uint16_t mc_addrs_num;
}; /* struct bnx2x_softc */Для DPDK, как обычно бывает и в других проектах, анализатор выдал много предупреждений V595 (указатель разыменовывается до проверки). Однако природа бага может быть разной. Начнём с редкой разновидности.
int qbman_fq_query(struct qbman_swp *s, uint32_t fqid,
struct qbman_fq_query_rslt *r)
{
struct qbman_fq_query_desc *p;
p = (struct qbman_fq_query_desc *)qbman_swp_mc_start(s);
if (!p)
return -EBUSY;
p->fqid = fqid;
*r = *(struct qbman_fq_query_rslt *)qbman_swp_mc_complete(s, p,
QBMAN_FQ_QUERY);
if (!r) {
pr_err("qbman: Query FQID %d failed, no response\n",
fqid);
return -EIO;
}
....
}Предупреждение PVS-Studio:
V595 The 'r' pointer was utilized before it was verified against nullptr. Check lines: 211, 213. qbman_debug.c 211
Изучим внимательнее эти строки:
*r = *(struct qbman_fq_query_rslt *)
qbman_swp_mc_complete(s, p, QBMAN_FQ_QUERY);
if (!r) {Функция возвращает указатель на структуру. Значение этой структуры сразу копируется по некому указателю r. Затем сделана попытка проверить, не вернула ли функция NULL, но сделано всё неправильно:
Чтобы исправить ситуацию, следует воспользоваться промежуточным указателем:
p->fqid = fqid;
qbman_fq_query_rslt *tmp =
(struct qbman_fq_query_rslt *)
qbman_swp_mc_complete(s, p, QBMAN_FQ_QUERY);
if (tmp) {
pr_err("qbman: Query FQID %d failed, no response\n",
fqid);
return -EIO;
}
*r = *tmp;int
roc_bphy_cgx_set_link_mode(struct roc_bphy_cgx *roc_cgx, unsigned int lmac,
struct roc_bphy_cgx_link_mode *mode)
{
uint64_t scr1, scr0;
if (roc_model_is_cn9k() &&
(mode->use_portm_idx || mode->portm_idx || mode->mode_group_idx)) { // <=
return -ENOTSUP;
}
if (!roc_cgx)
return -EINVAL;
if (!roc_bphy_cgx_lmac_exists(roc_cgx, lmac))
return -ENODEV;
if (!mode) // <=
return -EINVAL;
....
}Предупреждение PVS-Studio:
V595 The 'mode' pointer was utilized before it was verified against nullptr. Check lines: 373, 383. roc_bphy_cgx.c 373
В начале указатель mode разыменовывается. А затем вдруг выясняется, что вообще-то он может быть нулевой, и его надо проверить. Но уже поздно.
Этот случай похож на предыдущий, но здесь указатель проверяется ещё более неумело. В каком-то смысле тут даже не одна, а две ошибки, поэтому PVS-Studio выдаёт два предупреждения.
static int
eth_dev_close(struct rte_eth_dev *eth_dev)
{
struct pmd_internals *internals =
(struct pmd_internals *)eth_dev->data->dev_private;
struct drv_s *p_drv = internals->p_drv;
internals->p_drv = NULL;
rte_eth_dev_release_port(eth_dev);
/* decrease initialized ethernet devices */
p_drv->n_eth_dev_init_count--;
/*
* rte_pci_dev has no private member for p_drv
* wait until all rte_eth_dev's are closed - then close adapters via p_drv
*/
if (!p_drv->n_eth_dev_init_count && p_drv)
drv_deinit(p_drv);
return 0;
}Во-первых, указатель p_drv в начале функции смело используется, а затем проверяется:
p_drv->n_eth_dev_init_count--;
if (.... && p_drv)Первое предупреждение:
V595 The 'p_drv' pointer was utilized before it was verified against nullptr. Check lines: 389, 395. ntnic_ethdev.c 389
Во-вторых, даже в условии, где проверяется указатель, умудрились накосячить:
if (!p_drv->n_eth_dev_init_count && p_drv)Опять сначала разыменование, затем проверка:
V713 The pointer 'p_drv' was utilized in the logical expression before it was verified against nullptr in the same logical expression. ntnic_ethdev.c 395
struct __rte_cache_aligned vhost_crypto_info {
....
uint32_t nb_inflight_ops;
....
};
static void
destroy_device(int vid)
{
struct vhost_crypto_info *info = NULL;
....
info = options.infos[i];
....
do {
} while (info->nb_inflight_ops);
info->initialized[j] = 0;
....
}Предупреждение PVS-Studio:
V712 Be advised that compiler may delete this cycle or make it infinity. Use volatile variable(s) or synchronization primitives to avoid this. main.c 353
Плохая идея — организовывать синхронизацию, используя обыкновенную переменную типа uint32_t. Такой код может работать, а может и не работать. Например, в целях оптимизации компилятор может один раз проверить переменную, и, если она не равна нулю, организовать вечный цикл.
static inline int
vrb_dma_desc_ld_fill(struct rte_bbdev_dec_op *op, ....)
{
....
if (op->ldpc_dec.harq_combined_input.data == 0) {
rte_bbdev_log(ERR, "HARQ input is not defined");
return -1;
}
h_p_size = fcw->hcin_size0 + fcw->hcin_size1;
if (fcw->hcin_decomp_mode == 1)
h_p_size = (h_p_size * 3 + 3) / 4;
else if (fcw->hcin_decomp_mode == 4)
h_p_size = h_p_size / 2;
if (op->ldpc_dec.harq_combined_input.data == 0) {
rte_bbdev_log(ERR, "HARQ input is not defined");
return -1;
}
....
}Предупреждение PVS-Studio:
V547 Expression is always false. rte_vrb_pmd.c 1861
В принципе, нестрашная штука. Просто два раза дублируется этот фрагмент кода:
if (op->ldpc_dec.harq_combined_input.data == 0) {
rte_bbdev_log(ERR, "HARQ input is not defined");
return -1;
}При этом между фрагментами значение проверяемой переменной не меняется. Так что один из блоков можно просто удалить. Впрочем, это только в том случае, если на самом деле в одном из них не хотели проверить что-то ещё, но опечатались.
Аналогичное:
static void
vmxnet3_dev_tx_queue_reset(void *txq)
{
vmxnet3_tx_queue_t *tq = txq;
....
if (tq != NULL) {
/* Release the cmd_ring mbufs */
vmxnet3_tx_cmd_ring_release_mbufs(&tq->cmd_ring);
}
....
size += tq->txdata_desc_size * data_ring->size;
....
}Предупреждение PVS-Studio:
V1004 The 'tq' pointer was used unsafely after it was verified against nullptr. Check lines: 213, 227. vmxnet3_rxtx.c 227
Указатель tq используется после предварительной проверки tq != NULL. Значит, он может быть нулевым. Но далее в коде про это забыли, и он используется tq->txdata_desc_size уже без проверки.
#define RTE_ETH_SPEED_NUM_2_5G 2500 /**< 2.5 Gbps */
#define RTE_ETH_SPEED_NUM_5G 5000 /**< 5 Gbps */
static uint32_t
tap_dev_speed_capa(void)
{
uint32_t speed = pmd_link.link_speed;
uint32_t capa = 0;
if (speed >= RTE_ETH_SPEED_NUM_10M)
capa |= RTE_ETH_LINK_SPEED_10M;
if (speed >= RTE_ETH_SPEED_NUM_100M)
capa |= RTE_ETH_LINK_SPEED_100M;
if (speed >= RTE_ETH_SPEED_NUM_1G)
capa |= RTE_ETH_LINK_SPEED_1G;
if (speed >= RTE_ETH_SPEED_NUM_5G)
capa |= RTE_ETH_LINK_SPEED_2_5G;
if (speed >= RTE_ETH_SPEED_NUM_5G)
capa |= RTE_ETH_LINK_SPEED_5G;
if (speed >= RTE_ETH_SPEED_NUM_10G)
capa |= RTE_ETH_LINK_SPEED_10G;
if (speed >= RTE_ETH_SPEED_NUM_20G)
capa |= RTE_ETH_LINK_SPEED_20G;
if (speed >= RTE_ETH_SPEED_NUM_25G)
capa |= RTE_ETH_LINK_SPEED_25G;
if (speed >= RTE_ETH_SPEED_NUM_40G)
capa |= RTE_ETH_LINK_SPEED_40G;
if (speed >= RTE_ETH_SPEED_NUM_50G)
capa |= RTE_ETH_LINK_SPEED_50G;
if (speed >= RTE_ETH_SPEED_NUM_56G)
capa |= RTE_ETH_LINK_SPEED_56G;
if (speed >= RTE_ETH_SPEED_NUM_100G)
capa |= RTE_ETH_LINK_SPEED_100G;
return capa;
}Предупреждение PVS-Studio:
V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 997, 999. rte_eth_tap.c 999
Такой код тяжело проверять при обзоре кода. Как результат — опечатка:
if (speed >= RTE_ETH_SPEED_NUM_5G)
capa |= RTE_ETH_LINK_SPEED_2_5G;
if (speed >= RTE_ETH_SPEED_NUM_5G)
capa |= RTE_ETH_LINK_SPEED_5G;Два раза происходит сравнение с одной и той же константой, но выставляются разные битовые флаги. Скорее всего, в первом условии задумывалось использовать константу RTE_ETH_SPEED_NUM_2_5G:
if (speed >= RTE_ETH_SPEED_NUM_2_5G)
capa |= RTE_ETH_LINK_SPEED_2_5G;
if (speed >= RTE_ETH_SPEED_NUM_5G)
capa |= RTE_ETH_LINK_SPEED_5G;Начнём с безобидного случая.
static int bnxt_rss_hash_update_op(struct rte_eth_dev *eth_dev,
struct rte_eth_rss_conf *rss_conf)
{
....
/* Cache the hash function */
bp->rss_conf.rss_hf = rss_conf->rss_hf;
/* Cache the hash function */
bp->rss_conf.rss_hf = rss_conf->rss_hf;
....
}Предупреждение PVS-Studio:
V519 The 'bp->rss_conf.rss_hf' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2322, 2325. bnxt_ethdev.c 2325
Одинаковые присваивания, одно из которых можно просто удалить. А теперь поинтереснее.
uint16_t
cnxk_eswitch_dev_tx_burst(....)
{
....
uint64_t aura_handle, data = 0;
....
for (pkt = 0; pkt < nb_tx; pkt++) {
....
data &= ~0x7ULL;
/*<15:12> = CNTM1: Count minus one of LMTSTs in the burst */
data = (0ULL << 12);
/* *<10:0> = LMT_ID:
Identifies which LMT line is used for the first LMTST
*/
data |= (uint64_t)lmt_id;
....
}
....
}Предупреждение PVS-Studio:
V519 The 'data' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 211, 213. cnxk_eswitch_rxtx.c 213
Хмм...
data &= ~0x7ULL;
data = (0ULL << 12);Не знаю, что здесь должно быть написано, но сейчас явно фигня какая-то.
Продолжим.
static int
enetfec_tx_queue_setup(struct rte_eth_dev *dev,
uint16_t queue_idx,
uint16_t nb_desc,
unsigned int socket_id __rte_unused,
const struct rte_eth_txconf *tx_conf)
{
....
struct bufdesc *bdp, *bd_base;
....
bdp = txq->bd.base;
bdp = txq->bd.cur;
....
}Предупреждение PVS-Studio:
V519 The 'bdp' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 417, 418. enet_ethdev.c 418
Здесь опечатка, и, наверное, должно быть написано:
bd_base = txq->bd.base;
bdp = txq->bd.cur;В следующем коде, наверное, забыли else.
static void ice_fill_sw_info(struct ice_hw *hw, struct ice_fltr_info *fi)
{
if ((fi->flag & ICE_FLTR_RX) &&
(fi->fltr_act == ICE_FWD_TO_VSI ||
fi->fltr_act == ICE_FWD_TO_VSI_LIST) &&
fi->lkup_type == ICE_SW_LKUP_LAST)
fi->lan_en = true;
fi->lb_en = false;
fi->lan_en = false;
....
}Предупреждение PVS-Studio:
V519 The 'fi->lan_en' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 3930, 3932. ice_switch.c 3932
Думаю, должно быть так (но не очень уверен):
if ((fi->flag & ICE_FLTR_RX) &&
(fi->fltr_act == ICE_FWD_TO_VSI ||
fi->fltr_act == ICE_FWD_TO_VSI_LIST) &&
fi->lkup_type == ICE_SW_LKUP_LAST)
fi->lan_en = true;
else
fi->lb_en = false;Подозреваю, что следующий баг возник из-за неудачного рефакторинга или слияния веток.
enum _ecore_status_t ecore_mcp_fill_shmem_func_info(....)
{
....
info->mtu = (u16)shmem_info.mtu_size;
if (info->mtu == 0)
info->mtu = 1500;
info->mtu = (u16)shmem_info.mtu_size;
....
}Предупреждение PVS-Studio:
V519 The 'info->mtu' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2607, 2609. ecore_mcp.c 2609
Последнее присваивание явно лишнее и всё ломает.
Анализатор выдал пачку предупреждений V1020 о том, что некий ресурс может остаться заблокированным. Диагностика не очень точная, да ещё дело осложняется вложенными макросами. Я заблудился в макросах DPDK и затрудняюсь сказать, где диагностика по делу, а где ложное срабатывание. Думаю, всё-таки большинство предупреждений мимо, и про них я упоминать не буду. Возможно, позже посмотрю повнимательнее препроцессированные файлы и выпишу идеи для доработки диагностического правила.
Однако там, где попроще и макросы не столь глубоки, код кажется мне крайне подозрительным. Давайте посмотрим на эти простые случаи.
В начале немного макросов, чтобы была понятна проблема, и откуда в предупреждении анализатора появилась функция rte_spinlock_unlock.
#define spin_lock(x) rte_spinlock_lock(x)
#define spin_unlock(x) rte_spinlock_unlock(x)
#define FQLOCK(fq) \
do { \
struct qman_fq *__fq478 = (fq); \
if (fq_isset(__fq478, QMAN_FQ_FLAG_LOCKED)) \
spin_lock(&__fq478->fqlock); \
} while (0)
#define FQUNLOCK(fq) \
do { \
struct qman_fq *__fq478 = (fq); \
if (fq_isset(__fq478, QMAN_FQ_FLAG_LOCKED)) \
spin_unlock(&__fq478->fqlock); \
} while (0)А теперь сам проблемный код.
int qman_set_vdq(struct qman_fq *fq, u16 num, uint32_t vdqcr_flags)
{
....
if (!p->vdqcr_owned) {
FQLOCK(fq);
if (fq_isset(fq, QMAN_FQ_STATE_VDQCR))
goto escape;
fq_set(fq, QMAN_FQ_STATE_VDQCR);
FQUNLOCK(fq);
p->vdqcr_owned = fq;
ret = 0;
}
escape:
if (!ret)
qm_dqrr_vdqcr_set(&p->p, vdqcr);
out:
return ret;
}Предупреждение PVS-Studio:
V1020 The function exited without calling the 'rte_spinlock_unlock' function. Check lines: 2149, 2136. qman.c 2149
Если функция fq_isset вернёт статус ошибки, то произойдёт переход к метке escape с последующим выходом из функции. При этом очень подозрительно, что не используется макрос FQUNLOCK. Мне кажется, что должно быть написано так:
FQLOCK(fq);
if (fq_isset(fq, QMAN_FQ_STATE_VDQCR))
{
FQUNLOCK(fq);
goto escape;
}
fq_set(fq, QMAN_FQ_STATE_VDQCR);
FQUNLOCK(fq);Рассмотрим ещё один случай.
static __rte_noinline int
l2fwd_get_free_event_port(struct l2fwd_event_resources *evt_rsrc)
{
static int index;
int port_id;
rte_spinlock_lock(&evt_rsrc->evp.lock);
if (index >= evt_rsrc->evp.nb_ports) {
printf("No free event port is available\n");
return -1;
}
port_id = evt_rsrc->evp.event_p_id[index];
index++;
rte_spinlock_unlock(&evt_rsrc->evp.lock);
return port_id;
}Предупреждение PVS-Studio:
V1020 The function exited without calling the 'rte_spinlock_unlock' function. Check lines: 144, 141. l2fwd_event.c 144
Кажется, в обработчике ошибок опять забыли сделать unlock. Предполагаю, что должно быть так:
rte_spinlock_lock(&evt_rsrc->evp.lock);
if (index >= evt_rsrc->evp.nb_ports) {
rte_spinlock_unlock(&evt_rsrc->evp.lock);
printf("No free event port is available\n");
return -1;
}Вначале изучим, как работает функция roc_cpt_to_cpt_priv.
struct roc_cpt {
struct plt_pci_device *pci_dev;
struct roc_cpt_lf *lf[ROC_CPT_MAX_LFS];
uint16_t nb_lf;
uint16_t nb_lf_avail;
uintptr_t lmt_base;
/**< CPT device capabilities */
union cpt_eng_caps hw_caps[CPT_MAX_ENG_TYPES];
uint8_t eng_grp[CPT_MAX_ENG_TYPES];
uint8_t cpt_revision;
void *opaque;
#define ROC_CPT_MEM_SZ (6 * 1024)
uint8_t reserved[ROC_CPT_MEM_SZ] __plt_cache_aligned;
} __plt_cache_aligned;
static inline struct cpt *
roc_cpt_to_cpt_priv(struct roc_cpt *roc_cpt)
{
return (struct cpt *)&roc_cpt->reserved[0];
}Если указатель roc_cpt окажется нулевым, функция вернёт невалидный адрес, физически равный смещению массива относительно начала структуры. Использовать этот адрес нельзя. Единственное, что можно про него сказать, он будет ненулевой.
На самом деле мы вообще сейчас сталкиваемся с темой неопределённого поведения при работе с нулевыми указателями. И нельзя сказать, как и что будет работать. Однако для простоты можем пока так считать, чтобы продолжить повествование.
Посмотрим, как используется вернувшийся указатель.
int
roc_cpt_dev_fini(struct roc_cpt *roc_cpt)
{
struct cpt *cpt = roc_cpt_to_cpt_priv(roc_cpt);
if (cpt == NULL)
return -EINVAL;
....
return dev_fini(&cpt->dev, cpt->pci_dev);
}Предупреждение PVS-Studio:
V547 Expression 'cpt == NULL' is always false. roc_cpt.c 970
Происходит попытка проверить указатель на равенство NULL, после чего этот указатель будет использоваться для разных целей. Если roc_cpt окажется равен NULL, то возникает неопределённое поведение. Проверка указателя ни от чего не защищает.
Думаю, следует доработать функцию roc_cpt_to_cpt_priv следующим образом:
static inline struct cpt *
roc_cpt_to_cpt_priv(struct roc_cpt *roc_cpt)
{
if (roc_cpt == NULL)
return NULL;
return (struct cpt *)&roc_cpt->reserved[0];
}Есть. Тем не менее надо где-то остановиться, и 100 выглядит красивым числом в названии статьи. А ещё я устал.
Разработчики и все желающие могут воспользоваться бесплатной триальной версией PVS-Studio, чтобы проверить проект и изучить отчёт более тщательно. Также напоминаю, что у нас есть вариант бесплатного лицензирования PVS-Studio для открытых проектов.
Найти реальные ошибки в коде — лучший способ популяризации методологии статического анализа кода. Однако надо понимать, что хотя это и хорошая демонстрация, но это непродуктивный способ использования инструмента. Анализаторы должны использоваться регулярно в процессе разработки, чтобы выявлять ошибки на самом раннем этапе, делая их исправление быстрым и дешёвым. Внедряйте статический анализ в процесс, а не ищите с его помощью баги.
В любом случае 100 багов лучше 1000 слов о пользе статического анализа кода. Попробуйте анализатор PVS-Studio.
0
0
0
55
0
64
1
95
1
0