From b6a60a7d4947c582941b96bdca82eecc7344bdbe Mon Sep 17 00:00:00 2001 From: Jerry Zhao Date: Thu, 9 Jul 2026 16:30:27 +0000 Subject: [PATCH 1/2] fix: reject large values over the 2 GiB on-disk length cap The very-large-value on-disk header encodes the length in 31 bits: bit 31 of word0 is the has-metadata flag and bits 30..0 hold actual_length (kLargeValueLengthMask = 2 GiB - 1). WriteLargeValue cast the caller's value size (size_t) straight to uint32 -- both the IoStringBuffer path (iosb->Size()) and the pinned path (pinned->second) -- with only a debug-only assert in EncodeLargeValueContent guarding the cap. A value in [2 GiB, 4 GiB) set the metadata flag and stored a wrong length; >= 4 GiB truncated outright. Either way the segment-id array no longer matches what DecodeLargeValueHeader derives, so every subsequent read returns Corrupted and a later delete dereferences a disengaged optional. EloqStore does not support values this large. Validate the length against kLargeValueLengthMask before the cast on both paths and return InvalidArgs when it is exceeded. The check runs before any memory is dereferenced. Adds a large_value_concurrency regression test: a pinned write with a declared length of 2 GiB (over a small real chunk) returns InvalidArgs instead of asserting/corrupting. --- src/tasks/batch_write_task.cpp | 16 ++++++++++++++++ tests/large_value_concurrency.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/tasks/batch_write_task.cpp b/src/tasks/batch_write_task.cpp index d7093b51..6edf4309 100644 --- a/src/tasks/batch_write_task.cpp +++ b/src/tasks/batch_write_task.cpp @@ -1482,6 +1482,16 @@ KvError BatchWriteTask::WriteLargeValue(const WriteDataEntry &entry) std::get_if(&entry.large_val_); iosb != nullptr) { + // The on-disk large-value header encodes the length in 31 bits (bit 31 + // is the has-metadata flag), so a value longer than + // kLargeValueLengthMask (2 GiB - 1) cannot be represented: casting to + // uint32 would truncate it and/or collide with the metadata flag, + // corrupting the blob so every later read fails. EloqStore does not + // support values this large; reject. + if (iosb->Size() > kLargeValueLengthMask) + { + return KvError::InvalidArgs; + } const auto &fragments = iosb->Fragments(); const uint32_t num_segments = fragments.size(); std::vector ptrs(num_segments); @@ -1504,6 +1514,12 @@ KvError BatchWriteTask::WriteLargeValue(const WriteDataEntry &entry) const size_t size = pinned->second; assert(base != nullptr); assert(size > 0); + // See the IoStringBuffer path above: values longer than + // kLargeValueLengthMask (2 GiB - 1) cannot be encoded on disk. + if (size > kLargeValueLengthMask) + { + return KvError::InvalidArgs; + } const uint32_t num_segments = static_cast((size + seg_size - 1) / seg_size); diff --git a/tests/large_value_concurrency.cpp b/tests/large_value_concurrency.cpp index a97681e1..b0f682b8 100644 --- a/tests/large_value_concurrency.cpp +++ b/tests/large_value_concurrency.cpp @@ -978,3 +978,33 @@ TEST_CASE( store->Stop(); CleanupStore(opts); } + +// Audit finding rank 22: a large value whose length exceeds +// kLargeValueLengthMask (2 GiB - 1) cannot be represented in the 31-bit +// on-disk length field (bit 31 is the has-metadata flag). WriteLargeValue must +// reject it up front instead of casting the length to uint32 -- which would +// truncate it and/or collide with the metadata flag, corrupting the blob so +// every later read fails. The guard runs before any memory is dereferenced, so +// a huge declared length over a small real pinned chunk exercises it without +// allocating gigabytes. +TEST_CASE("pinned large value over the 2 GiB length cap is rejected", + "[large-value-concurrency][pinned]") +{ + PinnedMultiShardHarness h(kSegmentSize, /*num_shards=*/1); + eloqstore::KvOptions opts = + MakePinnedOpts(h, /*pinned_tail_scratch_slots=*/2); + eloqstore::EloqStore *store = InitStore(opts); + + eloqstore::TableIdent tbl{"lv-cap", 0}; + // 2 GiB == kLargeValueHasMetadataBit: one byte past the 31-bit length cap. + const size_t too_large = size_t{1} << 31; + std::pair dst{h.Base(0), too_large}; + + eloqstore::BatchWriteRequest req; + AsyncPinnedWrite(store, req, tbl, "big", dst, /*metadata=*/"", /*ts=*/1); + req.Wait(); + REQUIRE(req.Error() == eloqstore::KvError::InvalidArgs); + + store->Stop(); + CleanupStore(opts); +} From 5990aa9df152f0e6693e060b2a382ceee0fe84dd Mon Sep 17 00:00:00 2001 From: Jerry Zhao Date: Fri, 10 Jul 2026 04:22:14 +0000 Subject: [PATCH 2/2] test: describe the bug directly, without an internal audit reference --- tests/large_value_concurrency.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/large_value_concurrency.cpp b/tests/large_value_concurrency.cpp index b0f682b8..32b5c72d 100644 --- a/tests/large_value_concurrency.cpp +++ b/tests/large_value_concurrency.cpp @@ -979,7 +979,7 @@ TEST_CASE( CleanupStore(opts); } -// Audit finding rank 22: a large value whose length exceeds +// A large value whose length exceeds // kLargeValueLengthMask (2 GiB - 1) cannot be represented in the 31-bit // on-disk length field (bit 31 is the has-metadata flag). WriteLargeValue must // reject it up front instead of casting the length to uint32 -- which would