Data integrity review using AI (gpt-5.5) #66

Closed
opened 2026-05-11 05:04:39 +03:00 by maxpain · 4 comments

Аудит Vitastor на риски потери и повреждения данных

Дата аудита: 2026-05-11
Ревизия: caa70317faab41f92b0fd017f4437da5b2386a40
Фокус: сценарии, в которых production-кластер может подтвердить запись/delete/sync, а затем потерять данные, воскресить удаленные данные или закрепить неверный EC-набор как правильный.

Это read-only аудит кода. Я не запускал разрушительные тесты на реальном кластере. Ниже перечислены только находки, которые выглядят релевантными для durability/data-loss/data-corruption. Часть пунктов требует минимального воспроизводящего теста, но несколько нарушений подтверждаются прямо по потоку кода.

Краткое резюме

Самые опасные места:

  • BS_OP_SYNC в blockstore может вернуть успех второму sync до завершения фактического fsync.
  • Клиентский OSD_OP_SYNC очищает dirty_osds до результата и при retryable ошибке может повторно завершиться успешно без повторного sync.
  • Delete на misplaced/extra OSD может не попасть в dirty/sync set; после crash старый объект может воскреснуть.
  • Compaction пишет новые данные в data_fd, но не учитывает их в unsynced_data_write_count; metadata может стать durable раньше данных.
  • В meta_format=3 delete хранится как stable tombstone версии 0, что ломает версионную семантику delete/read/rollback и pg.ver_override.
  • EC scrub/recovery содержит несколько ошибок индексации, из-за которых можно выбрать неправильный "good subset" и затем пометить/восстановить неверные chunks.

DATA-001: BS_OP_SYNC может завершиться до фактического fsync

Серьезность: Critical
Уверенность: высокая
Компонент: src/blockstore

Код:

  • src/blockstore/blockstore_sync.cpp:29-87
  • src/blockstore/blockstore_sync.cpp:101-108
  • src/blockstore/blockstore_impl.cpp:138-142

Проблема:

submit_fsyncs() сбрасывает unsynced_data_write_count, unsynced_buffer_write_count и unsynced_meta_write_count сразу после отправки io_uring_prep_fsync, до completion callback:

  • src/blockstore/blockstore_sync.cpp:84-86

После этого второй BS_OP_SYNC, обработанный в той же очереди до completion первого fsync, видит has_unsynced() == false на src/blockstore/blockstore_sync.cpp:101 и возвращает успех немедленно.

Почему это может привести к потере данных:

  1. Write завершился и увеличил один из unsynced_* счетчиков.
  2. Первый BS_OP_SYNC отправил fsync и тут же обнулил счетчики.
  3. Второй BS_OP_SYNC увидел пустое dirty-состояние и завершился успешно.
  4. Процесс/хост падает до completion первого fsync.
  5. Клиент второго sync уже получил durable ack, но данные могли не попасть на носитель.

Это нарушает базовую семантику fsync: подтвержденный sync обязан переживать crash.

Идея исправления:

  • Не сбрасывать глобальные unsynced_* счетчики до completion всех отправленных fsync.
  • Или ввести поколение/epoch dirty-счетчиков: sync фиксирует поколение, новые writes попадают в следующее, а последующие sync ждут уже летящий fsync, если их требуемое поколение не durable.
  • Все sync, которые пришли после первого и зависят от того же dirty-поколения, должны завершаться только после completion реального fsync.

Regression-test:

  • Instrument/mock io_uring_prep_fsync, чтобы задержать completion.
  • Enqueue: write -> sync1 -> sync2.
  • Проверить, что callback sync2 не вызывается до completion fsync.
  • После simulated crash/drop volatile cache проверить, что подтвержденные данные читаются.

DATA-002: client continue_sync() теряет dirty_osds при retryable ошибке

Серьезность: Critical
Уверенность: высокая
Компонент: src/client

Код:

  • src/client/cluster_client.cpp:1463-1527

Проблема:

cluster_client_t::continue_sync() очищает dirty_osds сразу после отправки sync-подопераций:

  • src/client/cluster_client.cpp:1485-1501

Если один из OSD вернул retryable -EPIPE, -EIO или -ENOSPC, код вызывает wb->fsync_error(), сбрасывает состояние операции и возвращает retry:

  • src/client/cluster_client.cpp:1508-1519

Но список dirty_osds уже потерян. На следующем заходе continue_sync() попадает в fast path:

  • src/client/cluster_client.cpp:1467-1472

и завершает fsync успешно без повторной отправки OSD_OP_SYNC.

Почему это может привести к потере данных:

  1. Lazy/writeback write успешно записан на OSD и помечен как нуждающийся в sync.
  2. Клиент вызывает fsync.
  3. Sync отправляется на dirty OSD, затем dirty_osds.clear().
  4. OSD отвечает retryable ошибкой или временно недоступен.
  5. Клиент повторяет fsync, но dirty set уже пустой, поэтому fsync возвращает success.
  6. Crash OSD/хоста до реального sync приводит к потере записи, которую приложение считало fsync-нутой.

Идея исправления:

  • Держать snapshot dirty OSD внутри операции sync до успешного завершения.
  • Очищать глобальный dirty_osds только после успеха, причем аккуратно: не удалять OSD, которые стали dirty новыми writes во время inflight sync.
  • При retry восстанавливать pending dirty set или повторять тот же snapshot.

Regression-test:

  • Включить lazy/writeback режим.
  • Сделать write, затем fsync.
  • Заставить один SYNC subop вернуть -EIO/-ENOSPC с retry.
  • Проверить, что повторный fsync повторно отправляет SYNC на тот же OSD и не завершается через пустой dirty_osds.

DATA-003: delete на misplaced/extra OSD может не попасть в sync set и затем воскреснуть

Серьезность: Critical
Уверенность: высокая
Компонент: src/osd

Код:

  • src/osd/osd_primary.cpp:698-710
  • src/osd/osd_primary.cpp:766-797
  • src/osd/osd_primary_subops.cpp:545-570
  • src/blockstore/blockstore_heap.cpp:2175-2227

Проблема:

Delete отправляется во все chunks из object_state->osd_set, если объект находится не только в текущем clean set:

  • src/osd/osd_primary.cpp:769-770
  • src/osd/osd_primary_subops.cpp:545-570

После успешных delete subops код удаляет объект из object_state и вызывает deref_object_state():

  • src/osd/osd_primary.cpp:790-791

Если это был последний объект в данном state_dict entry, deref_object_state() стирает state и зануляет op_data->object_state:

  • src/osd/osd_primary.cpp:706-709

Только после этого код строит dirty OSD set:

  • src/osd/osd_primary.cpp:793-797

Если op_data->object_state уже стал NULL, цикл берет pg.cur_loc_set, а не фактический set, куда были отправлены delete. Extra/misplaced OSD, например X, теряется из dirty set.

Почему это может привести к resurrection старых данных:

  1. Объект имеет extra/misplaced копию на OSD X вне pg.cur_loc_set.
  2. DELETE отправлен и на current set, и на X.
  3. Delete subop на X вернул success, но при immediate_commit=none это еще не означает durable fsync.
  4. OSD X не добавлен в dirty_osds, потому что object_state был освобожден до построения dirty set.
  5. Клиентский fsync/sync синкает только current set.
  6. X падает/рестартует до durable delete.
  7. При peering старая stable копия на X снова попадает в list. Так как stable delete версии 0 не возвращается как нормальная stable version в list_objects() (src/blockstore/blockstore_heap.cpp:2210-2227), старая копия может быть воспринята как recoverable/misplaced данные.

Идея исправления:

  • До remove_object_from_state() сохранить точный loc_set, куда реально отправлялся delete.
  • Dirty marking должен проходить по этому сохраненному set, а не по мутированному object_state.
  • Добавить инвариант: каждый OSD, получивший successful non-immediate delete/write, должен попасть в sync set.

Regression-test:

  • Replicated pool, immediate_commit=none.
  • Создать объект с extra/misplaced копией на OSD X.
  • Удалить объект, вызвать fsync.
  • Симулировать crash X до fsync его delete.
  • После restart/repeer проверить, что объект не воскресает и не восстанавливается из stale copy.

