Методика проведения разметки результатов статического анализа

Для фильтрации предупреждений, относящихся к выбранным подсистемам, в режиме Review необходимо нажать на кнопку Filters->Basic... и внизу в поле Files ввести список интересующих путей. Например:

security/selinux;net/ipv4

При этом надо убедиться, что нажата кнопка .*, находящаяся рядом с Files.

По результатам анализа:
1. Необходимо выставить вердикт:

  • Confirmed - ошибка, которую рекомендуется исправить.
  • Won't fix - истинное срабатывание, которое по тем или иным причинам исправлять нецелесообразно.
  • False Positive - ложное срабатывание инструмента статического анализа.
  • Unclear - требуется дополнительный анализ.

2. На вкладке Comment необходимо оставить комментарий, поясняющий вердикт.

Типовые ситуации при разметке предупреждений в ядре

Защитное программирование

Как пример, детекторы вида UNREACHABLE_CODE.ENUM или UNREACHABLE_CODE.DEFAULT сообщают о недостижимости кода в switch для ветки по умолчанию, в которой содержится та или иная отладочная печать. Например:

switch () {
case ENUM1:
...
default:
    dev_dbg(ice_pf_to_dev(pf), "Invalid container type %d\n", c_type);
    return -EINVAL;
}

Этот код, действительно, недостижим при нормальной работе ядра. Тем не менее, его, как правило, имеет смысл оставить, чтобы при возникновении нештатных ситуаций, управление пошло по более контролируемой логике, и в журнале появилась информация об обнаружении нештатной ситуации.

Аналогичная ситуация возникает в детекторах вида UNREACHABLE_CODE при вызове функций, которые всегда возвращают 0. Тем не менее, для многих из них можно предположить, что реализация со временем может измениться, и функция начнёт возвращать код ошибки. Поэтому обработку кода ошибки можно оставить на будущее.

В этом случае рекомендуется выставить вердикт Won't fix и написать комментарий Защитное программирование.

Недостижимый код, зависящий от конфигурации

Детекторы вида UNREACHABLE_CODE.* сообщают о недостижимости кода, который, действительно, недостижим в конфигурации, которая подвергалась статическому анализу, но может быть достижим в другой конфигурации. В SVACE в будущем планируется поддержать выявление таких ситуаций и подавлять такие предупреждения.

До тех пор при разметке предупреждений рекомендуется выставить вердикт Won't fix и написать комментарий Код достижим в другой конфигурации.

FREE_OF_ARITHM при получении указателя на структуру по указателю на одно из её полей

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

Стандартный приём, используемый в ядре Linux: работа с указателем на одно из полей структуры (например, на поля являющиеся ссылками вперед-назад для двунаправленного списка) и при необходимости получение указателя на начало структуры путём применения макроса container_of к указателю на её поле. Используется для работы со списками, RCU, в различных видах циклов и пр. Если при приведении используется макрос contaner_of, вызванный с корректными параметрами, то использование такого приёма является безопасным, а освобождение памяти — корректным.

Пример:

struct ip6_flowlabel {
    ... 
    struct rcu_head 		 rcu; 
    ... 
};

static void fl_free_rcu(struct rcu_head *head)
{ 
    struct ip6_flowlabel *fl = container_of(head, struct ip6_flowlabel, rcu); 
    ... 
    kfree(fl);    // // <-- FREE_OF_ARITHM 
} 

static void fl_free(struct ip6_flowlabel *fl) 
{ 
    ... 
    call_rcu(&fl->rcu, fl_free_rcu); 
}

При разметке предупреждений рекомендуется выставить вердикт Won't fix и написать комментарий Использование container_of для получения указателя на начало структуры.

VARIABLE_IS_NOT_ARRAY.PROC при работе с массивами длины 1

Возникает при передаче переменной как массива (указатель и длина) при вызове функций типа of_property_read_*() или copy_from_user().

Пример:

static int mmc_of_get_func_num(struct device_node *node)
{
    u32 reg;
    int ret;

    ret = of_property_read_u32(node, "reg", &reg);
    if (ret < 0)
        return ret;
    return reg;
}

При применении этого правила важно убедиться, что тип указателя у вызываемой функции совпадает с типом переменной, а в качестве длины массива передаётся единица.

Рекомендации по разметке: Won't fix с комментарием Переменная рассматривается как массив размера 1 с целью унификации кода при работе как с единичными объектами, так и с массивами.

SIGNED_TO_BIGGER_UNSIGNED

Возникает при присваивании выражения знакового типа переменной беззнакового типа большей разрядности (при этом происходит sign extension). Как правило, надо исправлять типы переменных.

Пример:

int typec_set_mode(struct typec_port *port, int mode)
{
    struct typec_mux_state state = { };
    state.mode = mode;
    return typec_mux_set(port->mux, &state);
}

