vitalif/vitastor
5
6
Fork 12
Code Issues 4 Pull Requests 6 Actions Releases 111 Wiki Activity

Memory/logic vulnerabilities review using Opus 4.8 w/ ultracode #74

New Issue
Open
opened 2026-05-29 05:36:21 +03:00 by kamillaova · 1 comment
kamillaova commented 2026-05-29 05:36:21 +03:00
Copy Link

Аудит ошибок Vitastor (SDS) — отчёт

Метод: 19 агентов-«искателей» прошлись по всему пути данных (blockstore + heap-метаданные, OSD: RMW/EC/peering/scrub/primary, клиент: cluster_client/messenger/RDMA/NBD/ublk, etcd-клиент, NFS: XDR/KV/proxy/portmap, KV DB, util, disk_tool). Каждый кандидат независимо перепроверялся тремя скептиками с разными линзами (поток управления / время жизни памяти / достижимость триггера). Баг попадал в отчёт только при подтверждении ≥2/3.

Итог: рассмотрено 30 кандидатов, 28 подтверждено (все — 3/3), 2 отклонено как false-positive.

Баги разделены на две категории:

  • Категория A — проявляются сами по себе при штатной работе (старт, flush, scrub, peering, конфигурация, утечки, повреждённые на диске данные).
  • Категория B — провоцируются внешним/удалённым вводом (remote OOB/overflow по сети из NFS/RPC-клиентов).

Раздел в конце — отклонённые false-positive с обоснованием.

Сводка

ID Severity Категория Файл Тип
B-1 ? critical B (remote) nfs/proto/xdr_impl_inline.h:94-97 OOB read (~4 ГБ)
B-2 ? critical B (remote) nfs/proto/xdr_impl_inline.h:318-333 heap overflow
B-3 ? critical B (remote) nfs/nfs_portmap.cpp:134 heap overflow
B-4 ? critical B (remote/клиент) nfs/nfs_kv_write.cpp:430-439 OOB / abort
A-1 ? critical A blockstore/blockstore_disk.cpp:529-557 logic → BLKDISCARD по живым данным
A-2 ? critical A osd/osd_rmw.cpp:1190-1219,1341 heap overflow
A-3 ? critical A disk_tool/disk_tool_meta.cpp:929-937 unsigned underflow → OOB
A-4 ? critical A blockstore/v1/read.cpp:175,246-247 NULL-deref / wild memset
A-5 ? high A blockstore/blockstore_heap.cpp:560-569 && вместо ||
A-6 ? high A blockstore/blockstore_impl.cpp:348 memmove: count вместо bytes
A-7 ? high A client/cluster_client.cpp:347-369 use-after-free
A-8 ? high A client/etcd_state_client.cpp:1240-1255 разыменование end()-итератора
A-9 ? high A client/msgr_send.cpp:300-305 утечка памяти → OOM
A-10 ? high A osd/osd_primary_chain.cpp:52-64,85 assert-abort (crash-loop)
A-11 ? high A osd/osd_scrub.cpp:630-635 OOB read + порча метаданных
A-12 ? high A osd/osd_peering_pg.cpp:162-164 сдвиг — UB
A-13 ? high A disk_tool/disk_tool_journal.cpp:507-535 heap overflow (guard = no-op)
A-14 ? high A kv/kv_db.cpp:234-243 integer overflow → OOB read
A-15 ? high A util/rw_blocking.cpp:60-93 busy-loop на EOF
A-16 ? medium A blockstore/v1/read.cpp:364-370 OOB read iovec-массива
A-17 ? medium A blockstore/v1/read.cpp:682-689,947-963 утечка блока (loc==0)
A-18 ? medium A osd/osd_primary_write.cpp:247-251 утечка subops (ENOSPC)
A-19 ? medium A osd/osd_peering.cpp:421-457 logic → PG застревает в STOPPING
A-20 ? medium A client/nbd_proxy.cpp:74-78 утечка fd (free vs nl_socket_free)
A-21 ? medium A client/ublk_server.cpp:717-744 NULL-deref (get_sqe)
A-22 ? medium A util/rw_blocking.cpp:18-31 EOF по устаревшему errno
A-23 ? medium A kv/kv_db.cpp:601-618 OOB read (rescue)
A-24 ? medium A disk_tool/disk_tool_utils.cpp:36-45 OOB read строки

FP-1, FP-2 — см. раздел «Отклонённые false-positive».

Категория B — провоцируются внешним/удалённым вводом

Класс наибольшего риска: достижимы по сети от NFS/RPC-клиентов, часть — без аутентификации. Если NFS-прокси/портмаппер открыт наружу, это первый приоритет.

B-1 ? Удалённый OOB-read (~4 ГБ): переполнение в len_pad4() обходит проверку границ XDR-строки/opaque

  • Файл: src/nfs/proto/xdr_impl_inline.h:94-97, 132, 142-148
  • Категория: B — нужен crafted-ввод (поле длины 0xFFFFFFFF)
  • Верификация: 3/3 скептиков подтвердили

Механизм. В xdr_bytes() (декодирование) 32-битная длина с провода читается в len и передаётся в len_pad4():

uint32_t inline len_pad4(uint32_t len) { return ((len+3)/4) * 4; }
...
uint32_t padded = len_pad4(len);
if (xdrs->avail < 4+padded) return 0;
data->size = len;
data->data = (char*)(xdrs->buf+4);

Для len ∈ {0xFFFFFFFD, 0xFFFFFFFE, 0xFFFFFFFF} выражение len+3 переполняется в {0,1,2}, поэтому padded == 0. Проверка avail < 4+0 проходит для любого буфера ≥4 байт, и data->size устанавливается в len (~4 ГБ). Параметр maxlen на декодировании не проверяется — ничто не ограничивает длину.

Триггер. Один поддельный NFS/portmap-RPC-запрос с полем строки/opaque (имя в LOOKUP, путь монтирования, file handle, любое xdr_string/xdr_bytes-поле) с префиксом длины 0xFFFFFFFF. Любой потребитель (xdr_string_t::operator std::string() → std::string(data, size), operator==) читает ~4 ГБ за пределами маленького приёмного буфера.

Последствия. OOB-чтение → падение демона NFS (DoS) или утечка соседней heap-памяти. Без аутентификации, удалённо.

Исправление. Проверять переполнение до округления: if (len > xdrs->avail - 4) return 0; (в 64-битной арифметике) и применять maxlen на декодировании. Это системная проблема — чинить централизованно.

B-2 ? Удалённое heap-переполнение: переполнение счётчика элементов в xdr_array() → undersized malloc

  • Файл: src/nfs/proto/xdr_impl_inline.h:318-333
  • Категория: B — нужен crafted-ввод (счётчик элементов)
  • Верификация: 3/3 скептиков подтвердили

Механизм.

*len = be32toh(*((uint32_t*)xdrs->buf));
if (*len > maxlen) return 0;          // maxlen == ~0 для ряда массивов
*data = (char*)malloc_or_die(entry_size * (*len));   // 32-битное умножение
for (uint32_t i = 0; i < *len; i++)
    fn(xdrs, *data + entry_size*i);

Произведение entry_size * (*len) считается в 32-битной беззнаковой арифметике и переполняется. Атакующий подбирает *len так, чтобы произведение свернулось в малое значение, а сам *len остался огромным. Пример: SETACL3args.ace[], entry_size = sizeof(nfsacl_ace) = 12, *len = 0x55555556 → 12*0x55555556 mod 2^32 = 8 байт. Выделяется 8 байт, а цикл пишет 12-байтные записи далеко за их пределы.

Триггер. Поддельный NFSv3 SETACL (maxlen=~0, entry_size=12); сотни ACE-записей в пакете → переполнение на тысячи управляемых атакующим байт. Аналогично через xdr_xdr_write_chunk (RDMA, entry_size=16, maxlen=~0) и массивы GETACL/auth_flavors.

Последствия. Управляемое атакующим heap-переполнение → порча памяти / удалённый крах демона. Потенциально эксплуатируемо.

Исправление. Проверять if (*len != 0 && entry_size > SIZE_MAX / *len) return 0; в 64-битной арифметике до malloc, и проверять, что *len помещается в оставшийся ввод.

B-3 ? Удалённое heap-переполнение в PMAP3 DUMP: выделяются слоты под указатель, пишутся полные структуры

  • Файл: src/nfs/nfs_portmap.cpp:134
  • Категория: B — удалённо; срабатывает и при штатном монтировании
  • Верификация: 3/3 скептиков подтвердили

Механизм.

pmap3_mapping_list *list = (pmap3_mapping_list*)malloc_or_die(
    sizeof(pmap3_mapping_list*) * self->reg_ports.size());   // 8 байт на запись!
...
list[i] = (pmap3_mapping_list){...};   // пишется полная структура ~64 байта

Выделяется sizeof(указатель) = 8 байт на запись, но пишется полная pmap3_mapping_list (~64 байта: pmap3_mapping = 2×u_int + 3×xdr_string_t{size_t,char*} = 56 байт + указатель next). Соседний pmap2_dump_proc (строка 67) корректно использует sizeof(pmap2_mapping_list) — это опечатка, лишняя *. При 4 всегда регистрируемых сервисах (PMAP_V2, PMAP_V3, NFS_V3, MOUNT_V3): выделяется 32 байта, пишется 4×64 = 256 байт + list[i-1].next = NULL.

Триггер. Любой клиент, шлющий PMAP v3 DUMP на порт 111 — в т.ч. rpcinfo и стандартный rpcbind-зонд, который Linux NFS-клиенты отправляют при монтировании. Переполнение происходит при любом dump-запросе, включая легитимные.

Последствия. Удалённо провоцируемое heap-переполнение в демоне NFS-прокси → порча соседней heap-памяти, крах демона (DoS для всех NFS-клиентов), потенциальная эксплуатация.

Исправление. Убрать *: malloc_or_die(sizeof(pmap3_mapping_list) * self->reg_ports.size()).

B-4 ? Неверная длина зануления хвоста в NFS shared-write

  • Файл: src/nfs/nfs_kv_write.cpp:430-439
  • Категория: B — провоцируется последовательностью запросов NFS-клиента; может возникнуть и при легитимной нагрузке
  • Верификация: 3/3 скептиков подтвердили

Механизм. В nfs_do_shared_write хвост после новых данных в ветке has_old==false заполняется add_zero(op, st->offset, ...) — это copy-paste из ведущей области (строки 415-424). Должно быть st->new_size-(st->offset+st->size) (как в соседней ветке has_old):

if (st->offset+st->size < st->new_size) {
    if (has_old)
        op->iov.push_back(st->aligned_buf+st->offset+st->size, st->new_size-(st->offset+st->size));
    else
        add_zero(op, st->offset, st->proxy->kvfs->zero_block);   // должно быть new_size-(offset+size)
}

В iov попадает меньше байт, чем op->len (=aligned_size) → assert(left == 0) в cluster_client_t::add_iov (src/client/cluster_client.cpp:1232).

Триггер. Создать файл через CREATE/MKNOD UNCHECKED с атрибутом размера N>0 (nfs_kv_create.cpp:320-324 → empty=true, size=N), затем маленький WRITE, где offset+size+header < shared_inode_threshold. resume_1 ставит new_size=N, срабатывает перевод в shared-путь, nfs_do_shared_readmodify ничего не делает (shared_ino==0, aligned_buf==NULL), и nfs_do_shared_write идёт с has_old==false и offset+size<new_size.

Последствия. Падение демона NFS-прокси по assert (DoS). С отключёнными assert — короткая запись iov → запись неверной длины/мусора в shared-inode → тихая порча данных файла.

Исправление. В else-ветке использовать add_zero(op, st->new_size-(st->offset+st->size), ...).

