Тестуємо Linux-версію PVS-Studio на Linux Kernel



З моменту виходу публічної Linux-версії PVS-Studio, стаття про повторній перевірці ядра Linux залишалася лише питанням часу. Проект, який пишеться професіоналами з усього світу, який використовують велику кількість людей в самих різних сферах, який регулярно перевіряється і тестується різними інструментами — перевірка такого проекту буде серйозним випробуванням для будь-якого статичного аналізатора. Які помилки вдалося знайти PVS-Studio в таких умовах?



Як перевіряли
Ми вже перевіряли ядро Linux. З тих пір багато чого змінилося — тепер перевірка ОС так само проста і зручна, як перевірка будь-якого іншого проекту:
pvs-studio-analyzer trace - make
pvs-studio-analyzer analyze -o /path/to/report.log -j8

Всього кілька місяців пішло на адаптацію і тестування аналізатора в Linux, який до недавнього часу був доступний тільки для Windows. В цей раз перевірити ядро виявилося на порядок простіше.

Для перевірки була взята версія 4.9-rc4 (commit bc33b0ca11e3df467777a4fa7639ba488c9d4911).

Для написання статті були помітні тільки діагностики загального призначення 1-2 рівнів. Варто відзначити високу якість коду — щільність сигналів, що вказують на реальні недоробки, вкрай низька.

Для статті я вибирав ті попередження, які з найбільшою ймовірністю вказують на справжні помилки/друкарські помилки. Треба розуміти, що крім корисних попереджень аналізатор видає і помилкові спрацьовування або виявляє погано оформлений або «погано пахне» код, який тим не менш не є по-справжньому помилковим.

На жаль, кількість помилкових спрацьовувань під Linux більше, ніж хотілося б. Думаю, це пов'язано з молодістю версії аналізатора, призначеної для Linux систем. Ми зробили і продовжуємо робити величезну роботу з мінімізації кількості помилкових спрацьовувань. Код Linux допоміг нам стати краще і внести безліч корисних змін в аналізатор, і тепер хотілося б відповісти взаємністю.

Помилки
Найпоширеніша категорія помилок пов'язана з звичайними помилками і Copy-Paste. Якщо ви читали наші статті раніше, я думаю, ви вже в цьому переконалися. Вони з'являються в будь-яких проектах на будь-яких операційних системах на будь-яких мовах. В таких помилках і розкривається потенціал статичного аналізатора: іншими інструментами знайти їх значно складніше. Подивимося, як з ними справи в ядрі Linux:

Попередження PVS-Studio: V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 2384, 2390. debug.c 2390
int dbg_check_nondata_nodes_order (....)
{
....
sa = container_of(cur, struct ubifs_scan_node, list);
sb = container_of(cur- > next, struct ubifs_scan_node, list);

if (sa->type != UBIFS_INO_NODE && sa->type != UBIFS_DENT_NODE &&
sa->type != UBIFS_XENT_NODE) {
ubifs_err(c, "bad node type %d", sa->type);
ubifs_dump_node(c, sa->node);
return -EINVAL;
}
if (sa->type != UBIFS_INO_NODE && sa->type != UBIFS_DENT_NODE &&
sa->type != UBIFS_XENT_NODE) {
ubifs_err(c, "bad node type %d", sb->type);
ubifs_dump_node(c, sb->node);
return -EINVAL;
}
....
}

Аналізатор скаржиться на два однакових умови поспіль: мабуть, у другому забули поміняти sa sb. Ну і хто після цього скаже, що в крутих проектах не копіпастять?

