Предлагаем вашему вниманию цикл статей, посвященных рекомендациям по написанию качественного кода на примере ошибок, найденных в проекте Chromium. Это шестая часть, которая будет посвящена функции malloc. Вернее, тому, почему следует обязательно проверять указатель, возвращаемый этой функцией. Скорее всего, вы не догадываетесь, какой подвох связан с malloc, потому рекомендуем познакомиться с этой статьей.
Примечание. В статье под функцией malloc часто будет подразумеваться, что речь идёт не только именно об этой функции, но и о calloc, realloc, _aligned_malloc, _recalloc, strdup и так далее. Не хочется загромождать текст статьи, постоянно повторяя названия всех этих функций. Общее у них то, что они могут вернуть нулевой указатель.
malloc
Если функция malloc не смогла выделить буфер памяти, то она возвращает NULL. Любая нормальная программа должна проверять указатели, которые возвращает функция malloc, и соответствующим образом обрабатывать ситуацию, когда память выделить не получилось.
К сожалению, многие программисты небрежно относятся к проверке указателей, а иногда сознательно не проверяют, удалось ли выделить память или нет. Их логика следующая:
| Если функция malloc не смогла выделить память, то вряд ли моя программа продолжит функционировать должным образом. Скорее всего, памяти будет не хватать и для других операций, поэтому можно вообще не заморачиваться об ошибках выделения памяти. Первое же обращение к памяти по нулевому указателю приведёт к генерации Structured Exception в Windows, или процесс получит сигнал SIGSEGV, если речь идёт о Unix-подобных системах. В результате программа упадёт, что меня устраивает. Раз нет памяти, то и нечего мучаться. Как вариант, можно перехватить структурное исключение/сигнал и обрабатывать разыменовывания нулевого указателя более централизовано. Это удобнее, чем писать тысячи проверок. |
Кстати, существует ещё одно оправдание разработчиков, почему они не проверяют, что вернула функция malloc. Функция malloc только резервирует память, но вовсе нет гарантии, что хватит физической памяти, когда мы начнём использовать выделенный буфер памяти. Поэтому, раз всё равно гарантии нет, то и проверять не надо. Например, именно так Carsten Haitzler, являющийся одним из разработчиков библиотеки EFL Core, объяснял, почему я насчитал более 500 мест в коде библиотеки, где отсутствуют проверки. Вот его комментарий к статье:
| OK so this is a general acceptance that at least on Linux which was always our primary focus and for a long time was our only target, returns from malloc/calloc/realloc can't be trusted especially for small amounts. Linux overcommits memory by default. That means you get new memory but the kernel has not actually assigned real physical memory pages to it yet. Only virtual space. Not until you touch it. If the kernel cannot service this request your program crashes anyway trying to access memory in what looks like a valid pointer. So all in all the value of checking returns of allocs that are small at least on Linux is low. Sometimes we do it… sometimes not. But the returns cannot be trusted in general UNLESS its for very large amounts of memory and your alloc is never going to be serviced — e.g. your alloc cannot fit in virtual address space at all (happens sometimes on 32bit). Yes overcommit can be tuned but it comes at a cost that most people never want to pay or no one even knows they can tune. Secondly, fi an alloc fails for a small chunk of memory — e.g. a linked list node… realistically if NULL is returned… crashing is about as good as anything you can do. Your memory is so low that you can crash, call abort() like glib does with g_malloc because if you can't allocate 20-40 bytes… your system is going to fall over anyway as you have no working memory left anyway. I'm not talking about tiny embedded systems here, but large machines with virtual memory and a few megabytes of memory etc. which has been our target. I can see why PVS-Studio doesn't like this. Strictly it is actually correct, but in reality code spent on handling this stuff is kind of a waste of code given the reality of the situation. I'll get more into that later. |
Chromium
Chromium здесь при том, что в используемых в нём библиотеках имеется не менее 70 ошибок, связанных с отсутствием проверки после вызова таких функций, как malloc, calloc, realloc. Да, в самом Chromium эти функции почти нигде не используются. В Chromium применяются только контейнеры или operator new. Однако, раз ошибки есть в используемых библиотеках, то значит, можно сказать, что они есть и в Chromium. Конечно, какие-то части библиотек могут не использоваться при работе Chromium, но определять это сложно и ненужно. Всё равно надо править все ошибки.
Я не буду приводить в статье множество фрагментов кода с ошибками, так как они однотипны. Приведу для примера только одну ошибку, обнаруженную в библиотеке Yasm:
static SubStr *
SubStr_new_u(unsigned char *s, unsigned int l)
{
SubStr *r = malloc(sizeof(SubStr));
r->str = (char*)s;
r->len = l;
return r;
}Предупреждение PVS-Studio: V522 CWE-690 There might be dereferencing of a potential null pointer 'r'. Check lines: 52, 51. substr.h 52
В коде нет никакой защиты от нулевого указателя. Другие подобные ошибки из Chromium и используемых библиотек я собрал вместе в файл и выложил их здесь: chromium_malloc.txt. В файле упоминаются 72 ошибки, но на самом деле их может быть больше. Как я писал в вводной статье, я просматривал отчёт только поверхностно.
Согласно Common Weakness Enumeration обнаруженные ошибки PVS-Studio классифицирует как:
- CWE-690: Unchecked Return Value to NULL Pointer Dereference.
- CWE-628: Function Call with Incorrectly Specified Arguments.
- CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
Как видите, даже в таком высококачественном проекте как Chromium, можно заметить массу дефектов, связанных с отсутствием проверок. Теперь я перехожу к самому интересному и расскажу, почему проверки обязательно нужны.
Почему обязательно нужна проверка
Есть сразу 4 причины, каждой из которых достаточно, чтобы обязательно делать проверки после вызова функции malloc. Если кто-то в команде не пишет проверки, то обязательно заставьте его прочитать эту статью.
Прежде чем начать, небольшая теоретическая справка, почему возникают структурные исключения или сигналы, если происходит разыменовывание нулевого указателя. Это будет важно для дальнейшего повествования.
В начале адресного пространства одна или несколько страниц памяти защищены операционной системой от записи. Это позволяет выявить ошибки обращения к памяти по нулевому указателю, или указателю, значение которого близко к 0.
В разных операционных системах для этих целей резервируется разное количество памяти. При этом в некоторых ОС это значение можно настраивать. Поэтому нет смысла называть какое-то конкретное число зарезервированных байт памяти. Но чтобы как-то сориентировать читателя, скажу, что в Linux системах типовым значением является 64 Кбайт.
Важно, что, прибавив к нулевому указателю какое-то достаточно большое число, можно «промазать» мимо контрольных страниц памяти и случайно попасть в какие-то незащищенные от записи страницы. Таким образом можно испортить где-то какие-то данные, но операционная система этого не заметит и никакого сигнала / исключения она не сгенерирует.
Заваривайте кофе, мы начинаем!
Разыменовывание нулевого указателя — это неопределённое поведение
С точки зрения языка C и C++ разыменовывание нулевого указателя приводит к неопределенному поведению. Неопределённое поведение — это что угодно. Не думайте, что вы знаете, как будет вести себя программа, если произойдёт разыменовывание nullptr. Современные компиляторы занимаются серьезными оптимизациями, в результате чего бывает невозможно предсказать, как проявит себя та или иная ошибка в коде.
Неопределённое поведение программы — это очень плохо. Вы не должны допускать его в своём коде.
Не думайте, что сможете совладать с разыменовыванием нулевого указателя, используя обработчики структурных исключений (SEH в Windows) или сигналы (в UNIX-like системах). Раз разыменовывание нулевого указателя было, то работа программы уже нарушена, и может произойти что угодно. Давайте рассмотрим абстрактный пример, почему нельзя полагаться на SEH-обработчики и т.п.
size_t *ptr = (size_t *)malloc(sizeof(size_t) * N * 2);
for (size_t i = 0; i != N; ++i)
{
ptr[i] = i;
ptr[N * 2 - i - 1] = i;
}Этот код заполняет массив от краёв к центру. К центру значения элементов увеличиваются. Это придуманный за 1 минуту пример, поэтому не гадайте, зачем такой массив кому-то нужен. Я и сам не знаю. Мне было важно, чтобы в соседних строках программы происходила запись в начало массива и куда-то в его конец. Такое иногда бывает нужно и в практических задачах, и мы рассмотрим реальный код, когда доберёмся до 4-ой причины.
Ещё раз внимательно посмотрим на эти две строки:
ptr[i] = i;
ptr[N * 2 - i - 1] = i;С точки зрения программиста, в начале цикла произойдёт запись в элемент ptr[0], и возникнет структурное исключение/сигнал. Оно будет обработано, и всё будет хорошо.
Однако компилятор в каких-то целях оптимизации может переставить присваивания местами. Он имеет на это полное право. С точки зрения компилятора, если указатель разыменовывается, то он не может быть равен nullptr. Если указатель нулевой, то это неопределённое поведение, и компилятор не обязан думать о последствиях оптимизации.
Так вот, компилятор может решить, что в целях оптимизации выгоднее выполнить присваивания так:
ptr[N * 2 - i - 1] = i;
ptr[i] = i;В результате в начале произойдет запись по адресу ((size_t *)nullptr)[N * 2 — 0 — 1]. Если значение N достаточно велико, то страница защиты в начале памяти будет «перепрыгнута» и значение переменной i может быть записано в какую-то ячейку, доступную для записи. В общем, произойдёт порча каких-то данных.
И только после этого будет выполнено присваивание по адресу ((size_t *)nullptr)[0]. Операционная система заметит попытку записи в контролируемую ею область и сгенерирует сигнал/исключение.
Программа может обработать это структурное исключение/сигнал. Но уже поздно. Где-то в памяти есть испорченные данные. Причем непонятно, какие данные испорчены и к каким последствиям это может привести!
Виноват ли компилятор, что поменял операции присваивания местами? Нет. Программист допустил разыменовывание нулевого указателя и тем самым ввёл программу в состояние неопределённого поведения. В данном конкретном случае неопределённое поведение программы будет заключаться в том, что где-то в памяти испорчены данные.
Вывод
Исходите из аксиомы: любое разыменовывание нулевого указателя — это неопределённое поведение программы. Не бывает «безобидного» неопределённого поведения. Любое неопределённое поведение недопустимо.
Не допускайте разыменовывания указателей, которые вернула функция malloc и её аналоги, без их предварительной проверки. Не полагайтесь на какие-то другие способы перехвата разыменовывания нулевого указателя. Следует использовать только старый добрый оператор if.
Разыменовывание нулевого указателя — это уязвимость
То, что некоторые разработчики вообще не считают за ошибку, другие воспринимают как уязвимость. Именно так обстоит дело с разыменовыванием нулевого указателя.
Одним нормально, если программа из-за разыменовывания нулевого указателя упадёт или если ошибка будет обработана каким-то общим способом с помощью перехвата сигнала/структурного исключения.
Другие считают, что разыменовывание нулевого указателя приводят к ошибке «отказ в обслуживании» и является уязвимостью. Вместо того, чтобы штатно обработать нехватку памяти, программа, или одна из нитей программы, завершает свою работу. Это может приводить к потере данных, нарушению целостности данных и так далее. Другими словами, CAD система тупо закроется, если не сможет выделить память для какой-то сложной операции, не предложив пользователю даже сохранить результат своей работы.
Не буду голословным. Есть, такая программа как Ytnef, предназначенная для декодирования TNEF потоков, например, созданных в Outlook. Так вот, разработчики приложения считают отсутствие проверки после вызова calloc уязвимостью CVE-2017-6298.
Все исправленные места, в которых могло произойти разыменовывание нулевого указателя, имели приблизительно один и тот же вид:
vl->data = calloc(vl->size, sizeof(WORD));
temp_word = SwapWord((BYTE*)d, sizeof(WORD));
memcpy(vl->data, &temp_word, vl->size);Выводы
Если вы разрабатываете безответственное приложение, для которого упасть в процессе работы не является бедой, то да, писать проверки необязательно.
Однако, если вы разрабатываете какую-то библиотеку, то отсутствие проверок недопустимо! Вашей библиотекой могут пользоваться не только ленивые программисты, создающие безответственные приложения, типа игры Тетрис. Надо заботиться и о нормальных программистах, и нормальных программах.
Поэтому я идеологически не согласен, например, с Carsten Haitzler, что в библиотеке EFL Core нет проверок (подробности в статье). Это не позволяет построить на основе таких библиотек надёжные приложения.
В общем, если вы создаёте библиотеку, то помните, что в некоторых приложениях разыменовывание нулевого указателя — это уязвимость. Необходимо обрабатывать ошибки выделения памяти и штатно возвращать информацию об неудаче.
Где гарантии, что будет разыменовывание именно нулевого указателя?
Те, кто ленится писать проверки, почему-то думают, что разыменование затрагивает именно нулевые указатели. Да, часто именно так и бывает. Но может ли поручиться программист за код всего приложения? Уверен, что нет.
Сейчас я на практических примерах покажу, что я имею в виду. Возьмём, например, код из библиотеки LLVM-subzero, которая используется в Chromium. Если честно, я теряюсь в догадках, какая связь между проектом Chromium и LLVM, но она есть.
void StringMapImpl::init(unsigned InitSize) {
assert((InitSize & (InitSize-1)) == 0 &&
"Init Size must be a power of 2 or zero!");
NumBuckets = InitSize ? InitSize : 16;
NumItems = 0;
NumTombstones = 0;
TheTable = (StringMapEntryBase **)
calloc(NumBuckets+1,
sizeof(StringMapEntryBase **) +
sizeof(unsigned));
// Allocate one extra bucket, set it to look filled
// so the iterators stop at end.
TheTable[NumBuckets] = (StringMapEntryBase*)2;
}Предупреждение PVS-Studio: V522 CWE-690 There might be dereferencing of a potential null pointer 'TheTable'. Check lines: 65, 59. stringmap.cpp 65
Сразу после выделения буфера памяти происходит запись в ячейку TheTable[NumBuckets]. Если значение переменной NumBuckets достаточно большое, то мы испортим какие-то данные с непредсказуемыми последствиями. После такой порчи вообще нет смысла рассуждать, как будет работать программа. Могут последовать самые неожиданнейшие последствия.
Аналогичные опасные присваивания я вижу ещё в двух местах этого проекта:
- V522 CWE-690 There might be dereferencing of a potential null pointer 'Buckets'. Check lines: 219, 217. foldingset.cpp 219
- V769 CWE-119 The 'NewTableArray' pointer in the 'NewTableArray + NewSize' expression could be nullptr. In such case, resulting value will be senseless and it should not be used. Check lines: 218, 216. stringmap.cpp 218
Так что это не уникальный случай, а вполне типовая ситуация, когда данные записываются не точно по нулевому указателю, а по какому-то произвольному смещению.
Продолжу заочную дискуссию с Carsten Haitzler. Он утверждает, что они понимают, что делают, когда не проверяют результат вызова функции malloc. Нет, не понимают. Давайте взглянем, например, на вот такой фрагмент кода из библиотеки EFL:
static void
st_collections_group_parts_part_description_filter_data(void)
{
....
filter->data_count++;
array = realloc(filter->data,
sizeof(Edje_Part_Description_Spec_Filter_Data) *
filter->data_count);
array[filter->data_count - 1].name = name;
array[filter->data_count - 1].value = value;
filter->data = array;
}Предупреждение PVS-Studio: V522 There might be dereferencing of a potential null pointer 'array'. edje_cc_handlers.c 14249
Примечание. Я использую старые исходники EFL Core Libraries, которые остались у меня со времён написания статьи про эту библиотеку. Поэтому код или номера строк могут уже не соответствовать тому, что есть сейчас. Однако это не важно для повествования.
Перед нами типовая ситуация: в буфере не хватает свободного места для хранения данных, и его следует увеличить. Для увеличения размера буфера используется функция realloc, которая может вернуть NULL.
Если это произойдёт, то вовсе не обязательно возникнет структурное исключение/сигнал из-за разыменовывания нулевого указателя. Взглянем вот на эти строчки:
array[filter->data_count - 1].name = name;
array[filter->data_count - 1].value = value;Если значение переменной filter->data_count достаточно большое, то значения будут записаны по какому-то непонятному адресу.
В памяти будут испорчены какие-то данные, а программа продолжит своё выполнение. Последствия вновь непредсказуемые, но ничего хорошего точно не получится.
Я внимательно не стал изучать старый отчёт, касающийся EFL Core Libraries, но это точно не единственная подобная ошибка. Я заметил как минимум ещё два похожих места, где после realloc данные дописываются по какому-то индексу.
Вывод
Я вновь задаю вопрос: «где гарантии, что будет разыменовывание именно нулевого указателя?». Нет такой гарантий. Невозможно, разрабатывая или модифицируя код, помнить про только что рассмотренный нюанс. Запросто можно что-то испортить в памяти, при этом программа продолжит выполняться как ни в чём не бывало.
Единственный способ написать надёжный и правильный код — это всегда проверять результат, который вернула функция malloc. Проверь и живи спокойно.
Где гарантии, что memset заполняет память в прямом порядке?
Найдётся кто-то, кто скажет что-то подобное:
| Я отлично понимаю про realloc и всё остальное, что написано в статье. Но я профессионал и, выделяя память, сразу заполняю её нулями с помощью memset. Там, где действительно необходимо, я использую проверки. Но лишние проверки после каждого malloc я писать не буду. |
int Resampler::Reset(int inFreq, int outFreq, size_t num_channels) {
....
state1_ = malloc(8 * sizeof(int32_t));
memset(state1_, 0, 8 * sizeof(int32_t));
....
}Выделяется память, затем буфер заполняется нулями. Очень частая практика, хотя, на самом деле, две строчки можно сократить до одной, используя calloc. Но всё это не важно.
Главное, что даже подобный код не безопасен! Функция memset не обязана начать заполнять память с начала и тем самым вызывать разыменовывание нулевого указателя.
Функция memset имеет право начать заполнять буфер с конца. И, если выделялся большой буфер, то могут быть затёрты какие-то полезные данные. Да, заполняя память, функция memset рано или поздно достигнет страницы, защищённой от записи, и операционная система сгенерирует структурное исключение/сигнал. Однако обрабатывать их уже не имеет смысла. К этому моменту будет испорчен большой фрагмент памяти, и дальнейшая работа программы будет непредсказуема.
Читатель может возразить, что всё это носит исключительно теоретический характер. Да, функция memset теоретически может заполнять буфер начиная с конца буфера, но на практике никто не будет так реализовывать эту функцию.
Соглашусь, что подобная реализация memset действительно экзотика, и я даже задавал вопрос на StackOverflow на эту тему. В ответе говорится:
The Linux kernel's memset for the SuperH architecture has this property: link.
К сожалению, это код на незнакомой мне разновидности ассемблера, поэтому я не берусь рассуждать о нём. Зато ещё есть вот такая интересная реализация на языке Си. Приведу начало это функции:
void *memset(void *dest, int c, size_t n)
{
unsigned char *s = dest;
size_t k;
if (!n) return dest;
s[0] = c;
s[n-1] = c;
....
}Обратите внимание на:
s[0] = c;
s[n-1] = c;Здесь мы возвращаемся к причине N1 «Разыменовывание нулевого указателя — это неопределённое поведение». Нет гарантии, что компилятор в целях оптимизации не поменяет присваивания местами. Если компилятор это сделает, и аргумент n будет иметь большое значение, то вначале будет испорчен какой-то байт памяти. И только потом произойдёт разыменовывание нулевого указателя.
Опять не убедительно? Хорошо, а как вам вот такая реализация:
void *memset(void *dest, int c, size_t n)
{
size_t k;
if (!n) return dest;
s[0] = s[n-1] = c;
if (n <= 2) return dest;
....
}Вывод
Нельзя доверять даже функции memset. Да, это во многом искусственная и надуманная проблема. Я просто хотел показать, как много существует нюансов, если не проверять значение указателя. Просто невозможно всё это учесть. Поэтому не надо выпендриваться, а следует аккуратно проверять каждый указатель, который вернула функция malloc и аналогичные ей. И вот именно тогда вы станете профессионалом.
Заключение
Всегда сразу проверяйте указатель, который вернула функция malloc или аналогичная ей.
Как видите, анализатор PVS-Studio совсем не зря предупреждает о том, что нет проверки указателя после вызова malloc. Невозможно написать надёжный код, не делая проверки. Особенно это важно и актуально для разработчиков библиотек.
Надеюсь, теперь вы по-новому взглянете на функцию malloc, проверки указателей в коде и предупреждения анализатора PVS-Studio. Не забудьте показать эту статью своим коллегам и начать использовать PVS-Studio. Желаю всем поменьше багов.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Why it is important to check what the malloc function returned.