DATA-004: compaction пишет data blocks без учета в unsynced_data_write_count

Серьезность: Critical/High
Уверенность: высокая
Компонент: src/blockstore

Код:

  • src/blockstore/blockstore_flush.cpp:380-395
  • src/blockstore/blockstore_flush.cpp:421-432
  • src/blockstore/blockstore_sync.cpp:21-33
  • src/blockstore/blockstore_write.cpp:428-449

Проблема:

Во время compaction flusher читает small writes из journal и пишет объединенные данные в data_fd:

  • src/blockstore/blockstore_flush.cpp:380-395

Затем добавляет metadata entry через add_compact() и пишет metadata block:

  • src/blockstore/blockstore_flush.cpp:421-432

Но эти data writes нигде не увеличивают unsynced_data_write_count. Поиск всех мест изменения счетчиков показывает, что data counter увеличивается только в обычном write path (src/blockstore/blockstore_write.cpp:428-449), а flusher data writes не учитываются.

Почему это может привести к silent corruption:

  1. Есть stable base object и несколько small overwrites в journal.
  2. Compaction пишет merged object data в data device.
  3. Metadata entry о compacted stable version попадает в meta device.
  4. Следующий sync видит только unsynced_meta_write_count и fsync-ит metadata, но не data device, особенно при раздельных data_device/meta_device.
  5. Crash после metadata fsync, но до фактического flush data device.
  6. После restart metadata указывает на compacted location, где могут лежать старые/частично записанные bytes.

Если checksums включены, это может проявиться как checksum error; без checksums это может быть тихое чтение старых байтов.

Идея исправления:

  • Все compaction writes в data_fd должны участвовать в той же durability accounting, что и normal big writes.
  • Перед durable publication metadata entry должен быть гарантирован fsync data device, либо metadata не должна становиться durable раньше data.
  • Для separated data/meta устройств нужен явный тест порядка fsync.

Regression-test:

  • Mock/block device с раздельными data_device и meta_device.
  • Записать stable base, затем small overwrite.
  • Принудительно запустить compaction.
  • Разрешить fsync metadata, но drop volatile cache data device.
  • Restart и read latest; ожидать корректные bytes/checksum.

DATA-005: meta_format=3 delete хранится как stable version 0 и ломает версионность

Серьезность: High/Critical
Уверенность: высокая
Компонент: src/blockstore, затрагивает src/osd

Код:

  • src/blockstore/blockstore.h:67-94
  • src/blockstore/blockstore_write.cpp:125-141
  • src/blockstore/blockstore_heap.cpp:1867-1871
  • src/blockstore/blockstore_read.cpp:36-40
  • src/blockstore/blockstore_read.cpp:455-495
  • src/osd/osd_primary.cpp:766-770
  • src/osd/osd_primary_chain.cpp:86-96
  • src/osd/osd_primary_chain.cpp:164-166

Проблема:

Blockstore API документирует:

  • для writes: version == 0 означает auto-assign новой версии;
  • для delete: version трактуется как у write, delete "initially written as an object version".

Но v3/heap path для delete всегда вызывает:

  • src/blockstore/blockstore_heap.cpp:1867-1871

и записывает BS_HEAP_DELETE|BS_HEAP_STABLE с version = 0, игнорируя op->version.

Это конфликтует с несколькими инвариантами:

  • 0 используется как sentinel для отсутствия stable version (src/blockstore/blockstore_heap.cpp:2210-2227).
  • read_bitmap() и обычный read останавливаются на delete. Так как delete имеет version 0, любой target_version >= 0 попадает в tombstone и не может прочитать предыдущую версию (src/blockstore/blockstore_read.cpp:36-40, src/blockstore/blockstore_read.cpp:463-468).
  • OSD delete специально ставит pg.ver_override[oid] = old_version перед отправкой delete, чтобы параллельные reads не увидели незавершенный delete (src/osd/osd_primary.cpp:766-770), но blockstore delete version 0 делает такой override неэффективным на OSD, где tombstone уже применился.

Отдельный явный баг в chained reads:

Fast path для clean replicated PG читает bitmaps локально с UINT64_MAX:

  • src/osd/osd_primary_chain.cpp:86-96

а slow path учитывает pg.ver_override:

  • src/osd/osd_primary_chain.cpp:164-166

То есть snapshot/chained read может увидеть незавершенный delete даже на уровне bitmap selection.

Почему это может привести к повреждению/потере данных:

  1. Delete начался и поставил pg.ver_override на старую версию.
  2. Локальный delete успел примениться, remote delete еще pending или потом fail.
  3. Параллельный read/chained read получает view как будто delete уже committed.
  4. Если этот read используется higher-level workflow для clone/snapshot/copy/repair, можно закрепить данные из неcommitted состояния.
  5. Rollback/stabilize semantics для delete также выглядят сломанными: unstable delete становится immediately stable tombstone version 0.

Идея исправления:

  • Хранить delete как нормальную версионную запись с op->version, а не как stable sentinel 0, либо явно перепроектировать все readers/list/rollback/ver_override вокруг sentinel delete.
  • read/read_bitmap должны уметь читать версию до delete, если target_version < delete_version.
  • В osd_primary_chain.cpp fast path должен использовать pg.ver_override, как slow path.
  • Добавить тесты на parallel read during delete failure и rollback delete.

Regression-test:

  • Write object v1, stable.
  • Start delete v2, до завершения всех subops выполнить read target v1/read_bitmap target v1 на OSD, где tombstone уже применился.
  • Ожидание: старая версия читается.
  • Отдельно: delete без stabilize, rollback к v1, read latest/stable должен вернуть v1.

DATA-006: version == 0 для write в meta_format=3 не auto-assignится

Серьезность: High для blockstore API, exposure зависит от callers
Уверенность: высокая
Компонент: src/blockstore

Код:

  • src/blockstore/blockstore.h:67-69
  • src/blockstore/blockstore_write.cpp:148-151
  • src/blockstore/blockstore_write.cpp:223-251
  • src/blockstore/blockstore_write.cpp:365-366
  • src/blockstore/blockstore_heap.cpp:1554-1601
  • src/blockstore/blockstore_heap.cpp:376-380
  • src/blockstore/v1/write.cpp:79-82

Проблема:

API говорит, что write с version == 0 получает новую версию автоматически. В v1 path это действительно есть:

  • src/blockstore/v1/write.cpp:79-82

В heap/v3 path op->version напрямую передается в add_small_write()/add_big_write() и затем в metadata:

  • src/blockstore/blockstore_write.cpp:223-251
  • src/blockstore/blockstore_write.cpp:365-366
  • src/blockstore/blockstore_heap.cpp:1571
  • src/blockstore/blockstore_heap.cpp:1600

Таким образом, публично допустимый write может попасть на диск как object version 0.

Почему это опасно:

  • Stable version 0 не возвращается list_objects(), потому что stable_version == 0 означает "нет stable object".
  • STABLE для такой записи может породить BS_HEAP_COMMIT version 0, а loader считает commit version 0 повреждением metadata:
    • src/blockstore/blockstore_heap.cpp:376-380
  • Успешная запись через blockstore API может стать невидимой после restart/list или сделать metadata unrecoverable.

Exposure:

OSD primary обычно назначает nonzero версии сам. Но fio_engine, fio_sec_osd, tests и потенциальные direct blockstore users полагаются на version=0 auto-assign. Поскольку meta_format=3 является heap format и выставляется по умолчанию в prepare path (src/disk_tool/disk_tool_prepare.cpp:126-128, src/blockstore/ondisk_formats.h:149-151), это не выглядит как legacy-only проблема.

Идея исправления:

  • Вернуть auto-assignment версии в v3 path до вызова heap add functions.
  • Запретить запись version 0 в heap metadata для normal write/commit.
  • Добавить assert/test, что successful write никогда не оставляет normal object version 0.

Regression-test:

  • meta_format=3.
  • BS_OP_WRITE или BS_OP_WRITE_STABLE с version=0.
  • Проверить, что callback возвращает assigned version > 0.
  • Sync/stable/restart/list/read должны видеть тот же объект.