Попередження PVS-Studio: V666 Consider inspecting third argument of the function 'strncmp'. It is possible that the value does not correspond with the length of a string which was passed with the first argument. spectral.c 341
static ssize_t write_file_spec_scan_ctl(struct file *file,
const char __user *user_buf,
size_t count, loff_t *ppos)
{
struct ath10k *ar = file->private_data;
char buf[32];
ssize_t len;
int res;

len = min(count, sizeof(buf) - 1);
if (copy_from_user(buf, user_buf, len))
return -EFAULT;

buf[len] = '\0';

mutex_lock(&ar->conf_mutex);

if (strncmp("trigger", buf, 7) == 0) {
....
} else if (strncmp("background", buf, 9) == 0) {
res = ath10k_spectral_scan_config(ar, SPECTRAL_BACKGROUND);
} else if (strncmp("manual", buf, 6) == 0) {
res = ath10k_spectral_scan_config(ar, SPECTRAL_MANUAL);
} else if (strncmp("disable", buf, 7) == 0) {
res = ath10k_spectral_scan_config(ar, SPECTRAL_DISABLED);
} else {
res = -EINVAL;
}

mutex_unlock(&ar->conf_mutex);

if (res < 0)
return res;

return count;
}

Класичний вид помилки: в функцію потрібно передати два аргументи: покажчик на рядок і її довжину. Часто, коли аргументом служить літерал, довжину вважати лінуються і пишуть просто число. У справу вступає людський фактор: помиляються часто.

Дивіться, в коді є кілька поспіль strncmp. У кожен з них передають літерал. І в strncmp(«background», buf, 9) довжину невірно розрахували: слово «background» складається з 10, а не з 9 символів.

Попередження PVS-Studio: V666 Consider inspecting third argument of the function 'memcpy'. It is possible that the value does not correspond with the length of a string which was passed with the second argument. dpt_i2o.c 403
static void adpt_inquiry(adpt_hba* pHba)
{
....
memset(pHba->detail, 0, sizeof(pHba->detail));
memcpy(&(pHba->detail), "Vendor: Adaptec ", 16);
memcpy(&(pHba->detail[16]), " Model: ", 8);
memcpy(&(pHba->detail[24]), (u8*) &buf[16], 16);
memcpy(&(pHba->detail[40]), " FW: ", 4); // <=
memcpy(&(pHba->detail[44]), (u8*) &buf[32], 4);
pHba->detail[48] = '\0'; /* precautionary */
....
}

Ще один приклад. Довжина рядка " FW: " дорівнює 5, а не 4 символів.

Як позбутися від такої помилки? В C можна використовувати макрос кшталт:
#define str_len(S) (sizeof(S) / sizeof((S)[0]))

Але використання таких макросів саме по собі небезпечно: краще звичайно додати compiler-specific перевірок на те, що переданий аргумент дійсно масив.

Для читачів, які пишуть на C++, можу порекомендувати std::string_view, який нарешті з'явився у C++17. Краще не передавати у функцію рядка парою покажчик-довжина. Але якщо потрібно вручну порахувати розмір масиву (наприклад, щоб передати його у функцію memcpy), то можна використовувати std::size(array) чи його аналог: для літералів розмір буде порахований в compile-time.

Уникайте повторення коду і не лінуйтеся використовувати засоби мови (макроси або шаблони) для compile time обчислень!

Попередження PVS-Studio: V653 A suspicious string consisting of two parts is used for array initialization. It is possible that a comma is missing. Consider inspecting this literal: «30min» «No timeout». lp8788-charger.c 657
static ssize_t lp8788_show_eoc_time(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct lp8788_charger *pchg = dev_get_drvdata(dev);
char *stime[] = { "400ms", "5min", "10min", "15min",
"20min", "25min", "30min" "No timeout" };
....
}

Як відомо, два записаних поспіль літерала конкатенируются. Це дозволяє їх зручно використовувати, наприклад, в макросах. Небезпека виникає, коли ми пишемо масив з таких символів: можна пропустити кому і отримати несподіваний результат.

В даному випадку «злипнуться» два останніх літерала і вийде «30minNo timeout». Це подвійна помилка. По-перше, неправильний текст, по-друге, у масиві не вистачатиме одного елемента, що може привести до виходу за кордон масиву.

