More tests for incorrect data cases

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",
+                fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u write offset (%d) exceeds block boundaries, skipping object\n",
-                    block_num, block_offset, obj->write_pos);
+                    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",
+                fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u write offset (%d) intersects the object itself, skipping object\n",
-                    block_num, block_offset, obj->write_pos);
+                    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",
+                    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",
-                        block_num, block_offset, wr_i, wr_pos, wr->size, wr->get_size(this));
+                        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 %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",
-                    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",
+                        obj->inode, obj->stripe, block_num, block_offset, wr_i, wr_pos, wr_pos+wr->size, offset_it->start, offset_it->end);
-                        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",
+                    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",
-                        block_num, block_offset, wr_i, wr_pos, wr->next_pos);
+                        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",
+                    fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is an older duplicate, skipping\n",
-                        block_num, block_offset);
+                        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",
+                    fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is a newer duplicate, overriding\n",
-                        block_num, block_offset);
+                        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",
+                fprintf(stderr, "Warning: Object %jx:%jx in metadata block %u at %u is corrupt (crc32c mismatch: expected %08x, got %08x), skipping\n",
-                    block_num, block_offset, expected_crc32c, obj->crc32c);
+                    obj->inode, obj->stripe, block_num, block_offset, expected_crc32c, obj->crc32c);
                 goto skip_corrupted;
             }
             bool to_recheck = false, to_compact = true;

src/test/test_heap.cpp

@@ -796,6 +796,7 @@ void test_reshard_list()
 
 void _test_invalid_data_setup(blockstore_disk_t & dsk, std::vector<uint8_t> & buffer_area, std::vector<uint8_t> & 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");
 }