DATA-007: EC scrub ec_find_good() может выбрать неверный good subset или выйти за границы

Серьезность: High
Уверенность: высокая
Компонент: src/osd EC/XOR scrub/recovery

Код:

  • src/osd/osd_scrub.cpp:372-425
  • src/osd/osd_scrub.cpp:542-546
  • src/osd/osd_rmw.cpp:1277-1400
  • src/osd/osd_rmw_test.cpp:1140 и далее

Проблема 1: неверная инициализация eq_to

ec_find_good() создает int eq_to[stripe_count], но инициализирует цикл до pg_size:

  • src/osd/osd_rmw.cpp:1282-1287

А scrub для unclean object строит stripe_count из object_state->osd_set, исключая outdated chunks:

  • src/osd/osd_scrub.cpp:395-413

В результате stripe_count может быть меньше или больше pg_size.

  • Если stripe_count < pg_size: stack out-of-bounds write.
  • Если stripe_count > pg_size: часть eq_to[] остается неинициализированной, а дальше читается на src/osd/osd_rmw.cpp:1308.

Проблема 2: временный буфер выделен по stripe_count, а используется по pg_size

tmp_buf выделяется как:

  • src/osd/osd_rmw.cpp:1340

но ec_check_combination() вызывается с pg_size и может индексировать временные data/bitmap buffers по ролям до pg_size. Для degraded EC, например 5+3, где scrub видит 6 или 7 live chunks при pg_size=8, это может дать heap out-of-bounds.

Проблема 3: перебор duplicate role variants использует неправильный индекс

Чтение subset делает правильное отображение role ordinal -> real role:

  • src/osd/osd_rmw.cpp:1353

Но счетчик subvariant проверяется так:

  • src/osd/osd_rmw.cpp:1384

То есть используется live_variants[combination[i]], а не live_variants[comb_to_subset[combination[i]]].

Для sparse roles, например {2,4,5} или при duplicate role после rebalance/recovery, перебор может пропустить правильный variant или остановиться по размеру другого role.

Почему это может привести к data loss/corruption:

Scrub с scrub_find_best может:

  • не найти существующий корректный EC набор;
  • выбрать неправильный набор как authoritative;
  • пометить хороший chunk как corrupted;
  • оставить bad chunk как источник recovery.

После этого recovery/fix может переписать missing/outdated chunks уже на основе неверного good_subset.

Идея исправления:

  • Все массивы в ec_find_good() должны иметь четкую модель: indexed by dense stripe index или by EC role. Сейчас эти модели смешаны.
  • eq_to инициализировать по stripe_count.
  • Temporary buffers выделять по pg_size, если downstream indexing идет по role/pg index.
  • В subvariant loop использовать comb_to_subset.
  • Добавить bounds checks/asserts на stripes[i].role < pg_size.

Regression-tests:

  • EC 5+3, pg_size=8, pg_minsize=5, stripe_count=6/7, nonzero bitmap, missing high role; запуск под ASAN.
  • Case со stripe_count > pg_size, duplicate role, missing role 0: первый duplicate variant битый, второй корректный. Проверить, что выбран корректный variant.
  • MSAN/ASAN для неинициализированного eq_to.

DATA-008: scrub marking использует chunk.role как индекс dense stripes[]

Серьезность: High/Medium-High
Уверенность: средне-высокая
Компонент: src/osd scrub state marking

Код:

  • src/osd/osd_scrub.cpp:599-646

Проблема:

После scrub code ищет в new_set тот же chunk через dense index i:

  • src/osd/osd_scrub.cpp:603-622

Но затем для очистки LOC_CORRUPTED использует:

  • src/osd/osd_scrub.cpp:628-632

op_data->stripes[chunk.role] корректно только если stripes[] индексирован по role. Для unclean object submit_scrub_subops() строит stripes[] как плотный список non-outdated chunks из object_state->osd_set:

  • src/osd/osd_scrub.cpp:395-413

Там index != role при EC degraded/misplaced/duplicate/sparse roles.

Почему это может привести к повреждению:

  • Можно прочитать missing от другого chunk.
  • Можно выйти за границы, если chunk.role >= stripe_count.
  • Можно ошибочно снять LOC_CORRUPTED с bad chunk.

Следующий recovery будет использовать состояние объекта, где corrupted/missing flags не соответствуют фактическим данным.

Идея исправления:

  • В marking lambda использовать текущий dense index i для состояния op_data->stripes[i].
  • Если нужен доступ по role, заранее построить role -> dense index map с учетом duplicates, или явно запретить ambiguous lookup.

Regression-test:

  • Создать EC object_state с roles {1,2,4,5} или duplicate role.
  • Scrub должен не обращаться к stripes[role].
  • Проверить, что LOC_CORRUPTED очищается/ставится только на фактически проверенный (osd_num, role).

DATA-009: writeback CACHE_REPEATING может не replay-иться при смене primary

Серьезность: High/Medium-High
Уверенность: средняя, зависит от гарантий старого primary/connection teardown
Компонент: src/client writeback/lazy write

Код:

  • src/client/cluster_client.cpp:613-620
  • src/client/cluster_client.cpp:794-800
  • src/client/cluster_client.cpp:1167-1170
  • src/client/cluster_client_wb.cpp:260-291
  • src/client/cluster_client_wb.cpp:294-321

Проблема:

При смене primary клиент вызывает:

  • src/client/cluster_client.cpp:613-620

repeat_ops_for() должен повторить операции PG, потому что OSD, переставший быть primary, мог не fsync-нуть операции. Но он исключает buffers в состояниях CACHE_REPEATING и CACHE_DIRTY:

  • src/client/cluster_client_wb.cpp:270

Обычная non-immediate write перед отправкой сохраняется как CACHE_REPEATING:

  • src/client/cluster_client.cpp:794-800

После success callback клиент помечает buffer как written:

  • src/client/cluster_client.cpp:1167-1170

Сценарий риска:

  1. Write отправлен старому primary и находится в состоянии CACHE_REPEATING.
  2. PG меняет primary.
  3. repeat_ops_for() не replay-ит этот buffer из-за фильтра CACHE_REPEATING.
  4. Старый primary позже возвращает success.
  5. Клиент помечает buffer как CACHE_WRITTEN, хотя новый authoritative primary мог не получить эту запись.
  6. Следующий fsync может синкать не тот набор OSD/PG state.

Если протокол всегда гарантированно рвет такие операции при смене primary и никогда не возвращает stale success, риск снимается. Но на стороне клиента эта гарантия явно не проверяется.

Идея исправления:

  • При primary change считать in-flight CACHE_REPEATING для затронутой PG потенциально obsolete и повторять/invalidating их.
  • Callback старого primary должен проверять, что op still belongs to current PG epoch/primary.
  • Добавить per-op PG epoch/primary token.

Regression-test:

  • Включить writeback/non-immediate writes.
  • Задержать response от old primary.
  • Сменить primary.
  • Проверить, что write replay-ится на new primary или stale response игнорируется.

DATA-010: writeback copy_write() копирует iovec без ограничения op->len

Серьезность: Medium-High
Уверенность: высокая для memory corruption при некорректном iov, production exposure зависит от callers
Компонент: src/client

Код:

  • src/client/cluster_client_wb.cpp:247-256
  • src/client/vitastor_c.cpp:316

Проблема:

copy_write() выделяет buffer под op->len, но копирует каждый iovec полным iov.iov_len:

  • src/client/cluster_client_wb.cpp:247-256

Переменная len инициализируется, но не уменьшается. Нет проверки, что сумма iovec равна op->len.

Почему это опасно:

  • sum(iov_len) > op->len: heap overwrite за пределы writeback buffer.
  • sum(iov_len) < op->len: хвост buffer остается неинициализированным и может быть позже записан как stale data.

Если все internal callers всегда формируют iovec строго под op->len, production exposure ниже. Но public C API прокидывает caller iov без очевидной нормализации суммы.