Раджу використовувати інший спосіб форматування, у ньому така помилка стане помітною:
char *stime[] = {
"400ms"
, "5min"
, "10min"
, "15min"
, "20min"
, "25min"
, "30min"
"No timeout"
};

Детальніше про такому способі табличного оформлення коду писав мій колега Андрій Карпов. Пропоную ознайомитися з головою N13 з його невеликий книги.

Попередження PVS-Studio: V764 Possible incorrect order of arguments passed to 'ahc_9005_subdevinfo_valid' function: 'device' and 'vendor'. aic7xxx_pci.c 695
const struct ahc_pci_identity *
ahc_find_pci_device(ahc_dev_softc_t pci)
{
....
if (ahc_get_pci_function(pci) > 0
&& ahc_9005_subdevinfo_valid(device, vendor, // <=
subdevice, subvendor)
&& SUBID_9005_MFUNCENB(subdevice) == 0)
return NULL);

....
}

Іноді буває складно зрозуміти, на що лається аналізатор. До речі, таке часто буває: чоловік не зрозумів, що йому написав аналізатор, відправив нам звіт з «помилковим спрацьовуванням», а там насправді помилка. Ось і мені тут здалося, що це помилкове спрацьовування: функція визначена трохи вище за кодом і там всі параметри на своїх місцях. Ось як вона виглядає:
static int
ahc_9005_subdevinfo_valid(uint16_t device, uint16_t vendor,
uint16_t subdevice, uint16_t subvendor)
{
....
}

У чому ж справа? Виявляється, що ще вище є оголошення цієї функції і ось там-то ці аргументи і переплутані. За фактом нічого страшного в логіці програми немає, але краще все-таки поправити, щоб нікого не бентежити і не збивати з пантелику.
static int ahc_9005_subdevinfo_valid(uint16_t vendor, uint16_t device,
uint16_t subvendor, uint16_t subdevice);



Але що найцікавіше, помилка тут вже була: параметри дійсно були переплутані, просто забули поправити оголошення. Добре, що аналізатор теж знайшов це місце.

Попередження PVS-Studio: V549 The first argument of 'memcpy' function is equal to the second argument. wilc_wfi_cfgoperations.c 1345
static int del_pmksa(struct wiphy *wiphy,
struct net_device *netdev,
struct cfg80211_pmksa *pmksa)
{
....
for (; i < (priv->pmkid_list.numpmkid - 1); i++) {
memcpy(priv->pmkid_list.pmkidlist[i].bssid,
priv->pmkid_list.pmkidlist[i + 1].bssid,
ETH_ALEN);
memcpy(priv->pmkid_list.pmkidlist[i].pmkid,
priv->pmkid_list.pmkidlist[i].pmkid,
PMKID_LEN);
}
....
}

В останньому memcpy збігаються покажчики. Можливо, хотіли написати за аналогією з попереднім виразом:
memcpy(priv->pmkid_list.pmkidlist[i].pmkid,
priv->pmkid_list.pmkidlist[i + 1].pmkid,
PMKID_LEN);

Невикористовувані змінні
Попередження PVS-Studio: V575 The 'strncasecmp' function processes '0' elements. Inspect the third argument. linux_wlan.c 1121
static int mac_ioctl(struct net_device *ndev,
struct ifreq *req,
int cmd)
{
u8 *buff = NULL;
s8 rssi;
u32 size = 0, length = 0;
struct wilc_vif *vif;
s32 ret = 0;
struct wilc *wilc;

vif = netdev_priv(ndev);
wilc = vif->wilc;

if (!wilc->initialized)
return 0;

switch (cmd) {
case SIOCSIWPRIV:
{
struct iwreq *примітка = (struct iwreq *)req;

size = примітка->u.data.length;

if (size && примітка->u.data.pointer) {
buff = memdup_user(примітка->u.data.pointer,
примітка->u.data.length);
if (IS_ERR(buff))
return PTR_ERR(buff);

if (strncasecmp(buff, "RSSI", length) == 0) { // <=
....
}
}
}
....
}

done:

kfree(buff);

return ret;
}

