metrica
Мы используем куки, чтобы пользоваться сайтом было удобно.
Хорошо
to the top
close form

Заполните форму в два простых шага ниже:

Ваши контактные данные:

Шаг 1
Поздравляем! У вас есть промокод!

Тип желаемой лицензии:

Шаг 2
Team license
Enterprise license
** Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности
close form
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Бесплатная лицензия PVS‑Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

close form
check circle
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

Вебинар: Трудности при интеграции SAST, как с ними справляться - 04.04

>
>
>
Что скрывает загрузчик GNU? Ищем ошибки…

Что скрывает загрузчик GNU? Ищем ошибки в Grub

30 Сен 2016

Анализатор PVS-Studio продолжает осваивать платформу Linux. Посмотрим, какие ошибки он смог обнаружить в загрузчике операционных систем Grub.

0432_Grub_ru/image1.png

Введение

Статья расскажет о проверке программы-загрузчика для операционных систем семейства Unix - Grub. Grub разработан Erich Boleyn и входит в проект GNU. GRUB является эталонной реализацией загрузчика, соответствующего спецификации Multiboot, и может загрузить любую совместимую с ней операционную систему.

Проект Grub написан на языке С. Он уже проверялся другими анализаторами, в том числе Coverity. В таком проекте достаточно сложно найти непроверенные места. Однако, анализатор PVS-Studio справился с этой задачей и обнаружил несколько интересных ошибок.

Результаты проверки

Опечатки являются одной из самых распространённых ошибок в программах. От них не застрахованы даже профессиональные разработчики. С опечаток и начнём.

Опечатка в имени константы

typedef enum
{
  GRUB_PARSER_STATE_TEXT = 1,
  GRUB_PARSER_STATE_ESC,
  GRUB_PARSER_STATE_QUOTE,
  GRUB_PARSER_STATE_DQUOTE,
  ....
} grub_parser_state_t;

char * grub_normal_do_completion (....)
{
  ....
  if (*escstr == ' ' 
      && cmdline_state != GRUB_PARSER_STATE_QUOTE
      && cmdline_state != GRUB_PARSER_STATE_QUOTE)  // <=
        *(newstr++) = '\\';
  ....
}

Предупреждение: V501 There are identical sub-expressions 'cmdline_state != GRUB_PARSER_STATE_QUOTE' to the left and to the right of the '&&' operator. completion.c 502

Опечатки в именах констант, близких по написанию, - достаточно частое явление. Наверняка и в этом примере, вместо повторного сравнения значения cmdline_state с константой GRUB_PARSER_STATE_QUOTE, требовалось сверить его с константой GRUB_PARSER_STATE_DQUOTE.

Опечатка в имени регистра

struct grub_bios_int_registers
{
  grub_uint32_t eax;
  grub_uint16_t es;
  grub_uint16_t ds;
  grub_uint16_t flags;
  grub_uint16_t dummy;
  grub_uint32_t ebx;
  grub_uint32_t ecx;
  grub_uint32_t edi;
  grub_uint32_t esi;
  grub_uint32_t edx;
};

grub_vbe_status_t 
grub_vbe_bios_getset_dac_palette_width (....)
{
  struct grub_bios_int_registers regs;

  regs.eax = 0x4f08;
  regs.ebx = (*dac_mask_size & 0xff) >> 8;
  regs.ebx = set ? 1 : 0;                 // <=
  ....
}

Предупреждение: V519 The 'regs.ebx' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 312, 313. vbe.c 313

Структура regs является обёрткой для работы с регистрами памяти. Учитывая близость наименований регистров, очень легко допустить ошибку. Вероятнее всего, во втором случае, вместо ebx должен быть использован какой-то другой регистр. Точно указать, как именно исправить ошибку, не зная особенностей кода, невозможно. Главная цель анализатора - указать на проблемное место, а решить, как именно его следует исправить, разработчик сможет сам. Именно поэтому статический анализ наиболее актуален непосредственно во время разработки проектов.

Бессмысленное присваивание

static void free_subchunk (....)
{
  switch (subchu->type)
    {
    case CHUNK_TYPE_REGION_START:
      {
       grub_mm_region_t r1, r2, *rp;
       ....
       if (*rp)
       {
        ....
       }
       else
       {
         r1->pre_size = pre_size;
         r1->size = (r2 - r1) * sizeof (*r2);
         for (rp = &grub_mm_base; *rp; rp = &((*rp)->next))
           if ((*rp)->size > r1->size)
             break;
         r1->next = *rp;               // <=
         *rp = r1->next;               // <=
         h = (grub_mm_header_t) (r1 + 1);
         r1->first = h;
         h->next = h;
         h->magic = GRUB_MM_FREE_MAGIC;
         h->size = (r2 - r1 - 1);
       }
       ....
       if (r2)
       {
         ....
         r2->size += r1->size;
         ....
         hl2->next = r2->first;
         r2->first = r1->first;
         hl->next = r2->first;
         *rp = (*rp)->next;
         ....
       } 
       ....
      }
     ....
    }
  ....
}

Предупреждение: V587 An odd sequence of assignments of this kind: A = B; B = A;. Check lines: 338, 339. relocator.c 339