Идея исправления:

  • Копировать не больше оставшегося len.
  • Проверять sum(iov_len) == op->len, иначе возвращать -EINVAL.
  • Обнулять/запрещать неполный input.

Regression-test:

  • C API write с len=4096, iov sum 8192: ожидать -EINVAL, не overwrite.
  • C API write с len=4096, iov sum 2048: ожидать -EINVAL, не запись мусора.

DATA-011: messenger не сверяет reply opcode с исходным request opcode

Серьезность: Medium
Уверенность: высокая по коду, production exposure в основном при bug/corrupt/malicious peer
Компонент: src/client messenger

Код:

  • src/client/msgr_receive.cpp:348-394
  • src/client/msgr_receive.cpp:395-434

Проблема:

Reply сопоставляется с pending request только по id:

  • src/client/msgr_receive.cpp:350

Затем header копируется в original op:

  • src/client/msgr_receive.cpp:359

Но нет проверки, что op->reply.hdr.opcode == op->req.hdr.opcode. Далее receive path доверяет reply opcode, чтобы решить, надо ли читать payload:

  • src/client/msgr_receive.cpp:361-394

Риск:

Wrong/corrupt peer reply с корректным id, но другим opcode может:

  • завершить read без чтения data payload;
  • интерпретировать write как read и принять payload в request iov;
  • десинхронизировать stream.

Это больше protocol robustness/security finding, но для storage-системы лучше fail-stop, чем silent wrong reply handling.

Идея исправления:

  • После lookup по id проверять opcode и тип payload.
  • При mismatch закрывать connection и возвращать retryable error.
  • Добавить protocol fuzz test.

DATA-012: read error reply может overflow-нуть bitmap buffer

Серьезность: Medium
Уверенность: высокая по коду, production exposure в основном при bug/corrupt/malicious peer
Компонент: src/client messenger

Код:

  • src/client/msgr_receive.cpp:361-380

Проблема:

Проверка bmp_len > op->bitmap_len выполняется только если retval >= 0:

  • src/client/msgr_receive.cpp:366-374

Но после этого код все равно добавляет bmp_len bytes в receive list, даже при retval < 0:

  • src/client/msgr_receive.cpp:375-380

Bad/corrupt error reply для OSD_OP_READ или OSD_OP_SEC_READ с nonzero bitmap_len/attr_len может записать за пределы bitmap/RMW buffer.

Идея исправления:

  • Валидировать bmp_len <= op->bitmap_len независимо от retval.
  • Для error replies не принимать data/bitmap payload вообще, если протокол этого не требует.

DATA-013: wire lengths/counts приводятся к int read_remaining

Серьезность: Medium
Уверенность: высокая по коду, production exposure зависит от max object list sizes и trust peer
Компонент: src/client messenger/listing

Код:

  • src/client/messenger.h:79
  • src/client/msgr_receive.cpp:395-434
  • src/client/cluster_client_list.cpp:409

Проблема:

read_remaining имеет тип int, а некоторые размеры приходят как int64_t/uint64_t или вычисляются из count:

  • src/client/msgr_receive.cpp:401
  • src/client/msgr_receive.cpp:411
  • src/client/msgr_receive.cpp:421
  • src/client/msgr_receive.cpp:431

Например sizeof(obj_ver_id) * retval для OSD_OP_SEC_LIST может переполнить/truncate int, после чего consumer все еще может идти по полному retval.

Риск:

На очень больших листингах или corrupt reply можно получить partial receive, stream desync и out-of-bounds consumption. Если это попадет в repair/remove/list workflow, это может привести к неправильным решениям верхнего уровня.

Идея исправления:

  • read_remaining должен быть uint64_t/size_t с проверкой верхней границы.
  • Проверять retval <= max_objects_per_reply и payload_size <= configured_max.
  • Reject connection on overflow.

DATA-014: peering может индексировать cur_set invalid role у outdated chunk

Серьезность: Medium
Уверенность: средняя
Компонент: src/osd peering

Код:

  • src/osd/osd_peering_pg.cpp:289-309

Проблема:

Для outdated versions после выбранного target version код делает:

  • src/osd/osd_peering_pg.cpp:302

без повторной проверки, что (list[i].oid.stripe & STRIPE_MASK) < pg->pg_size.

Invalid role chunks проверяются раньше, пока выбирается target version, но stale/invalid chunk может встретиться только в более старой версии.

Риск:

Это больше похоже на crash/peering robustness bug, чем на прямую silent corruption. Но при поврежденных/stale replicas peering должен fail-stop с PG_HAS_INVALID, а не читать за пределами cur_set.

Идея исправления:

  • Перед каждым индексированием role в cur_set проверять range.
  • Для invalid role выставлять PG_HAS_INVALID/safe error path.

Рекомендованный порядок triage

  1. Сначала исправить BS_OP_SYNC accounting (DATA-001) и client sync retry (DATA-002). Это прямые acknowledged-durability нарушения.
  2. Затем закрыть delete resurrection и delete version semantics (DATA-003, DATA-005), потому что они затрагивают удаление и peering.
  3. Затем compaction data fsync ordering (DATA-004), особенно если используются раздельные data/meta устройства.
  4. Затем EC scrub/recovery (DATA-007, DATA-008), особенно если включены EC/XOR pools и scrub/fix.
  5. После этого client writeback primary-change и iovec validation (DATA-009, DATA-010).
  6. Messenger/peering robustness (DATA-011..DATA-014) как hardening, желательно с fuzz/property tests.

Общие рекомендации по тестам

  • Добавить crash-consistency harness с controllable completion для data/meta/journal fsync.
  • Тестировать separated data_device, meta_device, journal_device, потому что часть багов маскируется общим fd.
  • Запускать EC scrub/RMW tests под ASAN/MSAN/UBSAN.
  • Добавить property tests для ec_find_good() на случайные sparse roles, duplicate roles, missing roles и random corruption.
  • Добавить failpoint tests для смены primary во время in-flight lazy/writeback write.
  • Добавить invariant test: любой successful non-immediate write/delete на OSD обязан добавить этот OSD в dirty/sync accounting до client-visible fsync success.