У функцію strncasecmp в якості аргументу довжини передали 0. У коді немає місця, де б змінювалася мінлива length, тому її значення залишиться нулем. Напевно, потрібно було використовувати size.

Попередження PVS-Studio: V751 Parameter 'LCDheight' is not used inside function body. init.c 339
static
unsigned short
SiS_GetModeID(int VGAEngine, unsigned int VBFlags,
int HDisplay, int VDisplay,
int Depth, bool FSTN,
int LCDwidth, int LCDheight)
{
unsigned short ModeIndex = 0;

switch(HDisplay)
{
case 320:
if(VDisplay == 200) ModeIndex = ModeIndex_320x200[Depth];
else if(VDisplay == 240) {
if((VBFlags & CRT2_LCD) && (FSTN))
ModeIndex = ModeIndex_320x240_FSTN[Depth];
else
ModeIndex = ModeIndex_320x240[Depth];
}
break;
case 400:
if((!(VBFlags & CRT1_LCDA)) ||
((LCDwidth >= 800) && (LCDwidth >= 600))) { // <=
if(VDisplay == 300) ModeIndex = ModeIndex_400x300[Depth];
}
break;
case 512:
if((!(VBFlags & CRT1_LCDA)) ||
((LCDwidth >= 1024) && (LCDwidth >= 768))) { // <=
if(VDisplay == 384) ModeIndex = ModeIndex_512x384[Depth];
}
break;
....
}

return ModeIndex;
}

Не завжди використовуються в функції параметри — це помилка. У досить старих API виникають ситуації, коли параметр став не потрібен і його заміняють чи просто не використовують. Але придивіться уважніше до цього фрагменту: тут забули порівняти висоту. Замість цього з'явилися порівняння виду '(A > 5) && (A > 3)', які самі по собі надлишкові.

Плутанина в пріоритетах операцій
Попередження PVS-Studio: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower than the priority '|' operator. core.c 1046
static int nvme_pr_preempt(struct block_device *bdev,
u64 old, u64 new,
enum pr_type type, bool abort)
{
u32 cdw10 = nvme_pr_type(type) << 8 | abort ? 2 : 1;
return nvme_pr_command(bdev, cdw10, old, new,
nvme_cmd_resv_acquire);
}

Тернарний оператор C — це дуже небезпечний оператор. Не просто так одна з перших діагностик загального призначення в 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: 375, 377. trx.c 375
bool rtl92ee_rx_query_desc(struct ieee80211_hw *hw,
struct rtl_stats *status,
struct ieee80211_rx_status *rx_status,
u8 *pdesc, struct sk_buff *skb)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rx_fwinfo *p_drvinfo;
struct ieee80211_hdr *hdr;
u32 phystatus = GET_RX_DESC_PHYST(pdesc);

....

status->macid = GET_RX_DESC_MACID(pdesc);
if (GET_RX_STATUS_DESC_MAGIC_MATCH(pdesc))
status->wake_match = BIT(2);
else if (GET_RX_STATUS_DESC_MAGIC_MATCH(pdesc))
status->wake_match = BIT(1);
else if (GET_RX_STATUS_DESC_UNICAST_MATCH(pdesc))
status->wake_match = BIT(0);
else
status->wake_match = 0;

....
}

З боку складно зрозуміти, що тут не так. Два рази йде одна і та ж перевірка макросом GET_RX_STATUS_DESC_MAGIC_MATCH. Якщо подивимося його оголошення, то побачимо два інших макросу:
#define GET_RX_STATUS_DESC_PATTERN_MATCH(__pdesc)
LE_BITS_TO_4BYTE(__pdesc+12, 29, 1)
#define GET_RX_STATUS_DESC_UNICAST_MATCH(__pdesc)
LE_BITS_TO_4BYTE(__pdesc+12, 30, 1)
#define GET_RX_STATUS_DESC_MAGIC_MATCH(__pdesc)
LE_BITS_TO_4BYTE(__pdesc+12, 31, 1)