Подобные ошибки встречаются не так часто. Сложно судить, что именно предполагалось написать в данном случае. В коде происходит присваивание полю указателя, хранящегося в переменной *rp. А в следующей строке происходит обратная операция. И уже переменной *rp присваивается указатель r1->next. Подобный код не имеет смысла, так как переменная *rp уже хранит это значение. Рассматривая код, точно определить, есть здесь ошибка или это просто лишняя операция, практически невозможно. Всё же, я считаю, что это ошибка.

Некорректный аргумент memset

static void setup (....)
{
  ....
  struct grub_boot_blocklist *first_block, *block;
  ....
  /* Clean out the blocklists.  */
  block = first_block;
  while (block->len)
    {
     grub_memset (block, 0, sizeof (block)); // <=
     block--;
     ....
    }
  ....
}

Предупреждение: V579 The grub_memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. grub-setup.c 500

Функции для низкоуровневой работы с памятью служат рассадником для опечаток. Во время работы с ними часто неверно рассчитывают размер буфера. Вот и в данном примере вместо размера буфера block функция grub_memset получает в качестве третьего аргумента размер указателя. В результате буфер block будет очищен не полностью. Исправить код надо следующим образом:

grub_memset (block, 0, sizeof (*block));

Ещё несколько мест, где встретилась похожая ошибка:

  • V579 The grub_memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. mmap.c 148
  • V579 The grub_memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. mmap.c 165

Некорректная очистка памяти

static gcry_err_code_t do_arcfour_setkey (....)
{
  byte karr[256];
  ....
  for (i=0; i < 256; i++ )
    karr[i] = key[i%keylen];
  ....
  memset( karr, 0, 256 );   // <=

  return GPG_ERR_NO_ERROR;
}

Предупреждение: V597 The compiler could delete the 'memset' function call, which is used to flush 'karr' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. arcfour.c 108

Использование здесь функции memset для обнуления памяти некорректно. В ситуации, приведённой в этом фрагменте, сразу после вызова функции memset происходит выход из функции. В случае, если буфер более не используется, компилятор может удалить функцию во время сборки. Чтобы этого избежать, необходимо использовать функцию memset_s.

В проекте нашлось ещё несколько предупреждений, связанных с затиранием памяти:

  • V597 The compiler could delete the 'memset' function call, which is used to flush 'buf' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 209
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'bufhex' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 210
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'salt' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 213
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'salthex' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 214
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'buf' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 231
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'bufhex' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 232
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'salt' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 235
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'salthex' object. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 236
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'pass2' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 166
  • V597 The compiler could delete the 'memset' function call, which is used to flush 'pass1' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. grub-mkpasswd-pbkdf2.c 205

Лишняя операция

Int main (int argc, char *argv[])
{
  ....
  {
    FILE *f;
    size_t rd;
    f = fopen ("/dev/urandom", "rb");
    if (!f)
    {
      memset (pass1, 0, sizeof (pass1));
      free (buf);
      free (bufhex);
      free (salthex);
      free (salt);
      fclose (f);                     // <=
      ....
    }
    ....
    fclose (f);
  }
  ....
}

Предупреждение: V575 The null pointer is passed into 'fclose' function. Inspect the first argument. grub-mkpasswd-pbkdf2.c 184

В случае неудачного открытия файла происходит очистка временных переменных. Помимо этого, в условный блок затесалась функция fclose, предназначенная для закрытия файла. Проверка в выражении условного блока показывает, что файл открыт не был. Следовательно, закрывать его незачем. Согласно документации, в случае отправки в функцию значения NULL происходит вызов обработчика недопустимых параметров. Дальнейшее поведение программы зависит от настроек обработчика. Приведённый код в любом случае является ошибочным и для его исправления требуется удалить вызов функции fclose внутри условного оператора.

Ещё одно подозрительное место, найденное диагностикой V575:

  • V575 The null pointer is passed into 'free' function. Inspect the first argument. grub-setup.c 1187

Неиспользуемое значение

static grub_err_t grub_video_cirrus_setup (....)
{
  ....
  if (CIRRUS_APERTURE_SIZE >= 2 * framebuffer.page_size)
    err = grub_video_fb_setup (mode_type, mode_mask,
                   &framebuffer.mode_info,
                   framebuffer.ptr,
                   doublebuf_pageflipping_set_page,
                   framebuffer.ptr + framebuffer.page_size);
  else
    err = grub_video_fb_setup (mode_type, mode_mask,
                   &framebuffer.mode_info,
                   framebuffer.ptr, 0, 0);

  err = grub_video_cirrus_set_palette (0, 
                       GRUB_VIDEO_FBSTD_NUMCOLORS,
                       grub_video_fbstd_colors);
  return err;
}

Предупреждение: V519 The 'err' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 448, 460. cirrus.c 460

В сущности, в этом фрагменте нет серьёзной ошибки. Видимо предполагается, что функция grub_video_fb_setup не может вернуть ошибку. Если это так, то зачем заносить её результат в переменную, если сразу после этого значение перезаписывается. Можно предположить, что переменную просто используют для проверки значения при отладке. Но, возможно, тут действительно забыли добавить важную проверку. В любом случае, на это место стоит обратить внимание и скорректировать код.

Ещё одно подозрительное место:

  • V519 The 'err' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 368, 380. bochs.c 380

Заключение

Даже в хорошо проверенных проектах встречаются ошибки. Статический анализ полезен на любом этапе развития проекта. Ну а пока PVS-Studio for Linux готовится к выходу, можно почитать о результатах проверок других проектов.

Популярные статьи по теме


Комментарии (0)

Следующие комментарии next comments
close comment form