# Аудит Vitastor на риски потери и повреждения данных Дата аудита: 2026-05-11 Ревизия: `caa70317faab41f92b0fd017f4437da5b2386a40` Фокус: сценарии, в которых production-кластер может подтвердить запись/delete/sync, а затем потерять данные, воскресить удаленные данные или закрепить неверный EC-набор как правильный. Это read-only аудит кода. Я не запускал разрушительные тесты на реальном кластере. Ниже перечислены только находки, которые выглядят релевантными для durability/data-loss/data-corruption. Часть пунктов требует минимального воспроизводящего теста, но несколько нарушений подтверждаются прямо по потоку кода. ## Краткое резюме Самые опасные места: - `BS_OP_SYNC` в blockstore может вернуть успех второму sync до завершения фактического `fsync`. - Клиентский `OSD_OP_SYNC` очищает `dirty_osds` до результата и при retryable ошибке может повторно завершиться успешно без повторного sync. - Delete на misplaced/extra OSD может не попасть в dirty/sync set; после crash старый объект может воскреснуть. - Compaction пишет новые данные в `data_fd`, но не учитывает их в `unsynced_data_write_count`; metadata может стать durable раньше данных. - В `meta_format=3` delete хранится как stable tombstone версии `0`, что ломает версионную семантику delete/read/rollback и `pg.ver_override`. - EC scrub/recovery содержит несколько ошибок индексации, из-за которых можно выбрать неправильный "good subset" и затем пометить/восстановить неверные chunks. ## DATA-001: `BS_OP_SYNC` может завершиться до фактического `fsync` Серьезность: Critical Уверенность: высокая Компонент: `src/blockstore` Код: - `src/blockstore/blockstore_sync.cpp:29-87` - `src/blockstore/blockstore_sync.cpp:101-108` - `src/blockstore/blockstore_impl.cpp:138-142` Проблема: `submit_fsyncs()` сбрасывает `unsynced_data_write_count`, `unsynced_buffer_write_count` и `unsynced_meta_write_count` сразу после отправки `io_uring_prep_fsync`, до completion callback: - `src/blockstore/blockstore_sync.cpp:84-86` После этого второй `BS_OP_SYNC`, обработанный в той же очереди до completion первого fsync, видит `has_unsynced() == false` на `src/blockstore/blockstore_sync.cpp:101` и возвращает успех немедленно. Почему это может привести к потере данных: 1. Write завершился и увеличил один из `unsynced_*` счетчиков. 2. Первый `BS_OP_SYNC` отправил fsync и тут же обнулил счетчики. 3. Второй `BS_OP_SYNC` увидел пустое dirty-состояние и завершился успешно. 4. Процесс/хост падает до completion первого fsync. 5. Клиент второго sync уже получил durable ack, но данные могли не попасть на носитель. Это нарушает базовую семантику `fsync`: подтвержденный sync обязан переживать crash. Идея исправления: - Не сбрасывать глобальные `unsynced_*` счетчики до completion всех отправленных fsync. - Или ввести поколение/epoch dirty-счетчиков: sync фиксирует поколение, новые writes попадают в следующее, а последующие sync ждут уже летящий fsync, если их требуемое поколение не durable. - Все sync, которые пришли после первого и зависят от того же dirty-поколения, должны завершаться только после completion реального fsync. Regression-test: - Instrument/mock `io_uring_prep_fsync`, чтобы задержать completion. - Enqueue: write -> sync1 -> sync2. - Проверить, что callback sync2 не вызывается до completion fsync. - После simulated crash/drop volatile cache проверить, что подтвержденные данные читаются. ## DATA-002: client `continue_sync()` теряет `dirty_osds` при retryable ошибке Серьезность: Critical Уверенность: высокая Компонент: `src/client` Код: - `src/client/cluster_client.cpp:1463-1527` Проблема: `cluster_client_t::continue_sync()` очищает `dirty_osds` сразу после отправки sync-подопераций: - `src/client/cluster_client.cpp:1485-1501` Если один из OSD вернул retryable `-EPIPE`, `-EIO` или `-ENOSPC`, код вызывает `wb->fsync_error()`, сбрасывает состояние операции и возвращает retry: - `src/client/cluster_client.cpp:1508-1519` Но список `dirty_osds` уже потерян. На следующем заходе `continue_sync()` попадает в fast path: - `src/client/cluster_client.cpp:1467-1472` и завершает fsync успешно без повторной отправки `OSD_OP_SYNC`. Почему это может привести к потере данных: 1. Lazy/writeback write успешно записан на OSD и помечен как нуждающийся в sync. 2. Клиент вызывает fsync. 3. Sync отправляется на dirty OSD, затем `dirty_osds.clear()`. 4. OSD отвечает retryable ошибкой или временно недоступен. 5. Клиент повторяет fsync, но dirty set уже пустой, поэтому fsync возвращает success. 6. Crash OSD/хоста до реального sync приводит к потере записи, которую приложение считало fsync-нутой. Идея исправления: - Держать snapshot dirty OSD внутри операции sync до успешного завершения. - Очищать глобальный `dirty_osds` только после успеха, причем аккуратно: не удалять OSD, которые стали dirty новыми writes во время inflight sync. - При retry восстанавливать pending dirty set или повторять тот же snapshot. Regression-test: - Включить lazy/writeback режим. - Сделать write, затем fsync. - Заставить один SYNC subop вернуть `-EIO`/`-ENOSPC` с retry. - Проверить, что повторный fsync повторно отправляет SYNC на тот же OSD и не завершается через пустой `dirty_osds`. ## DATA-003: delete на misplaced/extra OSD может не попасть в sync set и затем воскреснуть Серьезность: Critical Уверенность: высокая Компонент: `src/osd` Код: - `src/osd/osd_primary.cpp:698-710` - `src/osd/osd_primary.cpp:766-797` - `src/osd/osd_primary_subops.cpp:545-570` - `src/blockstore/blockstore_heap.cpp:2175-2227` Проблема: Delete отправляется во все chunks из `object_state->osd_set`, если объект находится не только в текущем clean set: - `src/osd/osd_primary.cpp:769-770` - `src/osd/osd_primary_subops.cpp:545-570` После успешных delete subops код удаляет объект из `object_state` и вызывает `deref_object_state()`: - `src/osd/osd_primary.cpp:790-791` Если это был последний объект в данном `state_dict` entry, `deref_object_state()` стирает state и зануляет `op_data->object_state`: - `src/osd/osd_primary.cpp:706-709` Только после этого код строит dirty OSD set: - `src/osd/osd_primary.cpp:793-797` Если `op_data->object_state` уже стал `NULL`, цикл берет `pg.cur_loc_set`, а не фактический set, куда были отправлены delete. Extra/misplaced OSD, например `X`, теряется из dirty set. Почему это может привести к resurrection старых данных: 1. Объект имеет extra/misplaced копию на OSD `X` вне `pg.cur_loc_set`. 2. `DELETE` отправлен и на current set, и на `X`. 3. Delete subop на `X` вернул success, но при `immediate_commit=none` это еще не означает durable fsync. 4. OSD `X` не добавлен в `dirty_osds`, потому что `object_state` был освобожден до построения dirty set. 5. Клиентский fsync/sync синкает только current set. 6. `X` падает/рестартует до durable delete. 7. При peering старая stable копия на `X` снова попадает в list. Так как stable delete версии `0` не возвращается как нормальная stable version в `list_objects()` (`src/blockstore/blockstore_heap.cpp:2210-2227`), старая копия может быть воспринята как recoverable/misplaced данные. Идея исправления: - До `remove_object_from_state()` сохранить точный `loc_set`, куда реально отправлялся delete. - Dirty marking должен проходить по этому сохраненному set, а не по мутированному `object_state`. - Добавить инвариант: каждый OSD, получивший successful non-immediate delete/write, должен попасть в sync set. Regression-test: - Replicated pool, `immediate_commit=none`. - Создать объект с extra/misplaced копией на OSD `X`. - Удалить объект, вызвать fsync. - Симулировать crash `X` до fsync его delete. - После restart/repeer проверить, что объект не воскресает и не восстанавливается из stale copy. ## DATA-004: compaction пишет data blocks без учета в `unsynced_data_write_count` Серьезность: Critical/High Уверенность: высокая Компонент: `src/blockstore` Код: - `src/blockstore/blockstore_flush.cpp:380-395` - `src/blockstore/blockstore_flush.cpp:421-432` - `src/blockstore/blockstore_sync.cpp:21-33` - `src/blockstore/blockstore_write.cpp:428-449` Проблема: Во время compaction flusher читает small writes из journal и пишет объединенные данные в `data_fd`: - `src/blockstore/blockstore_flush.cpp:380-395` Затем добавляет metadata entry через `add_compact()` и пишет metadata block: - `src/blockstore/blockstore_flush.cpp:421-432` Но эти data writes нигде не увеличивают `unsynced_data_write_count`. Поиск всех мест изменения счетчиков показывает, что data counter увеличивается только в обычном write path (`src/blockstore/blockstore_write.cpp:428-449`), а flusher data writes не учитываются. Почему это может привести к silent corruption: 1. Есть stable base object и несколько small overwrites в journal. 2. Compaction пишет merged object data в data device. 3. Metadata entry о compacted stable version попадает в meta device. 4. Следующий sync видит только `unsynced_meta_write_count` и fsync-ит metadata, но не data device, особенно при раздельных `data_device`/`meta_device`. 5. Crash после metadata fsync, но до фактического flush data device. 6. После restart metadata указывает на compacted location, где могут лежать старые/частично записанные bytes. Если checksums включены, это может проявиться как checksum error; без checksums это может быть тихое чтение старых байтов. Идея исправления: - Все compaction writes в `data_fd` должны участвовать в той же durability accounting, что и normal big writes. - Перед durable publication metadata entry должен быть гарантирован fsync data device, либо metadata не должна становиться durable раньше data. - Для separated data/meta устройств нужен явный тест порядка fsync. Regression-test: - Mock/block device с раздельными `data_device` и `meta_device`. - Записать stable base, затем small overwrite. - Принудительно запустить compaction. - Разрешить fsync metadata, но drop volatile cache data device. - Restart и read latest; ожидать корректные bytes/checksum. ## DATA-005: `meta_format=3` delete хранится как stable version `0` и ломает версионность Серьезность: High/Critical Уверенность: высокая Компонент: `src/blockstore`, затрагивает `src/osd` Код: - `src/blockstore/blockstore.h:67-94` - `src/blockstore/blockstore_write.cpp:125-141` - `src/blockstore/blockstore_heap.cpp:1867-1871` - `src/blockstore/blockstore_read.cpp:36-40` - `src/blockstore/blockstore_read.cpp:455-495` - `src/osd/osd_primary.cpp:766-770` - `src/osd/osd_primary_chain.cpp:86-96` - `src/osd/osd_primary_chain.cpp:164-166` Проблема: Blockstore API документирует: - для writes: `version == 0` означает auto-assign новой версии; - для delete: `version` трактуется как у write, delete "initially written as an object version". Но v3/heap path для delete всегда вызывает: - `src/blockstore/blockstore_heap.cpp:1867-1871` и записывает `BS_HEAP_DELETE|BS_HEAP_STABLE` с `version = 0`, игнорируя `op->version`. Это конфликтует с несколькими инвариантами: - `0` используется как sentinel для отсутствия stable version (`src/blockstore/blockstore_heap.cpp:2210-2227`). - `read_bitmap()` и обычный read останавливаются на delete. Так как delete имеет version `0`, любой `target_version >= 0` попадает в tombstone и не может прочитать предыдущую версию (`src/blockstore/blockstore_read.cpp:36-40`, `src/blockstore/blockstore_read.cpp:463-468`). - OSD delete специально ставит `pg.ver_override[oid] = old_version` перед отправкой delete, чтобы параллельные reads не увидели незавершенный delete (`src/osd/osd_primary.cpp:766-770`), но blockstore delete version `0` делает такой override неэффективным на OSD, где tombstone уже применился. Отдельный явный баг в chained reads: Fast path для clean replicated PG читает bitmaps локально с `UINT64_MAX`: - `src/osd/osd_primary_chain.cpp:86-96` а slow path учитывает `pg.ver_override`: - `src/osd/osd_primary_chain.cpp:164-166` То есть snapshot/chained read может увидеть незавершенный delete даже на уровне bitmap selection. Почему это может привести к повреждению/потере данных: 1. Delete начался и поставил `pg.ver_override` на старую версию. 2. Локальный delete успел примениться, remote delete еще pending или потом fail. 3. Параллельный read/chained read получает view как будто delete уже committed. 4. Если этот read используется higher-level workflow для clone/snapshot/copy/repair, можно закрепить данные из неcommitted состояния. 5. Rollback/stabilize semantics для delete также выглядят сломанными: unstable delete становится immediately stable tombstone version `0`. Идея исправления: - Хранить delete как нормальную версионную запись с `op->version`, а не как stable sentinel `0`, либо явно перепроектировать все readers/list/rollback/ver_override вокруг sentinel delete. - `read`/`read_bitmap` должны уметь читать версию до delete, если `target_version < delete_version`. - В `osd_primary_chain.cpp` fast path должен использовать `pg.ver_override`, как slow path. - Добавить тесты на parallel read during delete failure и rollback delete. Regression-test: - Write object v1, stable. - Start delete v2, до завершения всех subops выполнить read target v1/read_bitmap target v1 на OSD, где tombstone уже применился. - Ожидание: старая версия читается. - Отдельно: delete без stabilize, rollback к v1, read latest/stable должен вернуть v1. ## DATA-006: `version == 0` для write в `meta_format=3` не auto-assignится Серьезность: High для blockstore API, exposure зависит от callers Уверенность: высокая Компонент: `src/blockstore` Код: - `src/blockstore/blockstore.h:67-69` - `src/blockstore/blockstore_write.cpp:148-151` - `src/blockstore/blockstore_write.cpp:223-251` - `src/blockstore/blockstore_write.cpp:365-366` - `src/blockstore/blockstore_heap.cpp:1554-1601` - `src/blockstore/blockstore_heap.cpp:376-380` - `src/blockstore/v1/write.cpp:79-82` Проблема: API говорит, что write с `version == 0` получает новую версию автоматически. В v1 path это действительно есть: - `src/blockstore/v1/write.cpp:79-82` В heap/v3 path `op->version` напрямую передается в `add_small_write()`/`add_big_write()` и затем в metadata: - `src/blockstore/blockstore_write.cpp:223-251` - `src/blockstore/blockstore_write.cpp:365-366` - `src/blockstore/blockstore_heap.cpp:1571` - `src/blockstore/blockstore_heap.cpp:1600` Таким образом, публично допустимый write может попасть на диск как object version `0`. Почему это опасно: - Stable version `0` не возвращается `list_objects()`, потому что `stable_version == 0` означает "нет stable object". - `STABLE` для такой записи может породить `BS_HEAP_COMMIT` version `0`, а loader считает commit version `0` повреждением metadata: - `src/blockstore/blockstore_heap.cpp:376-380` - Успешная запись через blockstore API может стать невидимой после restart/list или сделать metadata unrecoverable. Exposure: OSD primary обычно назначает nonzero версии сам. Но `fio_engine`, `fio_sec_osd`, tests и потенциальные direct blockstore users полагаются на `version=0` auto-assign. Поскольку `meta_format=3` является heap format и выставляется по умолчанию в prepare path (`src/disk_tool/disk_tool_prepare.cpp:126-128`, `src/blockstore/ondisk_formats.h:149-151`), это не выглядит как legacy-only проблема. Идея исправления: - Вернуть auto-assignment версии в v3 path до вызова heap add functions. - Запретить запись version `0` в heap metadata для normal write/commit. - Добавить assert/test, что successful write никогда не оставляет normal object version `0`. Regression-test: - `meta_format=3`. - `BS_OP_WRITE` или `BS_OP_WRITE_STABLE` с `version=0`. - Проверить, что callback возвращает assigned version > 0. - Sync/stable/restart/list/read должны видеть тот же объект. ## DATA-007: EC scrub `ec_find_good()` может выбрать неверный good subset или выйти за границы Серьезность: High Уверенность: высокая Компонент: `src/osd` EC/XOR scrub/recovery Код: - `src/osd/osd_scrub.cpp:372-425` - `src/osd/osd_scrub.cpp:542-546` - `src/osd/osd_rmw.cpp:1277-1400` - `src/osd/osd_rmw_test.cpp:1140` и далее Проблема 1: неверная инициализация `eq_to` `ec_find_good()` создает `int eq_to[stripe_count]`, но инициализирует цикл до `pg_size`: - `src/osd/osd_rmw.cpp:1282-1287` А scrub для unclean object строит `stripe_count` из `object_state->osd_set`, исключая outdated chunks: - `src/osd/osd_scrub.cpp:395-413` В результате `stripe_count` может быть меньше или больше `pg_size`. - Если `stripe_count < pg_size`: stack out-of-bounds write. - Если `stripe_count > pg_size`: часть `eq_to[]` остается неинициализированной, а дальше читается на `src/osd/osd_rmw.cpp:1308`. Проблема 2: временный буфер выделен по `stripe_count`, а используется по `pg_size` `tmp_buf` выделяется как: - `src/osd/osd_rmw.cpp:1340` но `ec_check_combination()` вызывается с `pg_size` и может индексировать временные data/bitmap buffers по ролям до `pg_size`. Для degraded EC, например 5+3, где scrub видит 6 или 7 live chunks при `pg_size=8`, это может дать heap out-of-bounds. Проблема 3: перебор duplicate role variants использует неправильный индекс Чтение subset делает правильное отображение role ordinal -> real role: - `src/osd/osd_rmw.cpp:1353` Но счетчик subvariant проверяется так: - `src/osd/osd_rmw.cpp:1384` То есть используется `live_variants[combination[i]]`, а не `live_variants[comb_to_subset[combination[i]]]`. Для sparse roles, например `{2,4,5}` или при duplicate role после rebalance/recovery, перебор может пропустить правильный variant или остановиться по размеру другого role. Почему это может привести к data loss/corruption: Scrub с `scrub_find_best` может: - не найти существующий корректный EC набор; - выбрать неправильный набор как authoritative; - пометить хороший chunk как corrupted; - оставить bad chunk как источник recovery. После этого recovery/fix может переписать missing/outdated chunks уже на основе неверного `good_subset`. Идея исправления: - Все массивы в `ec_find_good()` должны иметь четкую модель: indexed by dense stripe index или by EC role. Сейчас эти модели смешаны. - `eq_to` инициализировать по `stripe_count`. - Temporary buffers выделять по `pg_size`, если downstream indexing идет по role/pg index. - В subvariant loop использовать `comb_to_subset`. - Добавить bounds checks/asserts на `stripes[i].role < pg_size`. Regression-tests: - EC 5+3, `pg_size=8`, `pg_minsize=5`, `stripe_count=6/7`, nonzero bitmap, missing high role; запуск под ASAN. - Case со `stripe_count > pg_size`, duplicate role, missing role 0: первый duplicate variant битый, второй корректный. Проверить, что выбран корректный variant. - MSAN/ASAN для неинициализированного `eq_to`. ## DATA-008: scrub marking использует `chunk.role` как индекс dense `stripes[]` Серьезность: High/Medium-High Уверенность: средне-высокая Компонент: `src/osd` scrub state marking Код: - `src/osd/osd_scrub.cpp:599-646` Проблема: После scrub code ищет в `new_set` тот же chunk через dense index `i`: - `src/osd/osd_scrub.cpp:603-622` Но затем для очистки `LOC_CORRUPTED` использует: - `src/osd/osd_scrub.cpp:628-632` `op_data->stripes[chunk.role]` корректно только если `stripes[]` индексирован по role. Для unclean object `submit_scrub_subops()` строит `stripes[]` как плотный список non-outdated chunks из `object_state->osd_set`: - `src/osd/osd_scrub.cpp:395-413` Там index != role при EC degraded/misplaced/duplicate/sparse roles. Почему это может привести к повреждению: - Можно прочитать `missing` от другого chunk. - Можно выйти за границы, если `chunk.role >= stripe_count`. - Можно ошибочно снять `LOC_CORRUPTED` с bad chunk. Следующий recovery будет использовать состояние объекта, где corrupted/missing flags не соответствуют фактическим данным. Идея исправления: - В marking lambda использовать текущий dense index `i` для состояния `op_data->stripes[i]`. - Если нужен доступ по role, заранее построить `role -> dense index` map с учетом duplicates, или явно запретить ambiguous lookup. Regression-test: - Создать EC object_state с roles `{1,2,4,5}` или duplicate role. - Scrub должен не обращаться к `stripes[role]`. - Проверить, что `LOC_CORRUPTED` очищается/ставится только на фактически проверенный `(osd_num, role)`. ## DATA-009: writeback `CACHE_REPEATING` может не replay-иться при смене primary Серьезность: High/Medium-High Уверенность: средняя, зависит от гарантий старого primary/connection teardown Компонент: `src/client` writeback/lazy write Код: - `src/client/cluster_client.cpp:613-620` - `src/client/cluster_client.cpp:794-800` - `src/client/cluster_client.cpp:1167-1170` - `src/client/cluster_client_wb.cpp:260-291` - `src/client/cluster_client_wb.cpp:294-321` Проблема: При смене primary клиент вызывает: - `src/client/cluster_client.cpp:613-620` `repeat_ops_for()` должен повторить операции PG, потому что OSD, переставший быть primary, мог не fsync-нуть операции. Но он исключает buffers в состояниях `CACHE_REPEATING` и `CACHE_DIRTY`: - `src/client/cluster_client_wb.cpp:270` Обычная non-immediate write перед отправкой сохраняется как `CACHE_REPEATING`: - `src/client/cluster_client.cpp:794-800` После success callback клиент помечает buffer как written: - `src/client/cluster_client.cpp:1167-1170` Сценарий риска: 1. Write отправлен старому primary и находится в состоянии `CACHE_REPEATING`. 2. PG меняет primary. 3. `repeat_ops_for()` не replay-ит этот buffer из-за фильтра `CACHE_REPEATING`. 4. Старый primary позже возвращает success. 5. Клиент помечает buffer как `CACHE_WRITTEN`, хотя новый authoritative primary мог не получить эту запись. 6. Следующий fsync может синкать не тот набор OSD/PG state. Если протокол всегда гарантированно рвет такие операции при смене primary и никогда не возвращает stale success, риск снимается. Но на стороне клиента эта гарантия явно не проверяется. Идея исправления: - При primary change считать in-flight `CACHE_REPEATING` для затронутой PG потенциально obsolete и повторять/invalidating их. - Callback старого primary должен проверять, что op still belongs to current PG epoch/primary. - Добавить per-op PG epoch/primary token. Regression-test: - Включить writeback/non-immediate writes. - Задержать response от old primary. - Сменить primary. - Проверить, что write replay-ится на new primary или stale response игнорируется. ## DATA-010: writeback `copy_write()` копирует iovec без ограничения `op->len` Серьезность: Medium-High Уверенность: высокая для memory corruption при некорректном iov, production exposure зависит от callers Компонент: `src/client` Код: - `src/client/cluster_client_wb.cpp:247-256` - `src/client/vitastor_c.cpp:316` Проблема: `copy_write()` выделяет buffer под `op->len`, но копирует каждый iovec полным `iov.iov_len`: - `src/client/cluster_client_wb.cpp:247-256` Переменная `len` инициализируется, но не уменьшается. Нет проверки, что сумма iovec равна `op->len`. Почему это опасно: - `sum(iov_len) > op->len`: heap overwrite за пределы writeback buffer. - `sum(iov_len) < op->len`: хвост buffer остается неинициализированным и может быть позже записан как stale data. Если все internal callers всегда формируют iovec строго под `op->len`, production exposure ниже. Но public C API прокидывает caller iov без очевидной нормализации суммы. Идея исправления: - Копировать не больше оставшегося `len`. - Проверять `sum(iov_len) == op->len`, иначе возвращать `-EINVAL`. - Обнулять/запрещать неполный input. Regression-test: - C API write с `len=4096`, iov sum 8192: ожидать `-EINVAL`, не overwrite. - C API write с `len=4096`, iov sum 2048: ожидать `-EINVAL`, не запись мусора. ## DATA-011: messenger не сверяет reply opcode с исходным request opcode Серьезность: Medium Уверенность: высокая по коду, production exposure в основном при bug/corrupt/malicious peer Компонент: `src/client` messenger Код: - `src/client/msgr_receive.cpp:348-394` - `src/client/msgr_receive.cpp:395-434` Проблема: Reply сопоставляется с pending request только по `id`: - `src/client/msgr_receive.cpp:350` Затем header копируется в original op: - `src/client/msgr_receive.cpp:359` Но нет проверки, что `op->reply.hdr.opcode == op->req.hdr.opcode`. Далее receive path доверяет reply opcode, чтобы решить, надо ли читать payload: - `src/client/msgr_receive.cpp:361-394` Риск: Wrong/corrupt peer reply с корректным id, но другим opcode может: - завершить read без чтения data payload; - интерпретировать write как read и принять payload в request iov; - десинхронизировать stream. Это больше protocol robustness/security finding, но для storage-системы лучше fail-stop, чем silent wrong reply handling. Идея исправления: - После lookup по id проверять opcode и тип payload. - При mismatch закрывать connection и возвращать retryable error. - Добавить protocol fuzz test. ## DATA-012: read error reply может overflow-нуть bitmap buffer Серьезность: Medium Уверенность: высокая по коду, production exposure в основном при bug/corrupt/malicious peer Компонент: `src/client` messenger Код: - `src/client/msgr_receive.cpp:361-380` Проблема: Проверка `bmp_len > op->bitmap_len` выполняется только если `retval >= 0`: - `src/client/msgr_receive.cpp:366-374` Но после этого код все равно добавляет `bmp_len` bytes в receive list, даже при `retval < 0`: - `src/client/msgr_receive.cpp:375-380` Bad/corrupt error reply для `OSD_OP_READ` или `OSD_OP_SEC_READ` с nonzero `bitmap_len`/`attr_len` может записать за пределы bitmap/RMW buffer. Идея исправления: - Валидировать `bmp_len <= op->bitmap_len` независимо от `retval`. - Для error replies не принимать data/bitmap payload вообще, если протокол этого не требует. ## DATA-013: wire lengths/counts приводятся к `int read_remaining` Серьезность: Medium Уверенность: высокая по коду, production exposure зависит от max object list sizes и trust peer Компонент: `src/client` messenger/listing Код: - `src/client/messenger.h:79` - `src/client/msgr_receive.cpp:395-434` - `src/client/cluster_client_list.cpp:409` Проблема: `read_remaining` имеет тип `int`, а некоторые размеры приходят как `int64_t`/`uint64_t` или вычисляются из count: - `src/client/msgr_receive.cpp:401` - `src/client/msgr_receive.cpp:411` - `src/client/msgr_receive.cpp:421` - `src/client/msgr_receive.cpp:431` Например `sizeof(obj_ver_id) * retval` для `OSD_OP_SEC_LIST` может переполнить/truncate `int`, после чего consumer все еще может идти по полному `retval`. Риск: На очень больших листингах или corrupt reply можно получить partial receive, stream desync и out-of-bounds consumption. Если это попадет в repair/remove/list workflow, это может привести к неправильным решениям верхнего уровня. Идея исправления: - `read_remaining` должен быть `uint64_t`/`size_t` с проверкой верхней границы. - Проверять `retval <= max_objects_per_reply` и `payload_size <= configured_max`. - Reject connection on overflow. ## DATA-014: peering может индексировать `cur_set` invalid role у outdated chunk Серьезность: Medium Уверенность: средняя Компонент: `src/osd` peering Код: - `src/osd/osd_peering_pg.cpp:289-309` Проблема: Для outdated versions после выбранного target version код делает: - `src/osd/osd_peering_pg.cpp:302` без повторной проверки, что `(list[i].oid.stripe & STRIPE_MASK) < pg->pg_size`. Invalid role chunks проверяются раньше, пока выбирается target version, но stale/invalid chunk может встретиться только в более старой версии. Риск: Это больше похоже на crash/peering robustness bug, чем на прямую silent corruption. Но при поврежденных/stale replicas peering должен fail-stop с `PG_HAS_INVALID`, а не читать за пределами `cur_set`. Идея исправления: - Перед каждым индексированием role в `cur_set` проверять range. - Для invalid role выставлять `PG_HAS_INVALID`/safe error path. ## Рекомендованный порядок triage 1. Сначала исправить `BS_OP_SYNC` accounting (`DATA-001`) и client sync retry (`DATA-002`). Это прямые acknowledged-durability нарушения. 2. Затем закрыть delete resurrection и delete version semantics (`DATA-003`, `DATA-005`), потому что они затрагивают удаление и peering. 3. Затем compaction data fsync ordering (`DATA-004`), особенно если используются раздельные data/meta устройства. 4. Затем EC scrub/recovery (`DATA-007`, `DATA-008`), особенно если включены EC/XOR pools и scrub/fix. 5. После этого client writeback primary-change и iovec validation (`DATA-009`, `DATA-010`). 6. Messenger/peering robustness (`DATA-011`..`DATA-014`) как hardening, желательно с fuzz/property tests. ## Общие рекомендации по тестам - Добавить crash-consistency harness с controllable completion для data/meta/journal fsync. - Тестировать separated `data_device`, `meta_device`, `journal_device`, потому что часть багов маскируется общим fd. - Запускать EC scrub/RMW tests под ASAN/MSAN/UBSAN. - Добавить property tests для `ec_find_good()` на случайные sparse roles, duplicate roles, missing roles и random corruption. - Добавить failpoint tests для смены primary во время in-flight lazy/writeback write. - Добавить invariant test: любой successful non-immediate write/delete на OSD обязан добавить этот OSD в dirty/sync accounting до client-visible fsync success.
Owner