Можливо хотіли використовувати відсутній у вихідному фрагменті GET_RX_STATUS_DESC_PATTERN_MATCH. Інакше ця перевірка просто безглузда.

Попередження PVS-Studio: V547 Expression '(ptr[3] & 0x1E) != 0x03' is always true. sd.c 4115
int ext_sd_send_cmd_get_rsp(struct rtsx_chip *chip,
u8 cmd_idx, u32 arg, u8 rsp_type,
u8 *rsp, int rsp_len, bool special_check)
{
int retval;
int timeout = 100;
u16 reg_addr;
u8 *ptr;

....

if (cmd_idx == SELECT_CARD) {
if (rsp_type == SD_RSP_TYPE_R2) {
if ((ptr[3] & 0x1E) != 0x04) {
rtsx_trace(chip);
return STATUS_FAIL;
}

} else if (rsp_type == SD_RSP_TYPE_R0) {
if ((ptr[3] & 0x1E) != 0x03) { // <=
rtsx_trace(chip);
return STATUS_FAIL;
}
}
}

....
}

Помилка пов'язана з бітовими операціями. Результат побітовою кон'юнкції 0x1E з-за одного біта ніколи не буде дорівнює значенню 0x03:



Попередження PVS-Studio: V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 1277, 1282. ks_wlan_net.c 1277
static int ks_wlan_set_power(struct net_device *dev,
struct iw_request_info *info,
struct iw_param *vwrq, char *extra)
{
struct ks_wlan_private *priv =
(struct ks_wlan_private *)netdev_priv(dev);
short enabled;

if (priv->sleep_mode == SLP_SLEEP) {
return -EPERM;
}
/* for SLEEP MODE */
enabled = vwrq->disabled ? 0 : 1;
if (enabled == 0) { /* 0 */
priv->reg.powermgt = POWMGT_ACTIVE_MODE;
} else if (enabled) { /* 1 */
if (priv->reg.operation_mode == MODE_INFRASTRUCTURE)
priv->reg.powermgt = POWMGT_SAVE1_MODE;
else
return -EINVAL;
} else if (enabled) { /* 2 */
if (priv->reg.operation_mode == MODE_INFRASTRUCTURE)
priv->reg.powermgt = POWMGT_SAVE2_MODE;
else
return -EINVAL;
} else
return -EINVAL;

hostif_sme_enqueue(priv, SME_POW_MNGMT_REQUEST);

return 0;
}

Скоротимо приклад:
enabled = vwrq->disabled ? 0 : 1;
if (enabled == 0) { /* 0 */
....
} else if (enabled) { /* 1 */
....
} else if (enabled) { /* 2 */
....
} else
....

Даний код виглядає дуже дивно. Начебто область значень чітко обговорена виразом вище: enabled дорівнює 0 або 1. Але перевіряється цілих 4 значення. При цьому коментарі тільки заважають: якщо цифри повинні були позначати можливе значення змінної, то зараз вони не відповідають дійсності: перевірка на 1 2 записані однаковим чином.

Попередження PVS-Studio: V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 422, 424. Hal8188ERateAdaptive.c 422
static int odm_ARFBRefresh_8188E(
struct odm_dm_struct *dm_odm,
struct odm_ra_info *pRaInfo)
{ /* Wilson 2011/10/26 */
....
if (pRaInfo->HighestRate > 0x13)
pRaInfo->PTModeSS = 3;
else if (pRaInfo->HighestRate > 0x0b)
pRaInfo->PTModeSS = 2;
else if (pRaInfo->HighestRate > 0x0b)
pRaInfo->PTModeSS = 1;
else
pRaInfo->PTModeSS = 0;
....
return 0;
}

Ще одне місце, в якому йдуть підряд дві умови. Зверніть увагу, що тіла при цьому різні. Складно сказати, чи є тут реальна помилка, чи це просто невикористаний код: це вже завдання розробників проекту. Завдання аналізатора звернути увагу на підозріле місце.