Категория A — проявляются сами по себе

A-1 ? trim_data: continue пропускает j = i+1 → BLKDISCARD по живым (используемым) блокам данных

  • Файл: src/blockstore/blockstore_disk.cpp:529-557
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. Параметр-предикат trim_data называется is_free, но все вызывающие передают предикат is-used (blockstore_impl.cpp:93 heap->is_data_used(...); disk_tool_discard.cpp:97, v1/impl.cpp:97 data_alloc->get(...)) — он возвращает true для ЗАНЯТОГО блока. Когда discard_granularity делает выровненный диапазон пустым (range[0] >= range[1]), выполняется continue, который прыгает к инкременту цикла и пропускает j = i+1 (строка 556). Занятый блок i не исключается, j продолжает указывать в раннюю свободную область. На следующей границе i2 вычисляется диапазон [j, i2), охватывающий занятый блок, и по нему выдаётся BLKDISCARD.

if (range[0] >= range[1])
    continue;            // ← пропускает j = i+1 ниже
range[1] -= range[0];
r = ioctl(data_fd, BLKDISCARD, &range);
...
j = i+1;

Триггер. discard_granularity больше/невыровнен относительно квалифицирующего свободного прогона. Путь выполняется на живом OSD при старте с discard_on_start=true (blockstore_impl.cpp:91-93) и в vitastor-disk discard.

Последствия. BLKDISCARD по диапазону с занятыми блоками безвозвратно зануляет данные клиента → тихая потеря/порча данных тома.

Исправление. Выполнять j = i+1 всегда (вынести из-под continue), либо заменить continue на присваивание j перед ним.

A-2 ? Heap-переполнение в ec_check_combination: буфер размером по stripe_count, индексация по role

  • Файл: src/osd/osd_rmw.cpp:1190-1219, 1341
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. ec_find_good() выделяет tmp_buf = malloc_or_die(stripe_count*(chunk_size+bitmap_size)), где stripe_count — число реально прочитанных чанков. ec_check_combination() индексирует по РОЛИ i ∈ [0, pg_size):

uint8_t *tmp_buf = (uint8_t*)malloc_or_die(stripe_count*(chunk_size+bitmap_size));
...
bs.read_buf = bs.write_buf = tmp_buf + i*chunk_size;
bs.bmp_buf  = tmp_buf + stripe_count*chunk_size + i*bitmap_size;

При stripe_count < pg_size роли i ∈ [stripe_count, pg_size) дают указатели за пределами выделенной области, а reconstruct_stripes_ec()/calc_rmw_parity_ec() пишут туда полный chunk_size данных и bitmap_size.

Триггер. Scrub деградированного EC-объекта. submit_scrub_subops строит массив stripes из object_state->osd_set — по одной записи на присутствующий чанк, так что stripe_count = число присутствующих < pg_size. ec_find_good входит в перебор при live_roles > pg_minsize. Напр. EC 3+2 (pg_size=5, pg_minsize=3) с одним отсутствующим чанком: 4 живые роли > 3 → перебор с stripe_count=4 < pg_size=5.

Последствия. Запись полного чанка (обычно 128 КБ) + bitmap за концом heap-аллокации при scrub любого деградированного EC-объекта. Деградированные PG — штатное состояние, scrub автоматический → достижимый крах / порча памяти OSD.

Исправление. Выделять буфер по pg_size, а не stripe_count: malloc_or_die(pg_size*(chunk_size+bitmap_size)) и согласовать смещение bitmap-региона.

A-3 ? Underflow беззнакового счётчика в обработке rollback при write-meta → бесконечный цикл и OOB read

  • Файл: src/disk_tool/disk_tool_meta.cpp:929-937
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм.

for (size_t i = jbo.size()-1; i >= 0; i--)   // i — size_t, i >= 0 всегда истинно
{
    if (jbo[i]["version"].uint64_value() > rollback_to)
        jbo.erase(jbo.begin()+i, jbo.begin()+i+1);
}

i беззнаковый, i >= 0 всегда истинно. (1) Пустой jbo → jbo.size()-1 == SIZE_MAX → jbo[SIZE_MAX] сразу. (2) Непустой — после i==0 i-- сворачивается в SIZE_MAX → jbo[SIZE_MAX]["version"]. Цикл не завершается нормально.

Триггер. vitastor-disk write-meta при конвертации старого формата (v1/v2) metadata+journal JSON-дампа в heap-формат, если журнал содержит хоть одну запись типа "rollback".

Последствия. Крах (SIGSEGV) / UB с чтением далеко за границами вектора во время конвертации метаданных оператором; инструмент не может завершиться, при повреждённом индексе может записать мусорные метаданные перед крахом.

Исправление. for (size_t i = jbo.size(); i-- > 0; ) либо знаковый индекс.

A-4 ? Wild memset через NULL read_buf в fill_partial_checksum_blocks из flusher

  • Файл: src/blockstore/v1/read.cpp:175, 246-247 (вызов из v1/flush.cpp:1089-1092)
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. scan_dirty() вызывает fill_partial_checksum_blocks() с read_buf = NULL, но функция безусловно делает read_buf -= read_offset; (read.cpp:175), где read_offset = v[0].offset, округлённый вниз до csum_block_size, > 0 если первый перезаписываемый блок не на смещении 0. NULL превращается в (uint8_t*)(0 - read_offset). Дальше read_range_fulfilled() делает if (read_buf) memset(read_buf + el.offset, 0, el.len); — guard на NULL уже не срабатывает, и memset пишет нули по адресу около нуля.

read_buf -= read_offset;          // NULL → (uint8_t*)(0 - read_offset)
...
if (read_buf)                     // guard уже не NULL
    memset(read_buf + el.offset, 0, el.len);

Триггер. Flush частично перезаписанного checksum-объекта (csum_block_size > bitmap_granularity, запись не выровнена по csum-блоку, первый патчируемый блок на offset>0, старый объект имеет дыры).

Последствия. Запись нулей по адресу около NULL → SIGSEGV в основном пути flush; если адрес замаплен — тихая порча памяти. Перемежающийся крах storage-демона.

Исправление. Не вычитать read_offset, если read_buf == NULL; держать NULL до самого memset-guard.

A-5 ? finish_load: одна insert_list_items для разных объектов (&& вместо ||)

  • Файл: src/blockstore/blockstore_heap.cpp:560-569
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. finish_load() сортирует postponed_items по (inode ASC, stripe ASC, lsn DESC) и должен нарезать на пакеты по одному объекту. insert_list_items() разрешает голову списка ровно один раз из v[0] (inode_map_get(..., wr->stripe)) и сливает все остальные элементы в эту цепочку только по LSN (is_before). Значит, пакет ОБЯЗАН содержать один объект (одинаковые inode И stripe). Но граница задана так:

if (e >= n || postponed_items[e]->entry.inode != postponed_items[s]->entry.inode &&
    postponed_items[e]->entry.stripe != postponed_items[s]->entry.stripe)
{
    insert_list_items(postponed_items.data()+s, e-s, false);
    s = e;
}

По приоритету (!= > && > ||) это e>=n || (inode_differs && stripe_differs) — новый пакет начинается, только когда РАЗЛИЧАЮТСЯ ОБА. Для частого случая «один inode (образ) владеет многими stripe-объектами» разбиения нет → записи разных stripe сливаются в первую цепочку, а головы остальных объектов не регистрируются в inode-map.

Триггер. Старт/загрузка metadata-стора, где ≥2 объекта накопили глубокую историю версий (postpone срабатывает при пропуске >POSTPONE_INSERT_COUNT=10 записей того же объекта — норма при тяжёлой перезаписи до компакции).

Последствия. Порча in-memory индекса объектов при старте: записи одного объекта вклеиваются в цепочку другого, головы части объектов не попадают в inode-map. read_entry()/list_objects() возвращают версии/локации чужого объекта (тихая порча данных) или NULL для существующих (потеря данных); кривые цепочки роняют assert() в unlink_list_item/remove_list_item/mark_garbage.

Исправление. Заменить && на ||: новый пакет, когда различается inode ИЛИ stripe.

A-6 ? process_list: memmove использует число элементов как число байт (в 24 раза меньше)

  • Файл: src/blockstore/blockstore_impl.cpp:348
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм.

if (stable_count > op->list_stable_limit) {
    memmove(result + op->list_stable_limit, result + stable_count, unstable_count);
    stable_count = op->list_stable_limit;
}

result — obj_ver_id*, sizeof(obj_ver_id)=24 (object_id{inode,stripe}=16 + version=8). Указатели масштабируются арифметикой, но размер memmove — unstable_count (число элементов) вместо unstable_count * sizeof(obj_ver_id). Копируется ~1/24 данных, остальные unstable-слоты сохраняют мусор от отброшенных stable-записей. retval = limit + unstable_count, поэтому вызывающий читает мусор как unstable-объекты.

Триггер. BS_OP_LIST с list_stable_limit (scrub/recovery), когда stable-объектов в PG больше лимита И есть unstable-объекты.

Последствия. Scrub/recovery работает по неверным (inode,stripe,version) → неверные решения о целостности → возможна порча данных.

Исправление. memmove(..., unstable_count * sizeof(obj_ver_id));

A-7 ? Use-after-free в continue_ops(): закэшированный next_op может быть освобождён реентрантным завершением

  • Файл: src/client/cluster_client.cpp:347-369
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. continue_ops() кэширует next_op = op->next до прогона op через continue_rw()/continue_sync(), затем безусловно op = next_op и перечитывает op->next. Прогон op может синхронно завершить и освободить next_op.

for (auto op = op_queue_head; op; ) {
    cluster_op_t *next_op = op->next;
    ...
    if (!op->retry_after && !op->prev_wait) {
        if (op->opcode == OSD_OP_SYNC) continue_sync(op); else continue_rw(op);
    }
    op = next_op;
    if (continuing_ops == 2) goto restart;   // никогда не срабатывает: continuing_ops ∈ {0,1}
}

Сценарий: WRITE (не immediate-commit) + следующий за ним SYNC, ждущий WRITE. WRITE ретраится по таймеру; на ретрае try_send() → TRY_SEND_OFFLINE, истёк OSD_OP_WAIT_UP_TIMEOUT → -ETIMEDOUT → erase_op(WRITE) → inc_wait → continue_sync(SYNC); dirty_osds пуст → SYNC стирается, его callback удаляет cluster_op_t. Возврат в цикл → op = next_op указывает на освобождённый SYNC. Защита continuing_ops == 2 мертва (присваивание убрано в коммите f8ff39b0).

Последствия. UAF cluster_op_t в I/O-пути клиента: разыменование освобождённой операции, возможный повторный прогон уже завершённого SYNC → крах клиента (QEMU/NBD/fio/vitastor-cli) или двойной callback.

Исправление. Восстановить корректную перезагрузку обхода (continuing_ops=2/goto restart) после реентрантного завершения, либо проверять валидность перед op = next_op.

A-8 ? Разыменование end()-итератора при дублирующихся именах inode

  • Файл: src/client/etcd_state_client.cpp:1240-1255
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм.