Посмотрел, тут резюме:

  • DATA-001 - надо проверить
  • DATA-002 - нет, т.к. continue_syncs не перенарезается, его dirty всегда остаются на месте
  • DATA-003 - да
  • DATA-004 - вроде да, но надо проверить
  • DATA-005 - нет, в heap delete и должно иметь version=0
  • DATA-006 - нет, в heap версии записей и не должны auto-assignиться
  • DATA-007 - да, уже было от клода
  • DATA-008 - нет, мутный отчёт, не ясно к чему конкретно претензия
  • DATA-009 - надо проверить
  • DATA-010 - нет, на совести вызывающего
  • DATA-011 - нет
  • DATA-012 - фактически бага нет (условия в жизни совпадают), хотя для корректности поправить можно
  • DATA-013 - бага нет, но навести порядок в целочисленных типах было бы полезно в целом, такая задача есть
  • DATA-014 - вроде да, но надо проверить - как будто мусор в списках объектов может спровоцировать некорректную обработку

Интересно, что почти не пересекается с отчётом клода :)

Посмотрел, тут резюме: - DATA-001 - надо проверить - DATA-002 - нет, т.к. continue_syncs не перенарезается, его dirty всегда остаются на месте - DATA-003 - да - DATA-004 - вроде да, но надо проверить - DATA-005 - нет, в heap delete и должно иметь version=0 - DATA-006 - нет, в heap версии записей и не должны auto-assignиться - DATA-007 - да, уже было от клода - DATA-008 - нет, мутный отчёт, не ясно к чему конкретно претензия - DATA-009 - надо проверить - DATA-010 - нет, на совести вызывающего - DATA-011 - нет - DATA-012 - фактически бага нет (условия в жизни совпадают), хотя для корректности поправить можно - DATA-013 - бага нет, но навести порядок в целочисленных типах было бы полезно в целом, такая задача есть - DATA-014 - вроде да, но надо проверить - как будто мусор в списках объектов может спровоцировать некорректную обработку Интересно, что почти не пересекается с отчётом клода :)
Owner