Попередження PVS-Studio: V734 An excessive check. Examine the conditions containing search for the substrings «interleaver» and «deinterleaver». sst-atom-controls.c 1449
static int sst_fill_widget_module_info(
struct snd_soc_dapm_widget *w,
struct snd_soc_platform *platform)
{
struct snd_kcontrol *kctl;
int index, ret = 0;
struct snd_card *card = platform->component.card->snd_card;
char *idx;

down_read(&card->controls_rwsem);

list_for_each_entry(kctl, &card->controls, list) {
....

} else if (strstr(kctl->id.name, "interleaver")) {
struct sst_enum *e = (void *)kctl->private_value;

e->w = w;

} else if (strstr(kctl->id.name, "deinterleaver")) {
struct sst_enum *e = (void *)kctl->private_value;

e->w = w;
}

....
}

up_read(&card->controls_rwsem);
return 0;
}

В цьому фрагменті послідовно перевіряють наявність декількох підрядків у однієї рядку. Для наочності я залишив тільки цікавлять нас підрядка. Припустимо, що ми не знайшли interleaver — тоді немає сенсу шукати deinterleaver, адже підрядка interleaver вже точно немає. Тому цю ділянку коду ніколи не запрацює, але, так як тіла у if та else однакові, це не страшно. Це просто надлишковий код.

Попередження PVS-Studio: V547 Expression 'block' is always true. svclock.c 873
void
nlmsvc_grant_reply(struct nlm_cookie *cookie __be32 status)
{
struct nlm_block *block;

dprintk("grant_reply: looking for cookie %x, s=%d \n",
*(unsigned int *)(cookie->data), status);
if (!(block = nlmsvc_find_block(cookie)))
return;

if (block) {
if (status == nlm_lck_denied_grace_period) {
/* Try again in a couple of seconds */
nlmsvc_insert_block(block, 10 * HZ);
} else {
/* Lock now is held by client, or has been rejected.
* In both cases, the block should be removed. */
nlmsvc_unlink_block(block);
}
}
nlmsvc_release_block(block);
}

Цей приклад демонструє, чому статичного аналізатора недостатньо виконувати Pattern-based analysis, обходячи AST. Важливо вміти виконувати Control flow analysis і Data flow analysis. В момент, коли block == NULL відбувається return, відповідно далі за кодом ми точно можемо сказати, що вказівник ненульовий. І коли ми зустрічаємо перевірку на NULL, ми точно розуміємо, що щось тут не так.

Схоже, що друга перевірка покажчика тут просто зайва. Однак, раптом тут хотіли перевірити іншу змінну? Хто знає… Цей код аналізатора явно варто надати розробнику для перевірки.

Аналогічна ситуація:

Попередження PVS-Studio: V547 Expression 'sym' is always true. menu.c 498
bool menu_is_visible(struct menu *menu)
{
struct menu *child;
struct symbol *sym;

....

if (!sym || sym_get_tristate_value(menu->sym) == no) // <=
return false;

for (child = menu->list; child; child = child->next) {
if (menu_is_visible(child)) {
if (sym) // <=
sym->flags |= SYMBOL_DEF_USER;
return true;
}
}

return false;
}

Помилка в макросі
Попередження PVS-Studio: V733 It is possible that macro expansion resulted in incorrect evaluation order. Check expression: request->rq_timeout + 5 * 1000. niobuf.c 637
#define CFS_FAIL_TIMEOUT(id, secs) \
cfs_fail_timeout_set(id, 0, secs * 1000, CFS_FAIL_LOC_NOSET)

#define OBD_FAIL_TIMEOUT(id, secs) \
CFS_FAIL_TIMEOUT(id, secs)

int ptl_send_rpc(struct ptlrpc_request *request, int noreply)
{
....
OBD_FAIL_TIMEOUT(OBD_FAIL_PTLRPC_DELAY_SEND,
request->rq_timeout + 5);
....
}

