From 9063bcaa416558fe1e021f4c56e9669d6060528b Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Sun, 13 Jul 2025 02:07:19 +0300 Subject: [PATCH] More tests for incorrect data cases --- src/blockstore/blockstore_heap.cpp | 38 ++++++------- src/test/test_heap.cpp | 91 +++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/blockstore/blockstore_heap.cpp b/src/blockstore/blockstore_heap.cpp index 115a3a5c..687b1d82 100644 --- a/src/blockstore/blockstore_heap.cpp +++ b/src/blockstore/blockstore_heap.cpp @@ -310,21 +310,22 @@ skip_object: heap_object_t *obj = (heap_object_t *)data; if (!obj->write_pos) { - fprintf(stderr, "Warning: Object in metadata block %u at %u does not contain writes, skipping\n", block_num, block_offset); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u does not contain writes, skipping\n", + obj->inode, obj->stripe, block_num, block_offset); goto skip_corrupted; } // Verify write chain - if (obj->write_pos < -(int16_t)block_offset || obj->write_pos > (int16_t)(dsk->meta_block_size-block_offset)) + if (obj->write_pos < -(int16_t)block_offset || obj->write_pos > (int16_t)(dsk->meta_block_size-block_offset-sizeof(heap_write_t))) { - fprintf(stderr, "Warning: Object in metadata block %u at %u write offset (%d) exceeds block boundaries, skipping object\n", - block_num, block_offset, obj->write_pos); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u write offset (%d) exceeds block boundaries, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, obj->write_pos); goto skip_corrupted; } if (obj->write_pos < 0 && obj->write_pos > -sizeof(heap_write_t) || obj->write_pos > 0 && obj->write_pos < sizeof(heap_object_t)) { - fprintf(stderr, "Warning: Object in metadata block %u at %u write offset (%d) intersects the object itself, skipping object\n", - block_num, block_offset, obj->write_pos); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u write offset (%d) intersects the object itself, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, obj->write_pos); goto skip_corrupted; } uint32_t wr_i = 0; @@ -333,22 +334,21 @@ skip_object: uint32_t wr_pos = ((uint8_t*)wr - buf - buf_offset); if (wr->size != wr->get_size(this)) { - fprintf(stderr, "Warning: Object in metadata block %u at %u list entry #%u at %u size is invalid: %u instead of %u, skipping object\n", - block_num, block_offset, wr_i, wr_pos, wr->size, wr->get_size(this)); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u list entry #%u at %u size is invalid: %u instead of %u, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, wr_i, wr_pos, wr->size, wr->get_size(this)); goto skip_corrupted; } auto offset_it = offsets_seen.upper_bound({ .end = wr_pos }); if (offset_it != offsets_seen.end() && offset_it->start < wr_pos+wr->size) { - // FIXME: tests for it - fprintf(stderr, "Warning: Object in metadata block %u at %u list entry #%u (%u..%u) intersects with other entries (%u..%u) or is double-claimed, skipping object\n", - block_num, block_offset, wr_i, wr_pos, wr_pos+wr->size, offset_it->start, offset_it->end); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u list entry #%u (%u..%u) intersects with other entries (%u..%u) or is double-claimed, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, wr_i, wr_pos, wr_pos+wr->size, offset_it->start, offset_it->end); goto skip_corrupted; } if (wr->next_pos < -(int16_t)wr_pos || wr->next_pos > (int16_t)(dsk->meta_block_size - wr_pos)) { - fprintf(stderr, "Warning: Object in metadata block %u at %u list entry #%u at %u next item offset (%d) exceeds block boundaries, skipping object\n", - block_num, block_offset, wr_i, wr_pos, wr->next_pos); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u list entry #%u at %u next item offset (%d) exceeds block boundaries, skipping object\n", + obj->inode, obj->stripe, block_num, block_offset, wr_i, wr_pos, wr->next_pos); goto skip_corrupted; } offsets_seen.insert({ .start = wr_pos, .end = wr_pos+wr->size, .type = 2 }); @@ -363,14 +363,14 @@ skip_object: if (dup_obj->get_writes()->lsn >= lsn) { // Object is duplicated on disk - fprintf(stderr, "Warning: Object in metadata block %u at %u is an older duplicate, skipping\n", - block_num, block_offset); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is an older duplicate, skipping\n", + obj->inode, obj->stripe, block_num, block_offset); goto skip_object; } else { - fprintf(stderr, "Warning: Object in metadata block %u at %u is a newer duplicate, overriding\n", - block_num, block_offset); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is a newer duplicate, overriding\n", + obj->inode, obj->stripe, block_num, block_offset); erase_object(dup_block, dup_obj, 0, false); } } @@ -378,8 +378,8 @@ skip_object: uint32_t expected_crc32c = obj->calc_crc32c(); if (obj->crc32c != expected_crc32c) { - fprintf(stderr, "Warning: Object in metadata block %u at %u is corrupt (crc32c mismatch: expected %08x, got %08x), skipping\n", - block_num, block_offset, expected_crc32c, obj->crc32c); + fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is corrupt (crc32c mismatch: expected %08x, got %08x), skipping\n", + obj->inode, obj->stripe, block_num, block_offset, expected_crc32c, obj->crc32c); goto skip_corrupted; } bool to_recheck = false, to_compact = true; diff --git a/src/test/test_heap.cpp b/src/test/test_heap.cpp index a8537e7f..2ee2a552 100644 --- a/src/test/test_heap.cpp +++ b/src/test/test_heap.cpp @@ -796,6 +796,7 @@ void test_reshard_list() void _test_invalid_data_setup(blockstore_disk_t & dsk, std::vector & buffer_area, std::vector & tmp) { + tmp.clear(); tmp.resize(dsk.meta_block_size*2); heap_object_t *obj = (heap_object_t*)tmp.data(); @@ -919,11 +920,11 @@ void test_invalid_data() assert(heap.read_entry(oid, NULL)); } - // Object write size exceeds object size + // Bad write size { _test_invalid_data_setup(dsk, buffer_area, tmp); heap_object_t *obj = (heap_object_t*)tmp.data(); - obj->get_writes()->flags = BS_HEAP_BIG_WRITE|BS_HEAP_STABLE; + obj->get_writes()->size--; obj->crc32c = obj->calc_crc32c(); blockstore_heap_t heap(&dsk, buffer_area.data()); @@ -935,11 +936,97 @@ void test_invalid_data() oid = { .inode = INODE_WITH_POOL(1, 2), .stripe = 0 }; assert(heap.read_entry(oid, NULL)); + } + + // Bad write positions: + // 1) exceeds block back + // 2) exceeds block forward + // 3) intersects with object end + // 4) intersects with object beginning + for (int i = 0; i < 4; i++) + { + _test_invalid_data_setup(dsk, buffer_area, tmp); + heap_object_t *obj = (heap_object_t*)(tmp.data() + sizeof(heap_object_t) + sizeof(heap_write_t)); + if (i == 0) + obj->write_pos = -(int16_t)(sizeof(heap_object_t)+sizeof(heap_write_t)+1); + else if (i == 1) + obj->write_pos = dsk.meta_block_size-sizeof(heap_object_t)-2*sizeof(heap_write_t)+1; + else if (i == 2) + obj->write_pos = -1; + else if (i == 3) + obj->write_pos = 5; + obj->crc32c = obj->calc_crc32c(); + + blockstore_heap_t heap(&dsk, buffer_area.data()); + heap.load_blocks(0, dsk.meta_block_size*2, tmp.data()); + heap.finish_load(); + + object_id oid = { .inode = INODE_WITH_POOL(1, 3), .stripe = 0 }; + assert(!heap.read_entry(oid, NULL)); + + oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + assert(heap.read_entry(oid, NULL)); + + oid = { .inode = INODE_WITH_POOL(1, 2), .stripe = 0 }; + assert(heap.read_entry(oid, NULL)); + } + + // Object write intersects with other writes + { + _test_invalid_data_setup(dsk, buffer_area, tmp); + // Object2 Object1 BadLength Write2 + uint8_t *nb = tmp.data() + dsk.meta_block_size; + memcpy(nb, tmp.data() + sizeof(heap_object_t) + sizeof(heap_write_t), sizeof(heap_object_t)); + nb += sizeof(heap_object_t); + memcpy(nb, tmp.data(), sizeof(heap_object_t)); + nb += sizeof(heap_object_t); + *((uint16_t*)nb) = sizeof(heap_write_t) + 4; + nb += 2; + memcpy(nb, tmp.data() + 2*sizeof(heap_object_t) + sizeof(heap_write_t), sizeof(heap_write_t)); + + heap_object_t *obj = (heap_object_t*)(tmp.data() + dsk.meta_block_size); + obj->write_pos = 2*sizeof(heap_object_t) + 2; + obj->crc32c = obj->calc_crc32c(); + + obj = (heap_object_t*)(tmp.data() + dsk.meta_block_size + sizeof(heap_object_t)); + obj->write_pos = sizeof(heap_object_t); + obj->crc32c = obj->calc_crc32c(); + + memset(tmp.data(), 0, dsk.meta_block_size); + + blockstore_heap_t heap(&dsk, buffer_area.data()); + heap.load_blocks(0, dsk.meta_block_size*2, tmp.data()); + heap.finish_load(); + + object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + assert(!heap.read_entry(oid, NULL)); oid = { .inode = INODE_WITH_POOL(1, 3), .stripe = 0 }; assert(heap.read_entry(oid, NULL)); } + // Write list entry exceeds block boundaries + for (int i = 0; i < 2; i++) + { + _test_invalid_data_setup(dsk, buffer_area, tmp); + heap_object_t *obj = (heap_object_t*)tmp.data(); + obj->get_writes()->next_pos = (i == 0 ? -sizeof(heap_object_t)-1 : dsk.meta_block_size - sizeof(heap_object_t) - sizeof(heap_write_t) + 1); + obj->crc32c = obj->calc_crc32c(); + + blockstore_heap_t heap(&dsk, buffer_area.data()); + heap.load_blocks(0, dsk.meta_block_size*2, tmp.data()); + heap.finish_load(); + + object_id oid = { .inode = INODE_WITH_POOL(1, 1), .stripe = 0 }; + assert(!heap.read_entry(oid, NULL)); + + oid = { .inode = INODE_WITH_POOL(1, 3), .stripe = 0 }; + assert(heap.read_entry(oid, NULL)); + + oid = { .inode = INODE_WITH_POOL(1, 2), .stripe = 0 }; + assert(heap.read_entry(oid, NULL)); + } + printf("OK test_invalid_data\n"); }