Запускать EC scrub/RMW tests под ASAN/MSAN/UBSAN.

Вообще все тесты гоняются под asan

>Запускать EC scrub/RMW tests под ASAN/MSAN/UBSAN. Вообще все тесты гоняются под asan
Owner

Апдейт:

  • DATA-001 - исправлено (важное для конфигураций с fsync)
  • DATA-002 - подтверждено и исправлено
  • DATA-003 - исправлено (низкая важность)
  • DATA-004 - исправлено, добавил fsync данных в компакшен в новом сторе и добавил тест
  • DATA-005 - отклонено
  • DATA-006 - отклонено
  • DATA-007 - исправлено (редкое)
  • DATA-008 - на самом деле часть 007, исправлено (редкое)
  • DATA-009 - отклонено
  • DATA-010 - исправлено (на работу не влияло)
  • DATA-011 - исправлено (на работу не влияло)
  • DATA-012 - исправлено (на работу не влияло)
  • DATA-013 - наведение порядка в целочисленных типах оставлено на будущее
  • DATA-014 - не подтверждено
Апдейт: - DATA-001 - исправлено (важное для конфигураций с fsync) - DATA-002 - подтверждено и исправлено - DATA-003 - исправлено (низкая важность) - DATA-004 - исправлено, добавил fsync данных в компакшен в новом сторе и добавил тест - DATA-005 - отклонено - DATA-006 - отклонено - DATA-007 - исправлено (редкое) - DATA-008 - на самом деле часть 007, исправлено (редкое) - DATA-009 - отклонено - DATA-010 - исправлено (на работу не влияло) - DATA-011 - исправлено (на работу не влияло) - DATA-012 - исправлено (на работу не влияло) - DATA-013 - наведение порядка в целочисленных типах оставлено на будущее - DATA-014 - не подтверждено
Owner

Тут вроде всё

Тут вроде всё
Sign in to join this conversation.
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vitalif/vitastor#66
No description provided.