А ось такі помилки дуже рідкісні. До цього я бачив тільки одне спрацьовування цієї діагностики в реальному проекті: примітно, що це був FreeBSD. Помилку допустили у визначенні макросів: найкраще всі його параметри оточувати дужками. Якщо цього не робити, то можливий такий випадок: при підстановці 'x + 5' в 'secs * 1000' виходить 'x + 5 * 1000', а це явно не те, що очікував автор.

Безглуздий memset
Попередження PVS-Studio: V597 The compiler could delete the 'memset' function call, which is used to flush 'ps' buffer. The memset_s() function should be used to erase the private data. atom.c 1383
int amdgpu_atom_asic_init(struct atom_context *ctx)
{
int hwi = CU16(ctx->data_table + ATOM_DATA_FWI_PTR);
uint32_t ps[16];
int ret;

memset(ps, 0, 64);

ps[0] = cpu_to_le32(CU32(hwi + ATOM_FWI_DEFSCLK_PTR));
ps[1] = cpu_to_le32(CU32(hwi + ATOM_FWI_DEFMCLK_PTR));
if (!ps[0] || !ps[1])
return 1;

if (!CU16(ctx->cmd_table + 4 + 2 * ATOM_CMD_INIT))
return 1;
ret = amdgpu_atom_execute_table(ctx, ATOM_CMD_INIT, ps);
if (ret)
return ret;

memset(ps, 0, 64); // <=

return ret;
}

Немає сенсу додавати memset перед return: компілятор, побачивши, що ця операція не змінює видимий стан програми (масив все одно виходить з області видимості), віддалить. Якщо потрібно стерти якісь важливі дані, то для цього варто використовувати memset_s або написати свій аналог.

Це помилка до речі фактично є вразливістю. Не затираються дані, які повинні бути затерті. З подробицями можна ознайомитися в описі діагностики V597. До речі, це дуже поширена уразливість: proof.

Небезпечне використання memcmp
Попередження PVS-Studio: V642 Saving the 'memcmp' function result inside the 'unsigned char' type variable is inappropriate. The significant bits could be lost breaking the program's logic. host.c 1789
static void power_control_timeout(unsigned long data)
{
....
u8 other = memcmp(requester->frame_rcvd.iaf.sas_addr,
iphy->frame_rcvd.iaf.sas_addr,
sizeof(requester->frame_rcvd.iaf.sas_addr));

if (other == 0) {
....
}
....
}

Якщо уважно прочитати, що говорить документація про повернутому значенні memcmp, то ми побачимо, що гарантії про якомусь конкретному діапазоні немає: функція може повернути будь-яке число в межах свого типу. І це не завжди -1, 0 1. Тому не можна зберігати його значення змінної меншого типу: при втраті старших розрядів молодші можуть скласти нуль. Схожа помилка призвела до уразливості в MySQL/MariaDB.

Висновок


Як вже говорилося, Linux — дуже якісний, добре протестований проект. Знайти в ній помилку, навіть саму незначну, — це вже привід для гордості. Це ж і привід задуматися про те, скільки помилок можна було б знайти до налагодження і тестування: саме в такому амплуа цінний статичний аналіз. Ви можете в цьому переконатися, спробувавши PVS-Studio. Для лінукса ви можете отримати тріал-версію аналізатора, написавши нам на пошту. А для своїх некомерційних проектів можна використовувати PVS-Studio безкоштовно: для цього досить познайомитися з цією статтею і скористатися відкритою і вільною утилітою how-to-use-pvs-studio-free.



Якщо хочете поділитися цією статтею з англомовної аудиторією, то прошу використовувати посилання на переклад: Pavel Belikov. Linux Kernel, tested by the Linux-version of PVS-Studio

Прочитали статтю і є питання?Часто до наших статей задають одні і ті ж питання. Відповіді на них ми зібрали тут: Відповіді на питання читачів статей про PVS-Studio, версія 2015. Будь ласка, ознайомтеся зі списком.
Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.