25 августа 2021 года ядру Linux исполняется 30 лет. За это время ядро пережило множество изменений, так же, как и мы. Сегодня это огромный проект, работающий на миллионах различных устройств. Предыдущую проверку мы делали 5 лет назад, поэтому не можем пропустить такое событие и не заглянуть в код этого эпического проекта.
В прошлый раз мы выписали 7 интересных ошибок. Примечательно, что сейчас ошибок, заслуживающих рассмотрения в статье, ещё меньше!
На первый взгляд это странно. Размер ядра увеличился. В анализаторе PVS-Studio появились десятки новых диагностических правил. Улучшены внутренние механизмы, анализ потока данных, появился межмодульный анализ и так далее. Как же так получилось, что интересных ошибок почти нет?
Очень просто. Проект стал ещё более качественным! Именно с этим мы и поздравляем Linux в его 30-летие.
Высокое качество связано с тем, что инфраструктура проекта заметно улучшилась. Теперь ядро можно собирать не только GCC, но и Clang – для этого не требуется дополнительных патчей. Дорабатываются другие инструменты статического анализа (появился GCC -fanalyzer, развивается местный анализатор Coccinelle, проект проверяется через Clang Static Analyzer), а также автоматизированные системы проверки кода (kbuild test robot). К тому же за поиск уязвимостей выплачивается Bug Bounty компанией Google.
Однако это не значит, что ошибок нет :). И несколько хороших мы сейчас разберём. "Хорошие и красивые" они, конечно, исключительно в нашем понимании :). Более того, статический анализ следует использовать регулярно, а не вот так наскоками раз в пять лет. Так, действительно, ничего не найдёшь. Подробнее эта мысль рассмотрена в статье "Ошибки, которые не находит статический анализ кода, потому что он не используется".
Но вначале пара слов про запуск анализатора.
Поскольку ядро теперь умеет собираться компилятором Clang, под него в проекте также была заведена инфраструктура, в том числе генератор compile_commands.json, который создаёт файл с JSON Compilation Database из .cmd файлов, генерируемых во время сборки. Так что для его создания придётся скомпилировать ядро. Необязательно использовать компилятор Clang, но рекомендуется производить компиляцию именно им из-за возможных несовместимых флагов у GCC.
Так что сгенерировать полный compile_commands.json для проверки проекта можно таким образом:
make -j$(nproc) allmodconfig # делаем полный конфиг
make -j$(nproc) # собираем
./scripts/clang-tools/gen_compile_commands.pyПосле этого можно запускать анализатор:
pvs-studio-analyzer analyze -f compile_commands.json -a 'GA;OP' -j$(nproc) \
-e drivers/gpu/drm/nouveau/nouveau_bo5039.c \
-e drivers/gpu/drm/nouveau/nv04_fbcon.cЗачем исключать из анализа эти 2 файла? Они содержат большое количество макросов, которые раскрываются в огромные строчки кода (до 50 тысяч символов на строку), из-за чего анализатор обрабатывает их долго, и их анализ может не пройти.
В недавнем релизе PVS-Studio 7.14 была добавлена возможность межмодульного анализа C/C++ проектов, и мы не могли придумать лучшего момента, чтобы его опробовать. К тому же на такой огромной кодовой базе:
Числа, конечно, внушительные. Суммарный объём проекта – почти 30 млн. строк кода. Вследствие этого изначальная проверка в этом режиме у нас прошла не совсем успешно: на этапе слияния межмодульной информации у нас была "съедена" вся оперативная память, и процесс анализатора был благополучно убит OOM-killer'ом. Мы разобрались в проблеме и придумали, что можно улучшить. Так что в следующем релизе 7.15 появится важная оптимизация, которая это исправит.
Для того, чтобы проверить проект в режиме межмодульного анализа, нужно всего лишь добавить один флаг в команду pvs-studio-analyzer:
pvs-studio-analyzer analyze -f compile_commands.json -a 'GA;OP' -j$(nproc) \
--intermodular \
-e drivers/gpu/drm/nouveau/nouveau_bo5039.c \
-e drivers/gpu/drm/nouveau/nv04_fbcon.cПосле анализа мы получаем отчёт на тысячи предупреждений. К сожалению, настроить анализатор, чтобы отсечь ложные срабатывания, не было времени. Хотелось опубликовать статью сразу после дня рождения. Поэтому за час было выписано 4 наиболее интересных случая.
Однако настройка анализатора – это просто. Большинство предупреждений связано с неудачными макросами. Чуть позже мы отфильтруем отчёт и изучим его уже по-настоящему. По итогам постараемся порадовать вас другой более подробной статьёй про найденные ошибки.
V595 The 'speakup_console[vc->vc_num]' pointer was utilized before it was verified against nullptr. Check lines: 1804, 1822. main.c 1804
static void do_handle_spec(struct vc_data *vc, u_char value, char up_flag)
{
unsigned long flags;
int on_off = 2;
char *label;
if (!synth || up_flag || spk_killed)
return;
....
switch (value) {
....
case KVAL(K_HOLD):
label = spk_msg_get(MSG_KEYNAME_SCROLLLOCK);
on_off = vt_get_leds(fg_console, VC_SCROLLOCK);
if (speakup_console[vc->vc_num]) // <= проверка
speakup_console[vc->vc_num]->tty_stopped = on_off;
break;
....
}
....
}Анализатор ругается на то, что указатель speakup_console[vc->vc_num] разыменовывается до проверки. При беглом взгляде на этот код может показаться, что срабатывание ложное. Однако разыменование действительно присутствует.
И знаете, где оно? :) Внутри макроса spk_killed. Да-да, это не переменная, как может показаться на первый взгляд:
#define spk_killed (speakup_console[vc->vc_num]->shut_up & 0x40)Скорее всего, тот, кто менял этот код, ожидал, что разыменований не происходит и сделал проверку, т.к. где-то происходит передача нулевого указателя. Так что такие макросы, которые выглядят как переменные и не являются константами, усложняют сопровождение кода и делают его более уязвимым к ошибкам. Макросы – это зло.
V519 The 'data' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 6208, 6209. cik.c 6209
static void cik_enable_uvd_mgcg(struct radeon_device *rdev,
bool enable)
{
u32 orig, data;
if (enable && (rdev->cg_flags & RADEON_CG_SUPPORT_UVD_MGCG)) {
data = RREG32_UVD_CTX(UVD_CGC_MEM_CTRL);
data = 0xfff; // <=
WREG32_UVD_CTX(UVD_CGC_MEM_CTRL, data);
orig = data = RREG32(UVD_CGC_CTRL);
data |= DCM;
if (orig != data)
WREG32(UVD_CGC_CTRL, data);
} else {
data = RREG32_UVD_CTX(UVD_CGC_MEM_CTRL);
data &= ~0xfff; // <=
WREG32_UVD_CTX(UVD_CGC_MEM_CTRL, data);
orig = data = RREG32(UVD_CGC_CTRL);
data &= ~DCM;
if (orig != data)
WREG32(UVD_CGC_CTRL, data);
}
}Пример взят из кода драйвера для видеокарт Radeon. Судя по тому, как значение 0xfff используется в else-ветке, можно предположить, что это на самом деле битовая маска. В то время как в then-ветке значение, полученное строкой выше, перезаписывается без применения маски. Вероятно, правильный код должен был быть таким:
data = RREG32_UVD_CTX(UVD_CGC_MEM_CTRL);
data &= 0xfff;
WREG32_UVD_CTX(UVD_CGC_MEM_CTRL, data);V610 Undefined behavior. Check the shift operator '>>='. The right operand ('bitpos % 64' = [0..63]) is greater than or equal to the length in bits of the promoted left operand. master.c 354
// bitsperlong.h
#ifdef CONFIG_64BIT
#define BITS_PER_LONG 64
#else
#define BITS_PER_LONG 32
#endif /* CONFIG_64BIT */
// bits.h
/*
* Create a contiguous bitmask starting at bit position @l and ending at
* position @h. For example
* GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
*/
#define __GENMASK(h, l) ....
// master.h
#define I2C_MAX_ADDR GENMASK(6, 0)
// master.c
static enum i3c_addr_slot_status
i3c_bus_get_addr_slot_status(struct i3c_bus *bus, u16 addr)
{
int status, bitpos = addr * 2; // <=
if (addr > I2C_MAX_ADDR)
return I3C_ADDR_SLOT_RSVD;
status = bus->addrslots[bitpos / BITS_PER_LONG];
status >>= bitpos % BITS_PER_LONG; // <=
return status & I3C_ADDR_SLOT_STATUS_MASK;
}Обратите внимание, что макрос BITS_PER_LONG может иметь значение 64.
В коде содержится неопределенное поведение:
Вероятно, автор хотел сократить количество строк и сделал объявление переменной bitpos рядом с переменной status. Однако он не учёл, что int имеет 32-битный размер на 64-битных платформах, в отличии от типа long.
Для исправления следует объявить переменную status с типом long.
V522 Dereferencing of the null pointer 'item' might take place. mlxreg-hotplug.c 294
static void
mlxreg_hotplug_work_helper(struct mlxreg_hotplug_priv_data *priv,
struct mlxreg_core_item *item)
{
struct mlxreg_core_data *data;
unsigned long asserted;
u32 regval, bit;
int ret;
/*
* Validate if item related to received signal type is valid.
* It should never happen, excepted the situation when some
* piece of hardware is broken. In such situation just produce
* error message and return. Caller must continue to handle the
* signals from other devices if any.
*/
if (unlikely(!item)) {
dev_err(priv->dev, "False signal: at offset:mask 0x%02x:0x%02x.\n",
item->reg, item->mask);
return;
}
// ....
}Здесь допущена классическая ошибка: если указатель нулевой, то необходимо вывести сообщение об ошибке. Однако этот же указатель используется при формировании сообщения. Конечно, ошибки подобного рода выявляются достаточно легко на этапе тестирования. Но конкретно в этом случае есть нюанс – судя по комментарию, это может произойти, если "какая-либо часть железки сломана". В любом случае код написан некорректно и его стоит исправить.
Проект Linux стал для нас хорошим испытанием – на нём мы смогли проверить новую возможность межмодульного анализа. Само ядро Linux – важный для всего мира проект. За его качество борются разные люди и организации. Мы очень рады, что оно развивается. А также развиваемся и мы! Недавно мы открыли наш "запас" картинок, демонстрирующий начало и развитие дружбы нашего анализатора с Tux'ом. Вот они:
Единорог N81:
Единорог N57:
Альтернативный единорог с пингвином N1:
Спасибо за внимание! Приглашаем попробовать проверить ваши собственные проекты. В честь дня рождения – промокод на месяц использования: #linux30.