From 937337d228f9d5e625d543a0d54100315148c063 Mon Sep 17 00:00:00 2001 From: Changwei Ye Date: Fri, 28 Aug 2026 16:23:23 +0800 Subject: [PATCH] Fix false-positive -EDOM checksum errors on overlapping unaligned reads When csum_block_size > bitmap_granularity and perfect_csum_update = false (the default behavior), an unaligned INTENT_WRITE modifies the physical block directly in-place without recalculating and updating the large block's checksum. During a subsequent read that requires padding to the checksum block boundary (e.g., reading adjacent unaligned data), the read logic must skip checksum verification for the underlying BIG_WRITE because the physical data has been safely tampered with by the INTENT_WRITE. Previously, the need_skip logic in blockstore_read.cpp correctly identified and skipped the verification for overlapping SMALL_WRITEs but mistakenly omitted INTENT_WRITEs. This caused the padded read to verify the stale BIG_WRITE checksum against the newly modified physical data, leading to a fatal false-positive -EDOM (checksum mismatch) IO error on reads. This commit fixes the bug by ensuring BS_HEAP_INTENT_WRITE is evaluated alongside SMALL_WRITE to apply the COPY_BUF_SKIP_CSUM bypass during overlapping padded reads. A regression test case (test_intent_write_corruption) is also added to reproduce this specific data-layout scenario and verify the fix. --- src/blockstore/blockstore_read.cpp | 3 +- src/test/test_blockstore.cpp | 58 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/blockstore/blockstore_read.cpp b/src/blockstore/blockstore_read.cpp index b3782dde..4a571f00 100644 --- a/src/blockstore/blockstore_read.cpp +++ b/src/blockstore/blockstore_read.cpp @@ -75,10 +75,11 @@ int blockstore_impl_t::dequeue_read(blockstore_op_t *op) return false; } } - if (need_skip && wr->type() == BS_HEAP_SMALL_WRITE && + if (need_skip && (wr->type() == BS_HEAP_SMALL_WRITE || wr->type() == BS_HEAP_INTENT_WRITE) && wr->small().offset < blk_end && wr->small().offset+wr->small().len > blk_start) { // Small write may mutate big write checksums during flush + // Intent write directly mutates physical data in-place skip_csum = COPY_BUF_SKIP_CSUM; } return true; diff --git a/src/test/test_blockstore.cpp b/src/test/test_blockstore.cpp index ba734dde..1dec1286 100644 --- a/src/test/test_blockstore.cpp +++ b/src/test/test_blockstore.cpp @@ -1091,6 +1091,63 @@ static void test_list_limit() free(op.buf); } +static void test_intent_write_corruption() +{ + printf("\n-- test_intent_write_corruption\n"); + + bs_test_t test; + test.default_cfg(); + test.config["csum_block_size"] = "16384"; + test.config["bitmap_granularity"] = "4096"; + test.config["perfect_csum_update"] = "false"; + test.init(); + + // 1. Write BIG_WRITE (0-48K) + blockstore_op_t op; + op.opcode = BS_OP_WRITE_STABLE; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = 1; + op.offset = 0; + op.len = 49152; // 48K + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + memset(op.buf, 0xaa, op.len); + test.exec_op(&op); + assert(op.retval == op.len); + + + // 2. Write unaligned INTENT_WRITE B (8K-12K) + free(op.buf); + op.opcode = BS_OP_WRITE_STABLE; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = 2; + op.offset = 8192; // 8K + op.len = 4096; // 4K + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + memset(op.buf, 0xbb, op.len); + test.exec_op(&op); + assert(op.retval == op.len); + + // 3. Read 12-40K + // This will hit the hole logic where B provided 8-12K, but we don't even request it. + // The request is for 12-40K. + // 12K is not aligned to 16K, so A must pad by reading 0-16K. + // However, 8-12K of the physical block has been overwritten by B. + // A will try to verify the 0-16K block with its old checksum and fail. + free(op.buf); + op.opcode = BS_OP_READ; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = UINT64_MAX; // latest version + op.offset = 12288; // 12K + op.len = 28672; // 28K (so end = 40K) + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + test.exec_op(&op); + + // Assert that the read succeeds without checksum corruption errors + assert(op.retval == op.len); + + free(op.buf); +} + // FIXME Add a simple intent_write / big_intent test int main(int narg, char *args[]) @@ -1100,6 +1157,7 @@ int main(int narg, char *args[]) test_fsync(true); test_fsync_meta_before_complete(); test_intent_over_unstable(); + test_intent_write_corruption(); test_padded_csum_intent(false); test_padded_csum_intent(true); test_padded_csum_sparse_leading_hole(); -- 2.47.3