Рекомендации по разметке: Confirmed/Minor/Fix required.

DEREF_AFTER_FREE при итерации по спискам

Срабатывают следущие детекторы: DEREF_AFTER_FREE.

Стандартный прием безопасного удаления элемента списка при обходе. Для этого используется макрос

list_for_each_entry_safe():

#define list_for_each_entry_safe(pos, n, head, member)			\
	for (pos = list_first_entry(head, typeof(*pos), member),	\
		n = list_next_entry(pos, member);			\
	     !list_entry_is_head(pos, head, member); 			\
	     pos = n, n = list_next_entry(n, member))

Пример:

static void serio_free_event(struct serio_event *event)
{
	module_put(event->owner);
	kfree(event);
}

static void serio_remove_pending_events(void *object)
{
	struct serio_event *event, *next;
	unsigned long flags;

	spin_lock_irqsave(&serio_event_lock, flags);

	list_for_each_entry_safe(event, next, &serio_event_list, node) {  // <-- DEREF_AFTER_FREE

		if (event->object == object) {
			list_del_init(&event->node); // Элемент удаляется из списка
			serio_free_event(event);     // Освобождается память, но next элемент отстается! В конце цикла происходит инициализация event = next;
		}
	}

	spin_unlock_irqrestore(&serio_event_lock, flags);
}

Ввиду сложных операций с указателями инструмент считает, что возможно дальнейшее использование уже освобождённого указателя.

Рекомендации по разметке: False Positive с комментарием Происходит обращение к следующему элементу списка, а не к освобождённому.

SIZEOF_POINTER_TYPE для функций, не связанных с выделением памяти

Данный тип детектора предназначен для обнаружения несоответствий в конструкциях вида x = (T1*)alloc(sizeof(T2)), когда тип передаваемый в оператор sizeof() не соответствует по размеру типу, к которому приводится результирующий указатель. Если инструмент обнаруживает похожую конструкцию, которая используется не для выделения памяти, то предупреждение будет ошибочным.

Например при использовании макроса:

#define cmpxchg(ptr, ...)						\
({									\
	typeof(ptr) __ai_ptr = (ptr);					\
	instrument_atomic_write(__ai_ptr, sizeof(*__ai_ptr));		\
	arch_cmpxchg(__ai_ptr, __VA_ARGS__);				\
})
#endif

где arch_cmpxchg в свою очередь расширяется в:


#define __cmpxchg_wrapper(sfx, ptr, o, n)				\
({									\
	__typeof__(*(ptr)) __ret;					\
	__ret = (__typeof__(*(ptr)))					\
		__cmpxchg##sfx((ptr), (unsigned long)(o),		\
				(unsigned long)(n), sizeof(*(ptr)));	\
	__ret;								\
})

Рекомендации по разметке: False Positive с комментарием Вызываемая функция не является функцией выделения памяти, на которые нацелен данный детектор.

PROC_USE.VULNERABLE при использовании функции sprintf()

Предупреждения относительно небезопасности использования sprintf() при подготовки вывода содержимого псевдофайлов, например, в sysfs.

Пример:

return sprintf(buf, "%d\n", udev->rx_lanes);

Рекомендации по разметке: Confirmed/Minor/Fix required.

При подготовке патча согласно последним тенденциям в таких случаях предлагается использовать вспомогательные функции sysfs_emit()/sysfs_emit_at() или scnprintf().

В случае если подготовленный патч с данным исправлением не был принят мейнтейнерами, изменить разметку на Won't fix/Minor/Ignore и в комментарии к предупреждению добавить ссылку на ответ мейнтейнера.

NULL_AFTER_DEREF из-за бессмысленной проверки на NULL указателя на поле структуры

Иногда возникает при проверке на NULL, которому ранее был присвоен указатель на поле структуры.

Пример:

pipe_ctx = &aconnector->dc_link->dc->current_state->res_ctx.pipe_ctx[i];
if (pipe_ctx && pipe_ctx->stream &&
    pipe_ctx->stream->link == aconnector->dc_link)
    break;

Это избыточная проверка, так как после присваивания адреса поля структуры, указатель гарантированно не будет равным NULL, даже если значение указателя на структуру было NULL. Эта неадекватная проверка на NULL не является проблемой с точки зрения безопасности, но является проблемой с точки зрения качества кода. Если по случаю смещение поля структуры относительно начала структуры является нулевым, то всё равно код рекомендуется исправить.

Рекомендации по разметке: Confirmed/Minor/Fix required с комментарием вида: Да, pipe_ctx не может быть нулевым. Неадекватная проверка на NULL не является проблемой с точки зрения безопасности, но является проблемой с точки зрения качества кода.

Взаимодействие с международным сообществом

Для отправки патчей в основную ветку ядра следуйте следующим инструкциям:
Отправка патчей в ядро.