auto it = this->inode_config.find(inode_num);
if (it != this->inode_config.end() && it->second.name != "") {
    auto n_it = this->inode_by_name.find(it->second.name);
    if (n_it->second == inode_num)   // ← n_it может быть end()
        this->inode_by_name.erase(n_it);

Инвариант «inode_config[x].name != "" ⇒ inode_by_name содержит это имя» нарушается при двух inode с одинаковым именем. Последовательность: A с именем "foo" → B с именем "foo" (перезаписывает inode_by_name["foo"]=B, но inode_config[A].name остаётся "foo") → удаление B (стирает inode_by_name["foo"]) → удаление/обновление A: find("foo") возвращает end(), n_it->second разыменовывает end().

Триггер. Дубль имён образов в etcd (переходно при rename или из-за мисконфигурации) + определённый порядок удаления/обновления.

Последствия. SIGSEGV/UB в любом демоне (клиент QEMU/fio, OSD), читающем конфигурацию inode из etcd → кластерный простой I/O.

Исправление. Проверять n_it != inode_by_name.end() перед разыменованием.

A-9 ? zc_free_list: NULL-маркер не удаляется для OUT-only zerocopy-отправок → неограниченный рост памяти

  • Файл: src/client/msgr_send.cpp:300-305
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. На каждую zerocopy-отправку приходят два завершения (more=true, затем prev=true F_NOTIF). На первом безусловно добавляется NULL-маркер в zc_free_list. На втором батч освобождается:

int i = 0;
for (; i < cl->zc_free_list.size() && cl->zc_free_list[i]; i++)
    delete cl->zc_free_list[i];
if (i > 0)
    cl->zc_free_list.erase(cl->zc_free_list.begin(), cl->zc_free_list.begin()+i+1);

MSGR_SENDP_FREE ставится только для OSD_OP_IN-ответов; OSD_OP_OUT его не несёт. Для батча только из OUT-операций список = [NULL], i остаётся 0, if (i > 0) ложно — маркер не удаляется. Каждая такая отправка добавляет один NULL навсегда.

Триггер. Исходящие OSD↔OSD соединения шлют репликационные записи (OSD_OP_SEC_WRITE/_STABLE, все OUT). Большие записи (≥ min_zerocopy_send_size, дефолт 32 КБ) идут zerocopy-путём при поддержке sendmsg_zc.

Последствия. На каждую большую zerocopy-запись zc_free_list растёт на один элемент и не сжимается → неограниченная утечка → OOM на write-heavy OSD за длительное аптайм.

Исправление. Удалять одиночный NULL-маркер и при i == 0 (стирать [begin, begin+1)).

A-10 ? Ретрай chained-read на EIO/EDOM оставляет op_data->st устаревшим → assert-abort

  • Файл: src/osd/osd_primary_chain.cpp:52-64, 85
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. При ошибке чтения/контрольной суммы на subop chained-read op_data->st доходит до 4. На -EIO/-EDOM код делает check_corrupted_chained(), освобождает буферы и goto resume_1 без сброса op_data->st (он всё ещё 4). В read_bitmaps() st не равен ни base_state(1), ни base_state+1(2) → assert(op_data->st < base_state) = assert(4 < 1) → abort. Сборка релиза снимает -DNDEBUG (src/CMakeLists.txt:43-51), assert живой в продакшене.

goto resume_1;            // op_data->st всё ещё 4
...
assert(op_data->st < base_state);   // assert(4 < 1) → abort

Триггер. Один битый сектор (-EIO) или несовпадение контрольной суммы (-EDOM) при chained-read снапшота/клона (inode с parent_id).

Последствия. Крах primary OSD по assert вместо ошибки/реконструкции. Объект остаётся повреждённым → повторные клиентские чтения снова роняют OSD (crash-loop) → недоступность всех PG этого OSD.

Исправление. Перед goto resume_1 выставлять op_data->st = 0.

A-11 ? EC-scrub индексирует stripes[] по chunk.role вместо индекса stripe → OOB read + порча метаданных

  • Файл: src/osd/osd_scrub.cpp:630-635
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. op_data->stripes[] имеет размер stripe_count = число НЕ-outdated чанков, заполняется последовательно (stripes[i].role = chunk.role), индекс не связан с номером роли. Но код обращается к op_data->stripes[chunk.role], где chunk.role — до pg_size-1 (до 255 для EC):

else if (op_data->stripes[i].read_end > 0 && !op_data->stripes[chunk.role].missing &&
    (chunk.loc_bad & LOC_CORRUPTED)) { ... }

При наличии outdated-чанка stripe_count < число ролей → chunk.role >= stripe_count → OOB-чтение heap-массива; даже в границах читается .missing чужого stripe.

Триггер. Scrub EC/XOR-объекта с outdated-копией (stripe_count уменьшен) + чанк, ранее помеченный LOC_CORRUPTED, чья роль ≥ stripe_count, и который теперь читается корректно.

Последствия. OOB-чтение (крах на границе страницы) и чтение мусорного .missing → неверное решение о снятии LOC_CORRUPTED → primary может читать действительно битый чанк (выдача неверных данных) или отбросить хороший.

Исправление. Использовать op_data->stripes[i].missing вместо [chunk.role].

A-12 ? Маска n_roles: 32-битный 1 << replica для EC-ролей до 255 → UB сдвига

  • Файл: src/osd/osd_peering_pg.cpp:162-164
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм.

unsigned replica = (list[list_pos].oid.stripe & STRIPE_MASK);
...
if (!(has_roles & (1 << replica))) {
    has_roles = has_roles | (1 << replica);
    n_roles++;
}

1 — знаковый int, replica до pg_size-1 (EC допускает до 256, etcd_state_client.cpp:868). 1 << replica при replica >= 31 — UB (сдвиг ≥ ширины/в знаковый бит); даже с (uint64_t)1 роли ≥64 не представимы. n_roles (число присутствующих чанков) считается неверно при pg_size > 31.

Триггер. Любой EC-пул с pg_size > 31 (напр. 30+3) на peering/расчёте состояний объектов.

Последствия. На широких EC-пулах решение complete/degraded/incomplete считается по испорченной маске → завершённые объекты помечаются incomplete (отказ чтений/записей, зависание I/O) или деградированные — complete (чтение по недостаточному набору чанков). Плюс формальный UB.

Исправление. (uint64_t)1 << replica и хранилище шире 64 бит (напр. std::bitset/массив) при pg_size > 64.

A-13 ? write_json_journal: проверка границ читает small_write.len до присваивания → heap overflow

  • Файл: src/disk_tool/disk_tool_journal.cpp:507-535
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. ne указывает в обнулённый new_journal_buf. Проверка

if (new_journal_data - new_journal_buf + ne->small_write.len > new_journal_len) { ... return 1; }

выполняется ДО заполнения структуры (*((journal_entry_small_write*)ne) = {... .len = rec["len"] ...} идёт позже, строки 516-530). Поэтому ne->small_write.len читается как 0 → проверка всегда проходит. Затем fromhexstr(rec["data"]..., ne->small_write.len, new_journal_data) пишет реальные len байт, а guard, который должен был это поймать, считался против 0.

Триггер. vitastor-disk write-journal из JSON-дампа, где суммарные данные small-write превышают new_journal_len.

Последствия. Heap-overflow при реконструкции журнала; защита-no-op → порча соседней памяти, крах или запись повреждённого журнала на диск.

Исправление. Вычислять длину из rec["len"] в локальную переменную и проверять её до записи.

A-14 ? Integer overflow в read_string() → OOB read

  • Файл: src/kv/kv_db.cpp:234-243
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм.

uint32_t len = *(uint32_t*)(data+*pos);
*pos += sizeof(uint32_t);
if (*pos+len > size) { *pos = -1; return ""; }
std::string key((char*)data+*pos, len);

*pos — int, len — uint32_t: сложение в 32-битной беззнаковой арифметике. При len ≈ 0xFFFFFFFF *pos+len сворачивается в малое значение → проверка проходит → std::string(data+pos, ~4 ГБ) OOB read.

Триггер. Повреждённый/torn KV-блок с целыми заголовками (magic/block_size/type/items), но испорченным полем длины ключа/значения. Достижимо из get_block, find_size, rescue.

Последствия. Многогигабайтное OOB-чтение в kv_block_t::parse() → SIGSEGV вместо чистого -EILSEQ. Восстановимое состояние битого блока превращается в крах демона.

Исправление. Проверять if (len > (uint32_t)(size - *pos)) ... либо в 64-битной арифметике.

A-15 ? readv_blocking/writev_blocking/sendv_blocking: бесконечный цикл на EOF

  • Файл: src/util/rw_blocking.cpp:60-93
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. Ошибкой считается только r < 0. r == 0 (peer закрыл соединение/EOF до доставки всех байт iov) не обрабатывается: done += 0, внутренний цикл попадает на первый iov с iov_len > 0, берёт ветку iov_len > r (т.е. > 0), вычитает 0, сдвигает базу на 0 и break БЕЗ инкремента v. Внешний while (v < iovcnt) истинен → readv() снова возвращает 0 → 100% CPU навсегда.

Триггер. fio_sec_osd.cpp:377 readv_blocking(connect_fd, ...) читает payload ответа OSD; если OSD упал/закрыл соединение после заголовка — следующий readv возвращает 0.

Последствия. Отключение удалённого пира превращается в неубиваемый busy-loop (зависание на 100% CPU) вместо чистой ошибки.

Исправление. Обрабатывать r == 0 как EOF (возврат/ошибка), а не как «продолжить».

A-16 ? OOB read iovec-массива в read_checksum_block (батчинг IOV_MAX)

  • Файл: src/blockstore/v1/read.cpp:364-370
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. Выделяется массив ровно из n_iov iovec, подаётся батчами по IOV_MAX. Для каждого батча (кроме одиночного первого чанка) сумма длин считается for (int i = 0; i < IOV_MAX; i++) d_len += iov[n_pos+i].iov_len;. На последнем батче, когда n_iov > IOV_MAX и не кратно, реальных записей n_cur = n_iov-n_pos < IOV_MAX → читаются индексы >= n_iov за концом буфера.

Триггер. Очень крупные/фрагментированные checksum-block-чтения, дающие > IOV_MAX (обычно 1024) сегментов.

Последствия. Heap OOB-чтение за аллокацией + неверный data->iov.iov_len → handle_read_event ложно помечает чтение ошибкой → ложный short-read/IO-error клиенту.

Исправление. Ограничить внутренний цикл min(IOV_MAX, n_iov - n_pos).

A-17 ? Ссылка used_clean_objects не освобождается для блока данных в локации 0

  • Файл: src/blockstore/v1/read.cpp:682-689, 947-963
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. fulfill_clean_read() сохраняет PRIV(read_op)->clean_block_used = clean_loc. Для первого блока данных clean_loc == 0: used_clean_objects[0] создаётся, uo.refs++. Путь освобождения в handle_read_event защищён if (PRIV(op)->clean_block_used) — ложно при локации 0, refcount не уменьшается, запись не удаляется.

Триггер. Любое чтение, затрагивающее блок данных в локации 0.

Последствия. Постоянная (refs>0) запись в used_clean_objects. Если flusher позже релоцирует/освободит блок (was_freed=true), он не возвращается аллокатору → теряется один блок из free-пула + накопление stale-состояния.

Исправление. Использовать sentinel (напр. UINT64_MAX) для «нет ссылки» вместо 0, либо отдельный bool-флаг.

A-18 ? Утечка массива write-subops при частичном сбое EC-записи (ENOSPC)

  • Файл: src/osd/osd_primary_write.cpp:247-251
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. При частичном сбое записи (часть subops успешна) код намеренно НЕ удаляет op_data->subops (osd_primary_subops.cpp:491-497). Для реплик: mark_partial_write + delete[] op_data->subops. Для EC/XOR: submit_primary_rollback_subops() делает op_data->subops = new osd_op_t[n_subops];, перезаписывая ещё живой указатель без delete[]. Rollback-массив позже освобождается, исходный — утекает.

Триггер. EC/XOR-запись, частично упавшая с ENOSPC (диск полон на части чанк-OSD).

Последствия. Утечка одного new osd_op_t[n_subops] на каждую такую запись. При disk-full клиент/recovery повторяют записи → утечка растёт именно при дефиците памяти.

Исправление. Освобождать исходный op_data->subops перед переназначением в EC-ветке.

A-19 ? relock_pg стирает relock-записи из diff_osds без коррекции relock_osd_count

  • Файл: src/osd/osd_peering.cpp:421-457
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. diff_osds = [relock-записи (первые relock_osd_count)] ++ [unlock-записи]. Классификация: unlock_peer = (i >= relock_osd_count). Когда пир из relock-части подключён, но не поддерживает PG-locks, запись стирается diff_osds.erase(begin()+(i--)) без декремента relock_osd_count. Все последующие unlock-записи сдвигаются влево → реальная unlock-запись оказывается на индексе < relock_osd_count → классифицируется как relock.

Триггер. Смешанный по версиям кластер (rolling upgrade), где хотя бы один cur_peer OSD без фичи pg_locks, при наличии пиров на unlock.

Последствия. Пир, который надо разблокировать, записывается в pg.lock_peers с ненулевым state. can_stop() возвращает false → PG застревает в PG_STOPPING, блокируя миграцию/takeover.

Исправление. Декрементировать relock_osd_count при стирании записи из relock-части.

A-20 ? netlink-сокет освобождается через free() вместо nl_socket_free()

  • Файл: src/client/nbd_proxy.cpp:74-78
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. netlink_sock_free() освобождает struct nl_sock* (от nl_socket_alloc()) через libc free() вместо libnl nl_socket_free(). nl_socket_alloc() открывает netlink-fd (через genl_connect→socket()) и refcounted nl_cb. free() освобождает только структуру nl_sock, оставляя fd открытым и nl_cb утёкшим. (В том же файле строки 59, 66 в путях ошибок корректно вызывают nl_socket_free.)

Триггер. Каждая операция netlink-map/unmap/revive (netlink_configure(), netlink_disconnect()).

Последствия. Каждый netlink-map утекает netlink-fd + nl_cb. Для долгоживущего прокси fd держится всё время жизни устройства; повторные map/unmap исчерпывают таблицу дескрипторов.

Исправление. nl_socket_free(sk) вместо free(sk).

A-21 ? Непроверенный get_sqe() → NULL-deref при queue_depth больше размера io_uring-кольца

  • Файл: src/client/ublk_server.cpp:717-744
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. start_device() пред-сабмитит queue_depth fetch-запросов в цикле, каждый submit_request() → ringloop->get_sqe(), а submit() вызывается ПОСЛЕ цикла — значит все queue_depth SQE должны быть живы одновременно. Кольцо создаётся с фиксированным RINGLOOP_DEFAULT_SIZE (1024). get_sqe() возвращает NULL при исчерпании. submit_request() не проверяет результат: sqe->fd = cdev_fd;, ((ring_data_t*)sqe->user_data).

Триггер. Маппинг ublk-устройства с --queue_depth > 1024 (документированная опция; типично для высокопроизводительных NVMe-class устройств).

Последствия. Крах vitastor-ublk по NULL-deref (SIGSEGV) при старте, сразу после добавления kernel-устройства → полуинициализированный /dev/ublkbN, отказ в обслуживании при валидной конфигурации.

Исправление. Проверять get_sqe() на NULL (сабмитить и ждать освобождения SQE), либо создавать кольцо размером ≥ queue_depth.

A-22 ? read_blocking: определение EOF по устаревшему errno → abort процесса или зависание

  • Файл: src/util/rw_blocking.cpp:18-31
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. read() == 0 (EOF) не меняет errno. read_blocking входит в ветку r <= 0 и решает про EOF по if (!errno). errno хранит произвольное значение от прошлого syscall. Если 0 — вернёт short count; если EINTR/EAGAIN/EPIPE — continue → снова 0 → бесконечный цикл; иначе — perror("read"); exit(1). (Ср. read_all_fd() в str_util.cpp, корректно проверяющий !r.)

Триггер. disk_tool_meta.cpp читает метаданные read_blocking(meta_fd, hdr, meta_block_size); на усечённом/битом образе read возвращает частичный счётчик, затем 0.

Последствия. Short-read на EOF недетерминированно роняет disk_tool/клиент exit(1) или зацикливает, в зависимости от постороннего значения errno.

Исправление. Сначала проверять счётчик байт (if (r == 0) → EOF), а не errno.

A-23 ? OOB read в цикле rescue, когда kv_block_size не делит 1 МиБ-буфер

  • Файл: src/kv/kv_db.cpp:601-618
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. kv_rescue_t читает индекс чанками 1 МиБ (1048576 = 2²⁰), итерирует blk.parse(pos+cur_offset, buf.data()+cur_offset, kv_block_size, true) пока cur_offset < cur_size, шаг += kv_block_size. На полном чанке (cur_size == 1048576), если kv_block_size не делит 2²⁰, наибольшее кратное < 1048576 ещё проходит cur_offset < cur_size, но cur_offset + kv_block_size > 1048576 → parse() читает за концом буфера. open() требует лишь, чтобы kv_block_size делил ino_block_size (data_block_size*pg_data_size), но не 1048576. EC-пул (напр. pg_data=3, data_block_size=128K → ino_block_size=393216) допускает kv_block_size=12288, не делящий 1048576.

Триггер. rescue() БД с kv_block_size, не делящим 1048576 (легально при EC).

Последствия. OOB-чтение за heap-буфером на каждом полном чанке во время disaster recovery → крах (ASan) или мусор, разобранный как блок.

Исправление. Ограничивать parse() длиной min(kv_block_size, cur_size - cur_offset), либо требовать делимость 1048576.

A-24 ? fromhexstr читает за пределами исходной строки, если hex короче 2*bytes

  • Файл: src/disk_tool/disk_tool_utils.cpp:36-45
  • Категория: A
  • Верификация: 3/3 скептиков подтвердили

Механизм. Граница цикла i < from.size() && i < bytes, но тело обращается к from[2*i] и from[2*i+1]. На последней итерации i = min(from.size(), bytes)-1 индекс 2*i+1 может достигать 2*from.size()-1 — далеко за концом строки. Проверка if (x<0||y<0) break; стоит ПОСЛЕ OOB-чтения.

Триггер. Любой путь write-meta/write-journal с укороченным hex-полем ("bitmap", "data", "block_csums", "ext_bitmap") в JSON-дампе (disk_tool_journal.cpp:534-582, disk_tool_meta.cpp:570-853).

Последствия. Heap OOB-чтение по JSON-вводу при restore метаданных/журнала → крах (ASan/hardened-аллокатор) или тихое чтение соседних байт → запись неверных байт в реконструированные метаданные.

Исправление. Условие 2*i+1 < from.size() (и i < bytes).

Отклонённые false-positive

Эти кандидаты были предложены искателями, но отклонены скептиками (приведена причина FP).

FP-1 — delete для буфера, выделенного через malloc_or_die (defrag)

  • Файл: src/nfs/nfs_kv_defrag.cpp:44-51
  • Голоса: 1/3 (отклонён)

Заявка. Буфер, выделенный malloc_or_die (т.е. malloc), освобождается через delete → потенциальная порча кучи.

Почему FP. В кодовой базе нет перегруженного глобального operator new/operator delete нигде в src/. Стандартный operator delete форвардит в free(), а malloc_or_die использует обычный malloc без спец-выравнивания. Заявленная порча кучи требует инструментированного/кастомного аллокатора, который не линкуется. Drop-in аллокаторы (tcmalloc/jemalloc) заменяют malloc и operator new/delete согласованно, поэтому тоже не ломают. Это латентный hygiene/portability-нит (стоило бы использовать free(block_buf)), но не продакшен-баг. Наблюдаемый сбой ограничен ASan-сборками.

FP-2 — resize_init считает new_clean_entry_bitmap_size без округления

  • Файл: src/disk_tool/disk_tool_resize.cpp:219
  • Голоса: 0/3 (отклонён)

Заявка. new_clean_entry_bitmap_size вычисляется без округления вверх и расходится с системным размером bitmap.

Почему FP. Триггер недостижим при валидных конфигурациях. data_block_size обязан быть степенью двойки (blockstore_disk.cpp:125), кратной bitmap_granularity (строка 173, значит тоже степень двойки), и отношение ≥8 (строка 177). Следовательно data_block_size/bitmap_granularity — всегда степень двойки ≥8, т.е. всегда кратно 8 → floor и ceiling дают одинаковый результат, и new_clean_entry_bitmap_size == dsk.clean_entry_bitmap_size для всех достижимых конфигураций. Примеры искателя (16KiB/4KiB → отношение 4, отклоняется строкой 177; 48KiB → не степень двойки, отклоняется строкой 125) невалидны и отбраковываются на инициализации OSD. Floor-форма намеренно используется в эквивалентных вычислениях (osd.cpp:178, disk_simple_offsets.cpp:130). Косметическая несогласованность, не баг.

Рекомендованный порядок устранения

  1. Сетевые NFS (B-1, B-2, B-3, B-4) — удалённая порча памяти без аутентификации. Переполнения длины/счётчика в XDR (B-1, B-2) — системные: чинить len_pad4 и xdr_array 64-битной/overflow-safe арифметикой централизованно.
  2. Тихое разрушение данных (A-1 BLKDISCARD, A-2 EC-scrub overflow, A-5/A-6 порча индекса метаданных) — порча/удаление данных клиента при штатном старте/scrub.
  3. Крах/доступность (A-4, A-7, A-8, A-10, A-21) и утечки (A-9, A-18, A-20).

Примечание: все находки подтверждены агентами (каждый скептик самостоятельно читал код), но перед мерджем правок рекомендуется, чтобы мейнтейнер сверил каждую с задуманными инвариантами — отдельные (A-12 при pg_size>31, A-23 при выборе block_size) зависят от диапазонов конфигурации, специфичных для вашего деплоя.

# Аудит ошибок Vitastor (SDS) — отчёт > **Метод:** 19 агентов-«искателей» прошлись по всему пути данных (blockstore + heap-метаданные, OSD: RMW/EC/peering/scrub/primary, клиент: cluster_client/messenger/RDMA/NBD/ublk, etcd-клиент, NFS: XDR/KV/proxy/portmap, KV DB, util, disk_tool). Каждый кандидат независимо перепроверялся **тремя скептиками** с разными линзами (поток управления / время жизни памяти / достижимость триггера). Баг попадал в отчёт только при подтверждении ≥2/3. > > **Итог:** рассмотрено 30 кандидатов, **28 подтверждено (все — 3/3)**, **2 отклонено как false-positive**. > > Баги разделены на две категории: > - **Категория A — проявляются сами по себе** при штатной работе (старт, flush, scrub, peering, конфигурация, утечки, повреждённые на диске данные). > - **Категория B — провоцируются внешним/удалённым вводом** (remote OOB/overflow по сети из NFS/RPC-клиентов). > > Раздел в конце — **отклонённые false-positive** с обоснованием. --- ## Сводка | ID | Severity | Категория | Файл | Тип | |----|----------|-----------|------|-----| | B-1 | ? critical | B (remote) | `nfs/proto/xdr_impl_inline.h:94-97` | OOB read (~4 ГБ) | | B-2 | ? critical | B (remote) | `nfs/proto/xdr_impl_inline.h:318-333` | heap overflow | | B-3 | ? critical | B (remote) | `nfs/nfs_portmap.cpp:134` | heap overflow | | B-4 | ? critical | B (remote/клиент) | `nfs/nfs_kv_write.cpp:430-439` | OOB / abort | | A-1 | ? critical | A | `blockstore/blockstore_disk.cpp:529-557` | logic → BLKDISCARD по живым данным | | A-2 | ? critical | A | `osd/osd_rmw.cpp:1190-1219,1341` | heap overflow | | A-3 | ? critical | A | `disk_tool/disk_tool_meta.cpp:929-937` | unsigned underflow → OOB | | A-4 | ? critical | A | `blockstore/v1/read.cpp:175,246-247` | NULL-deref / wild memset | | A-5 | ? high | A | `blockstore/blockstore_heap.cpp:560-569` | `&&` вместо `\|\|` | | A-6 | ? high | A | `blockstore/blockstore_impl.cpp:348` | memmove: count вместо bytes | | A-7 | ? high | A | `client/cluster_client.cpp:347-369` | use-after-free | | A-8 | ? high | A | `client/etcd_state_client.cpp:1240-1255` | разыменование end()-итератора | | A-9 | ? high | A | `client/msgr_send.cpp:300-305` | утечка памяти → OOM | | A-10 | ? high | A | `osd/osd_primary_chain.cpp:52-64,85` | assert-abort (crash-loop) | | A-11 | ? high | A | `osd/osd_scrub.cpp:630-635` | OOB read + порча метаданных | | A-12 | ? high | A | `osd/osd_peering_pg.cpp:162-164` | сдвиг — UB | | A-13 | ? high | A | `disk_tool/disk_tool_journal.cpp:507-535` | heap overflow (guard = no-op) | | A-14 | ? high | A | `kv/kv_db.cpp:234-243` | integer overflow → OOB read | | A-15 | ? high | A | `util/rw_blocking.cpp:60-93` | busy-loop на EOF | | A-16 | ? medium | A | `blockstore/v1/read.cpp:364-370` | OOB read iovec-массива | | A-17 | ? medium | A | `blockstore/v1/read.cpp:682-689,947-963` | утечка блока (loc==0) | | A-18 | ? medium | A | `osd/osd_primary_write.cpp:247-251` | утечка subops (ENOSPC) | | A-19 | ? medium | A | `osd/osd_peering.cpp:421-457` | logic → PG застревает в STOPPING | | A-20 | ? medium | A | `client/nbd_proxy.cpp:74-78` | утечка fd (free vs nl_socket_free) | | A-21 | ? medium | A | `client/ublk_server.cpp:717-744` | NULL-deref (get_sqe) | | A-22 | ? medium | A | `util/rw_blocking.cpp:18-31` | EOF по устаревшему errno | | A-23 | ? medium | A | `kv/kv_db.cpp:601-618` | OOB read (rescue) | | A-24 | ? medium | A | `disk_tool/disk_tool_utils.cpp:36-45` | OOB read строки | FP-1, FP-2 — см. раздел «Отклонённые false-positive». --- # Категория B — провоцируются внешним/удалённым вводом > Класс наибольшего риска: достижимы по сети от NFS/RPC-клиентов, часть — без аутентификации. Если NFS-прокси/портмаппер открыт наружу, это первый приоритет. --- ## B-1 ? Удалённый OOB-read (~4 ГБ): переполнение в `len_pad4()` обходит проверку границ XDR-строки/opaque - **Файл:** `src/nfs/proto/xdr_impl_inline.h:94-97, 132, 142-148` - **Категория:** B — нужен crafted-ввод (поле длины `0xFFFFFFFF`) - **Верификация:** 3/3 скептиков подтвердили **Механизм.** В `xdr_bytes()` (декодирование) 32-битная длина с провода читается в `len` и передаётся в `len_pad4()`: ```c uint32_t inline len_pad4(uint32_t len) { return ((len+3)/4) * 4; } ... uint32_t padded = len_pad4(len); if (xdrs->avail < 4+padded) return 0; data->size = len; data->data = (char*)(xdrs->buf+4); ``` Для `len ∈ {0xFFFFFFFD, 0xFFFFFFFE, 0xFFFFFFFF}` выражение `len+3` переполняется в `{0,1,2}`, поэтому `padded == 0`. Проверка `avail < 4+0` проходит для любого буфера ≥4 байт, и `data->size` устанавливается в `len` (~4 ГБ). Параметр `maxlen` на декодировании не проверяется — ничто не ограничивает длину. **Триггер.** Один поддельный NFS/portmap-RPC-запрос с полем строки/opaque (имя в LOOKUP, путь монтирования, file handle, любое `xdr_string`/`xdr_bytes`-поле) с префиксом длины `0xFFFFFFFF`. Любой потребитель (`xdr_string_t::operator std::string()` → `std::string(data, size)`, `operator==`) читает ~4 ГБ за пределами маленького приёмного буфера. **Последствия.** OOB-чтение → падение демона NFS (DoS) или утечка соседней heap-памяти. Без аутентификации, удалённо. **Исправление.** Проверять переполнение до округления: `if (len > xdrs->avail - 4) return 0;` (в 64-битной арифметике) и применять `maxlen` на декодировании. Это системная проблема — чинить централизованно. --- ## B-2 ? Удалённое heap-переполнение: переполнение счётчика элементов в `xdr_array()` → undersized malloc - **Файл:** `src/nfs/proto/xdr_impl_inline.h:318-333` - **Категория:** B — нужен crafted-ввод (счётчик элементов) - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```c *len = be32toh(*((uint32_t*)xdrs->buf)); if (*len > maxlen) return 0; // maxlen == ~0 для ряда массивов *data = (char*)malloc_or_die(entry_size * (*len)); // 32-битное умножение for (uint32_t i = 0; i < *len; i++) fn(xdrs, *data + entry_size*i); ``` Произведение `entry_size * (*len)` считается в 32-битной беззнаковой арифметике и переполняется. Атакующий подбирает `*len` так, чтобы произведение свернулось в малое значение, а сам `*len` остался огромным. Пример: `SETACL3args.ace[]`, `entry_size = sizeof(nfsacl_ace) = 12`, `*len = 0x55555556` → `12*0x55555556 mod 2^32 = 8` байт. Выделяется 8 байт, а цикл пишет 12-байтные записи далеко за их пределы. **Триггер.** Поддельный NFSv3 SETACL (`maxlen=~0`, `entry_size=12`); сотни ACE-записей в пакете → переполнение на тысячи управляемых атакующим байт. Аналогично через `xdr_xdr_write_chunk` (RDMA, `entry_size=16`, `maxlen=~0`) и массивы GETACL/auth_flavors. **Последствия.** Управляемое атакующим heap-переполнение → порча памяти / удалённый крах демона. Потенциально эксплуатируемо. **Исправление.** Проверять `if (*len != 0 && entry_size > SIZE_MAX / *len) return 0;` в 64-битной арифметике до `malloc`, и проверять, что `*len` помещается в оставшийся ввод. --- ## B-3 ? Удалённое heap-переполнение в PMAP3 DUMP: выделяются слоты под указатель, пишутся полные структуры - **Файл:** `src/nfs/nfs_portmap.cpp:134` - **Категория:** B — удалённо; *срабатывает и при штатном монтировании* - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```c pmap3_mapping_list *list = (pmap3_mapping_list*)malloc_or_die( sizeof(pmap3_mapping_list*) * self->reg_ports.size()); // 8 байт на запись! ... list[i] = (pmap3_mapping_list){...}; // пишется полная структура ~64 байта ``` Выделяется `sizeof(указатель)` = 8 байт на запись, но пишется полная `pmap3_mapping_list` (~64 байта: `pmap3_mapping` = 2×u_int + 3×`xdr_string_t`{size_t,char*} = 56 байт + указатель `next`). Соседний `pmap2_dump_proc` (строка 67) корректно использует `sizeof(pmap2_mapping_list)` — это опечатка, лишняя `*`. При 4 всегда регистрируемых сервисах (PMAP_V2, PMAP_V3, NFS_V3, MOUNT_V3): выделяется 32 байта, пишется 4×64 = 256 байт + `list[i-1].next = NULL`. **Триггер.** Любой клиент, шлющий PMAP v3 DUMP на порт 111 — в т.ч. `rpcinfo` и стандартный rpcbind-зонд, который Linux NFS-клиенты отправляют при монтировании. Переполнение происходит при *любом* dump-запросе, включая легитимные. **Последствия.** Удалённо провоцируемое heap-переполнение в демоне NFS-прокси → порча соседней heap-памяти, крах демона (DoS для всех NFS-клиентов), потенциальная эксплуатация. **Исправление.** Убрать `*`: `malloc_or_die(sizeof(pmap3_mapping_list) * self->reg_ports.size())`. --- ## B-4 ? Неверная длина зануления хвоста в NFS shared-write - **Файл:** `src/nfs/nfs_kv_write.cpp:430-439` - **Категория:** B — провоцируется последовательностью запросов NFS-клиента; *может возникнуть и при легитимной нагрузке* - **Верификация:** 3/3 скептиков подтвердили **Механизм.** В `nfs_do_shared_write` хвост после новых данных в ветке `has_old==false` заполняется `add_zero(op, st->offset, ...)` — это copy-paste из ведущей области (строки 415-424). Должно быть `st->new_size-(st->offset+st->size)` (как в соседней ветке `has_old`): ```cpp if (st->offset+st->size < st->new_size) { if (has_old) op->iov.push_back(st->aligned_buf+st->offset+st->size, st->new_size-(st->offset+st->size)); else add_zero(op, st->offset, st->proxy->kvfs->zero_block); // должно быть new_size-(offset+size) } ``` В iov попадает меньше байт, чем `op->len` (`=aligned_size`) → `assert(left == 0)` в `cluster_client_t::add_iov` (`src/client/cluster_client.cpp:1232`). **Триггер.** Создать файл через CREATE/MKNOD UNCHECKED с атрибутом размера `N>0` (`nfs_kv_create.cpp:320-324` → `empty=true`, `size=N`), затем маленький WRITE, где `offset+size+header < shared_inode_threshold`. `resume_1` ставит `new_size=N`, срабатывает перевод в shared-путь, `nfs_do_shared_readmodify` ничего не делает (`shared_ino==0`, `aligned_buf==NULL`), и `nfs_do_shared_write` идёт с `has_old==false` и `offset+size<new_size`. **Последствия.** Падение демона NFS-прокси по assert (DoS). С отключёнными assert — короткая запись iov → запись неверной длины/мусора в shared-inode → тихая порча данных файла. **Исправление.** В else-ветке использовать `add_zero(op, st->new_size-(st->offset+st->size), ...)`. --- # Категория A — проявляются сами по себе --- ## A-1 ? `trim_data`: `continue` пропускает `j = i+1` → BLKDISCARD по живым (используемым) блокам данных - **Файл:** `src/blockstore/blockstore_disk.cpp:529-557` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** Параметр-предикат `trim_data` называется `is_free`, но все вызывающие передают предикат *is-used* (`blockstore_impl.cpp:93` `heap->is_data_used(...)`; `disk_tool_discard.cpp:97`, `v1/impl.cpp:97` `data_alloc->get(...)`) — он возвращает true для ЗАНЯТОГО блока. Когда `discard_granularity` делает выровненный диапазон пустым (`range[0] >= range[1]`), выполняется `continue`, который прыгает к инкременту цикла и **пропускает `j = i+1`** (строка 556). Занятый блок `i` не исключается, `j` продолжает указывать в раннюю свободную область. На следующей границе `i2` вычисляется диапазон `[j, i2)`, охватывающий занятый блок, и по нему выдаётся `BLKDISCARD`. ```c if (range[0] >= range[1]) continue; // ← пропускает j = i+1 ниже range[1] -= range[0]; r = ioctl(data_fd, BLKDISCARD, &range); ... j = i+1; ``` **Триггер.** `discard_granularity` больше/невыровнен относительно квалифицирующего свободного прогона. Путь выполняется на живом OSD при старте с `discard_on_start=true` (`blockstore_impl.cpp:91-93`) и в `vitastor-disk discard`. **Последствия.** `BLKDISCARD` по диапазону с занятыми блоками безвозвратно зануляет данные клиента → тихая потеря/порча данных тома. **Исправление.** Выполнять `j = i+1` всегда (вынести из-под `continue`), либо заменить `continue` на присваивание `j` перед ним. --- ## A-2 ? Heap-переполнение в `ec_check_combination`: буфер размером по `stripe_count`, индексация по `role` - **Файл:** `src/osd/osd_rmw.cpp:1190-1219, 1341` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `ec_find_good()` выделяет `tmp_buf = malloc_or_die(stripe_count*(chunk_size+bitmap_size))`, где `stripe_count` — число реально прочитанных чанков. `ec_check_combination()` индексирует по РОЛИ `i ∈ [0, pg_size)`: ```c uint8_t *tmp_buf = (uint8_t*)malloc_or_die(stripe_count*(chunk_size+bitmap_size)); ... bs.read_buf = bs.write_buf = tmp_buf + i*chunk_size; bs.bmp_buf = tmp_buf + stripe_count*chunk_size + i*bitmap_size; ``` При `stripe_count < pg_size` роли `i ∈ [stripe_count, pg_size)` дают указатели за пределами выделенной области, а `reconstruct_stripes_ec()`/`calc_rmw_parity_ec()` пишут туда полный `chunk_size` данных и `bitmap_size`. **Триггер.** Scrub деградированного EC-объекта. `submit_scrub_subops` строит массив stripes из `object_state->osd_set` — по одной записи на присутствующий чанк, так что `stripe_count = число присутствующих < pg_size`. `ec_find_good` входит в перебор при `live_roles > pg_minsize`. Напр. EC 3+2 (`pg_size=5`, `pg_minsize=3`) с одним отсутствующим чанком: 4 живые роли > 3 → перебор с `stripe_count=4 < pg_size=5`. **Последствия.** Запись полного чанка (обычно 128 КБ) + bitmap за концом heap-аллокации при scrub любого деградированного EC-объекта. Деградированные PG — штатное состояние, scrub автоматический → достижимый крах / порча памяти OSD. **Исправление.** Выделять буфер по `pg_size`, а не `stripe_count`: `malloc_or_die(pg_size*(chunk_size+bitmap_size))` и согласовать смещение bitmap-региона. --- ## A-3 ? Underflow беззнакового счётчика в обработке rollback при write-meta → бесконечный цикл и OOB read - **Файл:** `src/disk_tool/disk_tool_meta.cpp:929-937` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```cpp for (size_t i = jbo.size()-1; i >= 0; i--) // i — size_t, i >= 0 всегда истинно { if (jbo[i]["version"].uint64_value() > rollback_to) jbo.erase(jbo.begin()+i, jbo.begin()+i+1); } ``` `i` беззнаковый, `i >= 0` всегда истинно. (1) Пустой `jbo` → `jbo.size()-1 == SIZE_MAX` → `jbo[SIZE_MAX]` сразу. (2) Непустой — после `i==0` `i--` сворачивается в `SIZE_MAX` → `jbo[SIZE_MAX]["version"]`. Цикл не завершается нормально. **Триггер.** `vitastor-disk write-meta` при конвертации старого формата (v1/v2) metadata+journal JSON-дампа в heap-формат, если журнал содержит хоть одну запись типа `"rollback"`. **Последствия.** Крах (SIGSEGV) / UB с чтением далеко за границами вектора во время конвертации метаданных оператором; инструмент не может завершиться, при повреждённом индексе может записать мусорные метаданные перед крахом. **Исправление.** `for (size_t i = jbo.size(); i-- > 0; )` либо знаковый индекс. --- ## A-4 ? Wild memset через NULL `read_buf` в `fill_partial_checksum_blocks` из flusher - **Файл:** `src/blockstore/v1/read.cpp:175, 246-247` (вызов из `v1/flush.cpp:1089-1092`) - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `scan_dirty()` вызывает `fill_partial_checksum_blocks()` с `read_buf = NULL`, но функция безусловно делает `read_buf -= read_offset;` (read.cpp:175), где `read_offset = v[0].offset`, округлённый вниз до `csum_block_size`, > 0 если первый перезаписываемый блок не на смещении 0. NULL превращается в `(uint8_t*)(0 - read_offset)`. Дальше `read_range_fulfilled()` делает `if (read_buf) memset(read_buf + el.offset, 0, el.len);` — guard на NULL уже не срабатывает, и memset пишет нули по адресу около нуля. ```c read_buf -= read_offset; // NULL → (uint8_t*)(0 - read_offset) ... if (read_buf) // guard уже не NULL memset(read_buf + el.offset, 0, el.len); ``` **Триггер.** Flush частично перезаписанного checksum-объекта (csum_block_size > bitmap_granularity, запись не выровнена по csum-блоку, первый патчируемый блок на offset>0, старый объект имеет дыры). **Последствия.** Запись нулей по адресу около NULL → SIGSEGV в основном пути flush; если адрес замаплен — тихая порча памяти. Перемежающийся крах storage-демона. **Исправление.** Не вычитать `read_offset`, если `read_buf == NULL`; держать NULL до самого memset-guard. --- ## A-5 ? `finish_load`: одна `insert_list_items` для разных объектов (`&&` вместо `||`) - **Файл:** `src/blockstore/blockstore_heap.cpp:560-569` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `finish_load()` сортирует `postponed_items` по (inode ASC, stripe ASC, lsn DESC) и должен нарезать на пакеты по одному объекту. `insert_list_items()` разрешает голову списка ровно один раз из `v[0]` (`inode_map_get(..., wr->stripe)`) и сливает все остальные элементы в эту цепочку только по LSN (`is_before`). Значит, пакет ОБЯЗАН содержать один объект (одинаковые inode И stripe). Но граница задана так: ```cpp if (e >= n || postponed_items[e]->entry.inode != postponed_items[s]->entry.inode && postponed_items[e]->entry.stripe != postponed_items[s]->entry.stripe) { insert_list_items(postponed_items.data()+s, e-s, false); s = e; } ``` По приоритету (`!=` > `&&` > `||`) это `e>=n || (inode_differs && stripe_differs)` — новый пакет начинается, только когда РАЗЛИЧАЮТСЯ ОБА. Для частого случая «один inode (образ) владеет многими stripe-объектами» разбиения нет → записи разных stripe сливаются в первую цепочку, а головы остальных объектов не регистрируются в inode-map. **Триггер.** Старт/загрузка metadata-стора, где ≥2 объекта накопили глубокую историю версий (`postpone` срабатывает при пропуске >`POSTPONE_INSERT_COUNT`=10 записей того же объекта — норма при тяжёлой перезаписи до компакции). **Последствия.** Порча in-memory индекса объектов при старте: записи одного объекта вклеиваются в цепочку другого, головы части объектов не попадают в inode-map. `read_entry()`/`list_objects()` возвращают версии/локации чужого объекта (тихая порча данных) или NULL для существующих (потеря данных); кривые цепочки роняют `assert()` в `unlink_list_item`/`remove_list_item`/`mark_garbage`. **Исправление.** Заменить `&&` на `||`: новый пакет, когда различается inode ИЛИ stripe. --- ## A-6 ? `process_list`: `memmove` использует число элементов как число байт (в 24 раза меньше) - **Файл:** `src/blockstore/blockstore_impl.cpp:348` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```cpp if (stable_count > op->list_stable_limit) { memmove(result + op->list_stable_limit, result + stable_count, unstable_count); stable_count = op->list_stable_limit; } ``` `result` — `obj_ver_id*`, `sizeof(obj_ver_id)=24` (`object_id`{inode,stripe}=16 + version=8). Указатели масштабируются арифметикой, но размер `memmove` — `unstable_count` (число элементов) вместо `unstable_count * sizeof(obj_ver_id)`. Копируется ~1/24 данных, остальные unstable-слоты сохраняют мусор от отброшенных stable-записей. `retval = limit + unstable_count`, поэтому вызывающий читает мусор как unstable-объекты. **Триггер.** `BS_OP_LIST` с `list_stable_limit` (scrub/recovery), когда stable-объектов в PG больше лимита И есть unstable-объекты. **Последствия.** Scrub/recovery работает по неверным (inode,stripe,version) → неверные решения о целостности → возможна порча данных. **Исправление.** `memmove(..., unstable_count * sizeof(obj_ver_id));` --- ## A-7 ? Use-after-free в `continue_ops()`: закэшированный `next_op` может быть освобождён реентрантным завершением - **Файл:** `src/client/cluster_client.cpp:347-369` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `continue_ops()` кэширует `next_op = op->next` до прогона `op` через `continue_rw()`/`continue_sync()`, затем безусловно `op = next_op` и перечитывает `op->next`. Прогон `op` может синхронно завершить и освободить `next_op`. ```cpp for (auto op = op_queue_head; op; ) { cluster_op_t *next_op = op->next; ... if (!op->retry_after && !op->prev_wait) { if (op->opcode == OSD_OP_SYNC) continue_sync(op); else continue_rw(op); } op = next_op; if (continuing_ops == 2) goto restart; // никогда не срабатывает: continuing_ops ∈ {0,1} } ``` Сценарий: WRITE (не immediate-commit) + следующий за ним SYNC, ждущий WRITE. WRITE ретраится по таймеру; на ретрае `try_send()` → `TRY_SEND_OFFLINE`, истёк `OSD_OP_WAIT_UP_TIMEOUT` → `-ETIMEDOUT` → `erase_op(WRITE)` → `inc_wait` → `continue_sync(SYNC)`; `dirty_osds` пуст → SYNC стирается, его callback удаляет `cluster_op_t`. Возврат в цикл → `op = next_op` указывает на освобождённый SYNC. Защита `continuing_ops == 2` мертва (присваивание убрано в коммите f8ff39b0). **Последствия.** UAF `cluster_op_t` в I/O-пути клиента: разыменование освобождённой операции, возможный повторный прогон уже завершённого SYNC → крах клиента (QEMU/NBD/fio/vitastor-cli) или двойной callback. **Исправление.** Восстановить корректную перезагрузку обхода (`continuing_ops=2`/`goto restart`) после реентрантного завершения, либо проверять валидность перед `op = next_op`. --- ## A-8 ? Разыменование end()-итератора при дублирующихся именах inode - **Файл:** `src/client/etcd_state_client.cpp:1240-1255` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```cpp auto it = this->inode_config.find(inode_num); if (it != this->inode_config.end() && it->second.name != "") { auto n_it = this->inode_by_name.find(it->second.name); if (n_it->second == inode_num) // ← n_it может быть end() this->inode_by_name.erase(n_it); ``` Инвариант «`inode_config[x].name != ""` ⇒ `inode_by_name` содержит это имя» нарушается при двух inode с одинаковым именем. Последовательность: A с именем "foo" → B с именем "foo" (перезаписывает `inode_by_name["foo"]=B`, но `inode_config[A].name` остаётся "foo") → удаление B (стирает `inode_by_name["foo"]`) → удаление/обновление A: `find("foo")` возвращает `end()`, `n_it->second` разыменовывает `end()`. **Триггер.** Дубль имён образов в etcd (переходно при rename или из-за мисконфигурации) + определённый порядок удаления/обновления. **Последствия.** SIGSEGV/UB в любом демоне (клиент QEMU/fio, OSD), читающем конфигурацию inode из etcd → кластерный простой I/O. **Исправление.** Проверять `n_it != inode_by_name.end()` перед разыменованием. --- ## A-9 ? `zc_free_list`: NULL-маркер не удаляется для OUT-only zerocopy-отправок → неограниченный рост памяти - **Файл:** `src/client/msgr_send.cpp:300-305` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** На каждую zerocopy-отправку приходят два завершения (more=true, затем prev=true F_NOTIF). На первом безусловно добавляется NULL-маркер в `zc_free_list`. На втором батч освобождается: ```cpp int i = 0; for (; i < cl->zc_free_list.size() && cl->zc_free_list[i]; i++) delete cl->zc_free_list[i]; if (i > 0) cl->zc_free_list.erase(cl->zc_free_list.begin(), cl->zc_free_list.begin()+i+1); ``` `MSGR_SENDP_FREE` ставится только для `OSD_OP_IN`-ответов; `OSD_OP_OUT` его не несёт. Для батча только из OUT-операций список = `[NULL]`, `i` остаётся 0, `if (i > 0)` ложно — маркер не удаляется. Каждая такая отправка добавляет один NULL навсегда. **Триггер.** Исходящие OSD↔OSD соединения шлют репликационные записи (`OSD_OP_SEC_WRITE`/`_STABLE`, все OUT). Большие записи (≥ `min_zerocopy_send_size`, дефолт 32 КБ) идут zerocopy-путём при поддержке `sendmsg_zc`. **Последствия.** На каждую большую zerocopy-запись `zc_free_list` растёт на один элемент и не сжимается → неограниченная утечка → OOM на write-heavy OSD за длительное аптайм. **Исправление.** Удалять одиночный NULL-маркер и при `i == 0` (стирать `[begin, begin+1)`). --- ## A-10 ? Ретрай chained-read на EIO/EDOM оставляет `op_data->st` устаревшим → assert-abort - **Файл:** `src/osd/osd_primary_chain.cpp:52-64, 85` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** При ошибке чтения/контрольной суммы на subop chained-read `op_data->st` доходит до 4. На `-EIO`/`-EDOM` код делает `check_corrupted_chained()`, освобождает буферы и `goto resume_1` **без сброса `op_data->st`** (он всё ещё 4). В `read_bitmaps()` `st` не равен ни `base_state`(1), ни `base_state+1`(2) → `assert(op_data->st < base_state)` = `assert(4 < 1)` → abort. Сборка релиза снимает `-DNDEBUG` (`src/CMakeLists.txt:43-51`), assert живой в продакшене. ```cpp goto resume_1; // op_data->st всё ещё 4 ... assert(op_data->st < base_state); // assert(4 < 1) → abort ``` **Триггер.** Один битый сектор (`-EIO`) или несовпадение контрольной суммы (`-EDOM`) при chained-read снапшота/клона (inode с `parent_id`). **Последствия.** Крах primary OSD по assert вместо ошибки/реконструкции. Объект остаётся повреждённым → повторные клиентские чтения снова роняют OSD (crash-loop) → недоступность всех PG этого OSD. **Исправление.** Перед `goto resume_1` выставлять `op_data->st = 0`. --- ## A-11 ? EC-scrub индексирует `stripes[]` по `chunk.role` вместо индекса stripe → OOB read + порча метаданных - **Файл:** `src/osd/osd_scrub.cpp:630-635` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `op_data->stripes[]` имеет размер `stripe_count` = число НЕ-outdated чанков, заполняется последовательно (`stripes[i].role = chunk.role`), индекс не связан с номером роли. Но код обращается к `op_data->stripes[chunk.role]`, где `chunk.role` — до `pg_size-1` (до 255 для EC): ```cpp else if (op_data->stripes[i].read_end > 0 && !op_data->stripes[chunk.role].missing && (chunk.loc_bad & LOC_CORRUPTED)) { ... } ``` При наличии outdated-чанка `stripe_count < число ролей` → `chunk.role >= stripe_count` → OOB-чтение heap-массива; даже в границах читается `.missing` чужого stripe. **Триггер.** Scrub EC/XOR-объекта с outdated-копией (`stripe_count` уменьшен) + чанк, ранее помеченный `LOC_CORRUPTED`, чья роль ≥ `stripe_count`, и который теперь читается корректно. **Последствия.** OOB-чтение (крах на границе страницы) и чтение мусорного `.missing` → неверное решение о снятии `LOC_CORRUPTED` → primary может читать действительно битый чанк (выдача неверных данных) или отбросить хороший. **Исправление.** Использовать `op_data->stripes[i].missing` вместо `[chunk.role]`. --- ## A-12 ? Маска `n_roles`: 32-битный `1 << replica` для EC-ролей до 255 → UB сдвига - **Файл:** `src/osd/osd_peering_pg.cpp:162-164` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```cpp unsigned replica = (list[list_pos].oid.stripe & STRIPE_MASK); ... if (!(has_roles & (1 << replica))) { has_roles = has_roles | (1 << replica); n_roles++; } ``` `1` — знаковый `int`, `replica` до `pg_size-1` (EC допускает до 256, `etcd_state_client.cpp:868`). `1 << replica` при `replica >= 31` — UB (сдвиг ≥ ширины/в знаковый бит); даже с `(uint64_t)1` роли ≥64 не представимы. `n_roles` (число присутствующих чанков) считается неверно при `pg_size > 31`. **Триггер.** Любой EC-пул с `pg_size > 31` (напр. 30+3) на peering/расчёте состояний объектов. **Последствия.** На широких EC-пулах решение complete/degraded/incomplete считается по испорченной маске → завершённые объекты помечаются incomplete (отказ чтений/записей, зависание I/O) или деградированные — complete (чтение по недостаточному набору чанков). Плюс формальный UB. **Исправление.** `(uint64_t)1 << replica` и хранилище шире 64 бит (напр. `std::bitset`/массив) при `pg_size > 64`. --- ## A-13 ? `write_json_journal`: проверка границ читает `small_write.len` до присваивания → heap overflow - **Файл:** `src/disk_tool/disk_tool_journal.cpp:507-535` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `ne` указывает в обнулённый `new_journal_buf`. Проверка ```cpp if (new_journal_data - new_journal_buf + ne->small_write.len > new_journal_len) { ... return 1; } ``` выполняется ДО заполнения структуры (`*((journal_entry_small_write*)ne) = {... .len = rec["len"] ...}` идёт позже, строки 516-530). Поэтому `ne->small_write.len` читается как 0 → проверка всегда проходит. Затем `fromhexstr(rec["data"]..., ne->small_write.len, new_journal_data)` пишет реальные `len` байт, а guard, который должен был это поймать, считался против 0. **Триггер.** `vitastor-disk write-journal` из JSON-дампа, где суммарные данные small-write превышают `new_journal_len`. **Последствия.** Heap-overflow при реконструкции журнала; защита-no-op → порча соседней памяти, крах или запись повреждённого журнала на диск. **Исправление.** Вычислять длину из `rec["len"]` в локальную переменную и проверять её до записи. --- ## A-14 ? Integer overflow в `read_string()` → OOB read - **Файл:** `src/kv/kv_db.cpp:234-243` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** ```cpp uint32_t len = *(uint32_t*)(data+*pos); *pos += sizeof(uint32_t); if (*pos+len > size) { *pos = -1; return ""; } std::string key((char*)data+*pos, len); ``` `*pos` — `int`, `len` — `uint32_t`: сложение в 32-битной беззнаковой арифметике. При `len ≈ 0xFFFFFFFF` `*pos+len` сворачивается в малое значение → проверка проходит → `std::string(data+pos, ~4 ГБ)` OOB read. **Триггер.** Повреждённый/torn KV-блок с целыми заголовками (magic/block_size/type/items), но испорченным полем длины ключа/значения. Достижимо из `get_block`, `find_size`, `rescue`. **Последствия.** Многогигабайтное OOB-чтение в `kv_block_t::parse()` → SIGSEGV вместо чистого `-EILSEQ`. Восстановимое состояние битого блока превращается в крах демона. **Исправление.** Проверять `if (len > (uint32_t)(size - *pos)) ...` либо в 64-битной арифметике. --- ## A-15 ? `readv_blocking`/`writev_blocking`/`sendv_blocking`: бесконечный цикл на EOF - **Файл:** `src/util/rw_blocking.cpp:60-93` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** Ошибкой считается только `r < 0`. `r == 0` (peer закрыл соединение/EOF до доставки всех байт iov) не обрабатывается: `done += 0`, внутренний цикл попадает на первый iov с `iov_len > 0`, берёт ветку `iov_len > r` (т.е. > 0), вычитает 0, сдвигает базу на 0 и `break` БЕЗ инкремента `v`. Внешний `while (v < iovcnt)` истинен → `readv()` снова возвращает 0 → 100% CPU навсегда. **Триггер.** `fio_sec_osd.cpp:377` `readv_blocking(connect_fd, ...)` читает payload ответа OSD; если OSD упал/закрыл соединение после заголовка — следующий `readv` возвращает 0. **Последствия.** Отключение удалённого пира превращается в неубиваемый busy-loop (зависание на 100% CPU) вместо чистой ошибки. **Исправление.** Обрабатывать `r == 0` как EOF (возврат/ошибка), а не как «продолжить». --- ## A-16 ? OOB read iovec-массива в `read_checksum_block` (батчинг IOV_MAX) - **Файл:** `src/blockstore/v1/read.cpp:364-370` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** Выделяется массив ровно из `n_iov` iovec, подаётся батчами по `IOV_MAX`. Для каждого батча (кроме одиночного первого чанка) сумма длин считается `for (int i = 0; i < IOV_MAX; i++) d_len += iov[n_pos+i].iov_len;`. На последнем батче, когда `n_iov > IOV_MAX` и не кратно, реальных записей `n_cur = n_iov-n_pos < IOV_MAX` → читаются индексы `>= n_iov` за концом буфера. **Триггер.** Очень крупные/фрагментированные checksum-block-чтения, дающие > IOV_MAX (обычно 1024) сегментов. **Последствия.** Heap OOB-чтение за аллокацией + неверный `data->iov.iov_len` → `handle_read_event` ложно помечает чтение ошибкой → ложный short-read/IO-error клиенту. **Исправление.** Ограничить внутренний цикл `min(IOV_MAX, n_iov - n_pos)`. --- ## A-17 ? Ссылка `used_clean_objects` не освобождается для блока данных в локации 0 - **Файл:** `src/blockstore/v1/read.cpp:682-689, 947-963` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `fulfill_clean_read()` сохраняет `PRIV(read_op)->clean_block_used = clean_loc`. Для первого блока данных `clean_loc == 0`: `used_clean_objects[0]` создаётся, `uo.refs++`. Путь освобождения в `handle_read_event` защищён `if (PRIV(op)->clean_block_used)` — ложно при локации 0, refcount не уменьшается, запись не удаляется. **Триггер.** Любое чтение, затрагивающее блок данных в локации 0. **Последствия.** Постоянная (refs>0) запись в `used_clean_objects`. Если flusher позже релоцирует/освободит блок (`was_freed=true`), он не возвращается аллокатору → теряется один блок из free-пула + накопление stale-состояния. **Исправление.** Использовать sentinel (напр. `UINT64_MAX`) для «нет ссылки» вместо 0, либо отдельный bool-флаг. --- ## A-18 ? Утечка массива write-subops при частичном сбое EC-записи (ENOSPC) - **Файл:** `src/osd/osd_primary_write.cpp:247-251` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** При частичном сбое записи (часть subops успешна) код намеренно НЕ удаляет `op_data->subops` (`osd_primary_subops.cpp:491-497`). Для реплик: `mark_partial_write` + `delete[] op_data->subops`. Для EC/XOR: `submit_primary_rollback_subops()` делает `op_data->subops = new osd_op_t[n_subops];`, перезаписывая ещё живой указатель без `delete[]`. Rollback-массив позже освобождается, исходный — утекает. **Триггер.** EC/XOR-запись, частично упавшая с ENOSPC (диск полон на части чанк-OSD). **Последствия.** Утечка одного `new osd_op_t[n_subops]` на каждую такую запись. При disk-full клиент/recovery повторяют записи → утечка растёт именно при дефиците памяти. **Исправление.** Освобождать исходный `op_data->subops` перед переназначением в EC-ветке. --- ## A-19 ? `relock_pg` стирает relock-записи из `diff_osds` без коррекции `relock_osd_count` - **Файл:** `src/osd/osd_peering.cpp:421-457` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `diff_osds` = [relock-записи (первые `relock_osd_count`)] ++ [unlock-записи]. Классификация: `unlock_peer = (i >= relock_osd_count)`. Когда пир из relock-части подключён, но не поддерживает PG-locks, запись стирается `diff_osds.erase(begin()+(i--))` без декремента `relock_osd_count`. Все последующие unlock-записи сдвигаются влево → реальная unlock-запись оказывается на индексе `< relock_osd_count` → классифицируется как relock. **Триггер.** Смешанный по версиям кластер (rolling upgrade), где хотя бы один cur_peer OSD без фичи `pg_locks`, при наличии пиров на unlock. **Последствия.** Пир, который надо разблокировать, записывается в `pg.lock_peers` с ненулевым state. `can_stop()` возвращает false → PG застревает в `PG_STOPPING`, блокируя миграцию/takeover. **Исправление.** Декрементировать `relock_osd_count` при стирании записи из relock-части. --- ## A-20 ? netlink-сокет освобождается через `free()` вместо `nl_socket_free()` - **Файл:** `src/client/nbd_proxy.cpp:74-78` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `netlink_sock_free()` освобождает `struct nl_sock*` (от `nl_socket_alloc()`) через libc `free()` вместо libnl `nl_socket_free()`. `nl_socket_alloc()` открывает netlink-fd (через `genl_connect`→`socket()`) и refcounted `nl_cb`. `free()` освобождает только структуру `nl_sock`, оставляя fd открытым и `nl_cb` утёкшим. (В том же файле строки 59, 66 в путях ошибок корректно вызывают `nl_socket_free`.) **Триггер.** Каждая операция netlink-map/unmap/revive (`netlink_configure()`, `netlink_disconnect()`). **Последствия.** Каждый netlink-map утекает netlink-fd + `nl_cb`. Для долгоживущего прокси fd держится всё время жизни устройства; повторные map/unmap исчерпывают таблицу дескрипторов. **Исправление.** `nl_socket_free(sk)` вместо `free(sk)`. --- ## A-21 ? Непроверенный `get_sqe()` → NULL-deref при `queue_depth` больше размера io_uring-кольца - **Файл:** `src/client/ublk_server.cpp:717-744` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `start_device()` пред-сабмитит `queue_depth` fetch-запросов в цикле, каждый `submit_request()` → `ringloop->get_sqe()`, а `submit()` вызывается ПОСЛЕ цикла — значит все `queue_depth` SQE должны быть живы одновременно. Кольцо создаётся с фиксированным `RINGLOOP_DEFAULT_SIZE` (1024). `get_sqe()` возвращает NULL при исчерпании. `submit_request()` не проверяет результат: `sqe->fd = cdev_fd;`, `((ring_data_t*)sqe->user_data)`. **Триггер.** Маппинг ublk-устройства с `--queue_depth > 1024` (документированная опция; типично для высокопроизводительных NVMe-class устройств). **Последствия.** Крах `vitastor-ublk` по NULL-deref (SIGSEGV) при старте, сразу после добавления kernel-устройства → полуинициализированный `/dev/ublkbN`, отказ в обслуживании при валидной конфигурации. **Исправление.** Проверять `get_sqe()` на NULL (сабмитить и ждать освобождения SQE), либо создавать кольцо размером ≥ `queue_depth`. --- ## A-22 ? `read_blocking`: определение EOF по устаревшему `errno` → abort процесса или зависание - **Файл:** `src/util/rw_blocking.cpp:18-31` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `read() == 0` (EOF) не меняет `errno`. `read_blocking` входит в ветку `r <= 0` и решает про EOF по `if (!errno)`. `errno` хранит произвольное значение от прошлого syscall. Если 0 — вернёт short count; если EINTR/EAGAIN/EPIPE — `continue` → снова 0 → бесконечный цикл; иначе — `perror("read"); exit(1)`. (Ср. `read_all_fd()` в `str_util.cpp`, корректно проверяющий `!r`.) **Триггер.** `disk_tool_meta.cpp` читает метаданные `read_blocking(meta_fd, hdr, meta_block_size)`; на усечённом/битом образе read возвращает частичный счётчик, затем 0. **Последствия.** Short-read на EOF недетерминированно роняет disk_tool/клиент `exit(1)` или зацикливает, в зависимости от постороннего значения `errno`. **Исправление.** Сначала проверять счётчик байт (`if (r == 0) → EOF`), а не `errno`. --- ## A-23 ? OOB read в цикле rescue, когда `kv_block_size` не делит 1 МиБ-буфер - **Файл:** `src/kv/kv_db.cpp:601-618` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** `kv_rescue_t` читает индекс чанками 1 МиБ (1048576 = 2²⁰), итерирует `blk.parse(pos+cur_offset, buf.data()+cur_offset, kv_block_size, true)` пока `cur_offset < cur_size`, шаг `+= kv_block_size`. На полном чанке (`cur_size == 1048576`), если `kv_block_size` не делит 2²⁰, наибольшее кратное `< 1048576` ещё проходит `cur_offset < cur_size`, но `cur_offset + kv_block_size > 1048576` → `parse()` читает за концом буфера. `open()` требует лишь, чтобы `kv_block_size` делил `ino_block_size` (`data_block_size*pg_data_size`), но не 1048576. EC-пул (напр. pg_data=3, data_block_size=128K → ino_block_size=393216) допускает `kv_block_size=12288`, не делящий 1048576. **Триггер.** `rescue()` БД с `kv_block_size`, не делящим 1048576 (легально при EC). **Последствия.** OOB-чтение за heap-буфером на каждом полном чанке во время disaster recovery → крах (ASan) или мусор, разобранный как блок. **Исправление.** Ограничивать `parse()` длиной `min(kv_block_size, cur_size - cur_offset)`, либо требовать делимость 1048576. --- ## A-24 ? `fromhexstr` читает за пределами исходной строки, если hex короче `2*bytes` - **Файл:** `src/disk_tool/disk_tool_utils.cpp:36-45` - **Категория:** A - **Верификация:** 3/3 скептиков подтвердили **Механизм.** Граница цикла `i < from.size() && i < bytes`, но тело обращается к `from[2*i]` и `from[2*i+1]`. На последней итерации `i = min(from.size(), bytes)-1` индекс `2*i+1` может достигать `2*from.size()-1` — далеко за концом строки. Проверка `if (x<0||y<0) break;` стоит ПОСЛЕ OOB-чтения. **Триггер.** Любой путь write-meta/write-journal с укороченным hex-полем ("bitmap", "data", "block_csums", "ext_bitmap") в JSON-дампе (`disk_tool_journal.cpp:534-582`, `disk_tool_meta.cpp:570-853`). **Последствия.** Heap OOB-чтение по JSON-вводу при restore метаданных/журнала → крах (ASan/hardened-аллокатор) или тихое чтение соседних байт → запись неверных байт в реконструированные метаданные. **Исправление.** Условие `2*i+1 < from.size()` (и `i < bytes`). --- # Отклонённые false-positive > Эти кандидаты были предложены искателями, но отклонены скептиками (приведена причина FP). ## FP-1 — `delete` для буфера, выделенного через `malloc_or_die` (defrag) - **Файл:** `src/nfs/nfs_kv_defrag.cpp:44-51` - **Голоса:** 1/3 (отклонён) **Заявка.** Буфер, выделенный `malloc_or_die` (т.е. `malloc`), освобождается через `delete` → потенциальная порча кучи. **Почему FP.** В кодовой базе **нет** перегруженного глобального `operator new`/`operator delete` нигде в `src/`. Стандартный `operator delete` форвардит в `free()`, а `malloc_or_die` использует обычный `malloc` без спец-выравнивания. Заявленная порча кучи требует инструментированного/кастомного аллокатора, который не линкуется. Drop-in аллокаторы (tcmalloc/jemalloc) заменяют `malloc` и `operator new/delete` согласованно, поэтому тоже не ломают. Это латентный hygiene/portability-нит (стоило бы использовать `free(block_buf)`), но не продакшен-баг. Наблюдаемый сбой ограничен ASan-сборками. ## FP-2 — `resize_init` считает `new_clean_entry_bitmap_size` без округления - **Файл:** `src/disk_tool/disk_tool_resize.cpp:219` - **Голоса:** 0/3 (отклонён) **Заявка.** `new_clean_entry_bitmap_size` вычисляется без округления вверх и расходится с системным размером bitmap. **Почему FP.** Триггер недостижим при валидных конфигурациях. `data_block_size` обязан быть степенью двойки (`blockstore_disk.cpp:125`), кратной `bitmap_granularity` (строка 173, значит тоже степень двойки), и отношение ≥8 (строка 177). Следовательно `data_block_size/bitmap_granularity` — всегда степень двойки ≥8, т.е. всегда кратно 8 → floor и ceiling дают одинаковый результат, и `new_clean_entry_bitmap_size == dsk.clean_entry_bitmap_size` для всех достижимых конфигураций. Примеры искателя (16KiB/4KiB → отношение 4, отклоняется строкой 177; 48KiB → не степень двойки, отклоняется строкой 125) невалидны и отбраковываются на инициализации OSD. Floor-форма намеренно используется в эквивалентных вычислениях (`osd.cpp:178`, `disk_simple_offsets.cpp:130`). Косметическая несогласованность, не баг. --- ## Рекомендованный порядок устранения 1. **Сетевые NFS (B-1, B-2, B-3, B-4)** — удалённая порча памяти без аутентификации. Переполнения длины/счётчика в XDR (B-1, B-2) — системные: чинить `len_pad4` и `xdr_array` 64-битной/overflow-safe арифметикой централизованно. 2. **Тихое разрушение данных (A-1 BLKDISCARD, A-2 EC-scrub overflow, A-5/A-6 порча индекса метаданных)** — порча/удаление данных клиента при штатном старте/scrub. 3. **Крах/доступность (A-4, A-7, A-8, A-10, A-21)** и **утечки (A-9, A-18, A-20)**. > Примечание: все находки подтверждены агентами (каждый скептик самостоятельно читал код), но перед мерджем правок рекомендуется, чтобы мейнтейнер сверил каждую с задуманными инвариантами — отдельные (A-12 при pg_size>31, A-23 при выборе block_size) зависят от диапазонов конфигурации, специфичных для вашего деплоя.
kamillaova changed title from Memory/logic vulnerabilities review using Opus 4.6 w/ ultracode to Memory/logic vulnerabilities review using Opus 4.8 w/ ultracode 2026-05-29 15:55:39 +03:00
vitalif commented 2026-05-31 12:04:39 +03:00
Owner
Copy Link

A-1 ? trim_data: continue пропускает j = i+1 → BLKDISCARD по живым (используемым) блокам данных

Вот это прокомментирую, второй раз уже, короче там всё корректно только параметр называется is_free а должен называться is_used, это смущает всех ИИшек) - переименовал

>A-1 ? trim_data: continue пропускает j = i+1 → BLKDISCARD по живым (используемым) блокам данных Вот это прокомментирую, второй раз уже, короче там всё корректно только параметр называется is_free а должен называться is_used, это смущает всех ИИшек) - переименовал
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
vitalif
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vitalif/vitastor#74
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?