diff --git a/src/eloq_store.cpp b/src/eloq_store.cpp index deaaeeba..1fc3bd7b 100644 --- a/src/eloq_store.cpp +++ b/src/eloq_store.cpp @@ -142,9 +142,13 @@ bool EloqStore::ValidateOptions(KvOptions &opts) LOG(ERROR) << "Option max_global_request_batch cannot be zero"; return false; } - if ((opts.data_page_size & (page_align - 1)) != 0) + if (opts.data_page_size == 0 || + (opts.data_page_size & (page_align - 1)) != 0) { - LOG(ERROR) << "Option data_page_size is not page aligned"; + // data_page_size == 0 divides by zero in the PageManager constructor; + // it also satisfies the alignment mask, so guard it explicitly. + LOG(ERROR) << "Option data_page_size (" << opts.data_page_size + << ") must be non-zero and page aligned"; return false; } // segment_size: must be page-aligned. Enforced here so release builds @@ -157,6 +161,33 @@ bool EloqStore::ValidateOptions(KvOptions &opts) << "-byte aligned"; return false; } + // A data/segment file's in-file byte offset is computed as a uint32 + // (ConvFilePageId / ConvFileSegmentId), so a single file may span at most + // 4 GiB. EloqStore is not designed for files beyond that; a larger file + // wraps offsets back onto earlier pages and silently corrupts previously + // written data, so reject such configs at startup. The shift guard also + // avoids overflowing the size_t computation in DataFileSize() / + // SegmentFileSize() for pathological shift values. + constexpr uint64_t kMaxFileSize = uint64_t{1} << 32; // 4 GiB + if (opts.pages_per_file_shift >= 32 || opts.DataFileSize() > kMaxFileSize) + { + LOG(ERROR) << "Option data_page_size << pages_per_file_shift exceeds " + "the 4 GiB per-file limit imposed by 32-bit file offsets " + "(data_page_size=" + << opts.data_page_size << ", pages_per_file_shift=" + << int(opts.pages_per_file_shift) << ")"; + return false; + } + if (opts.segments_per_file_shift >= 32 || + opts.SegmentFileSize() > kMaxFileSize) + { + LOG(ERROR) << "Option segment_size << segments_per_file_shift exceeds " + "the 4 GiB per-file limit imposed by 32-bit file offsets " + "(segment_size=" + << opts.segment_size << ", segments_per_file_shift=" + << int(opts.segments_per_file_shift) << ")"; + return false; + } if ((opts.coroutine_stack_size & (page_align - 1)) != 0) { LOG(ERROR) << "Option coroutine_stack_size is not page aligned"; diff --git a/src/kv_options.cpp b/src/kv_options.cpp index 96b73b2b..12178142 100644 --- a/src/kv_options.cpp +++ b/src/kv_options.cpp @@ -336,7 +336,18 @@ int KvOptions::LoadFromIni(const char *path) std::string value_str = reader.Get(sec_permanent, "data_page_size", "4KB"); uint64_t parsed_size = ParseSizeWithUnit(value_str); - data_page_size = (parsed_size > 0) ? parsed_size : (1 << 12); + // data_page_size is a uint16 field, so anything at/above 64KB would + // silently truncate: 64KB -> 0 (SIGFPE on the divisions below and in + // PageManager), 68KB -> 4KB. Reject out-of-range values instead of + // corrupting the config or crashing at startup. + if (parsed_size == 0 || + parsed_size > std::numeric_limits::max()) + { + LOG(ERROR) << "Invalid data_page_size '" << value_str + << "': must be non-zero and below 64KB (uint16)"; + return -3; + } + data_page_size = static_cast(parsed_size); } if (reader.HasValue(sec_permanent, "data_file_size")) { diff --git a/tests/eloq_store_test.cpp b/tests/eloq_store_test.cpp index 391143a4..b69f2f97 100644 --- a/tests/eloq_store_test.cpp +++ b/tests/eloq_store_test.cpp @@ -54,6 +54,12 @@ TEST_CASE("EloqStore ValidateOptions validates all parameters", "[eloq_store]") REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); options = CreateValidOptions(test_dir); // restore valid value + // Test data_page_size == 0 (satisfies the alignment mask but divides by + // zero in the PageManager constructor). + options.data_page_size = 0; + REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); + options = CreateValidOptions(test_dir); // restore valid value + // Test coroutine_stack_size that is not page-aligned options.coroutine_stack_size = 8193; // not page-aligned REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); @@ -149,6 +155,80 @@ TEST_CASE("EloqStore ValidateOptions validates all parameters", "[eloq_store]") CleanupTestDir(test_dir); } +// In-file byte offsets are computed as uint32 +// (ConvFilePageId / ConvFileSegmentId), so a data/segment file may span at +// most 4 GiB. ValidateOptions must reject configs whose file size exceeds that, +// otherwise offsets wrap onto earlier pages and silently corrupt data. +TEST_CASE("EloqStore ValidateOptions rejects >4GiB data/segment files", + "[eloq_store]") +{ + auto test_dir = CreateTestDir("_validate_file_size"); + auto options = CreateValidOptions(test_dir); + + // Boundary: data_page_size << pages_per_file_shift == 4 GiB is allowed + // (the largest in-file offset is 4 GiB - data_page_size, which fits + // uint32). + options.data_page_size = 4 * 1024; // 2^12 + options.pages_per_file_shift = 20; // 2^12 << 20 == 2^32 == 4 GiB + REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == true); + + // Just over 4 GiB -> reject. + options.pages_per_file_shift = 21; // 8 GiB + REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); + options = CreateValidOptions(test_dir); + + // Same limit reached via a larger page size. + options.data_page_size = 32 * 1024; // 2^15 + options.pages_per_file_shift = 18; // 2^15 << 18 == 2^33 == 8 GiB + REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); + options = CreateValidOptions(test_dir); + + // Segment file over 4 GiB -> reject. + options.segment_size = 256 * 1024; // 2^18 + options.segments_per_file_shift = 15; // 2^18 << 15 == 2^33 == 8 GiB + REQUIRE(eloqstore::EloqStore::ValidateOptions(options) == false); + + CleanupTestDir(test_dir); +} + +// data_page_size is a uint16 field, but LoadFromIni +// parsed it into a uint64 and assigned with only a >0 guard on the +// pre-truncation value -- 64KB wrapped to 0 (SIGFPE at startup), 68KB silently +// became 4KB. LoadFromIni must reject out-of-range values instead. +TEST_CASE("KvOptions LoadFromIni rejects out-of-range data_page_size", + "[eloq_store]") +{ + auto test_dir = CreateTestDir("_ini_data_page_size"); + auto write_ini = [&](const std::string &name, const std::string &page_size) + { + fs::path p = test_dir / name; + std::ofstream f(p); + f << "[run]\nnum_threads = 2\n[permanent]\ndata_page_size=" << page_size + << "\n"; + f.close(); + return p; + }; + + // 64KB overflows the uint16 field to 0 -> reject, don't truncate to 0. + { + eloqstore::KvOptions opts; + REQUIRE(opts.LoadFromIni(write_ini("bad64k.ini", "64KB").c_str()) < 0); + } + // 68KB would silently truncate to 4KB -> reject. + { + eloqstore::KvOptions opts; + REQUIRE(opts.LoadFromIni(write_ini("bad68k.ini", "68KB").c_str()) < 0); + } + // A valid in-range page size loads correctly. + { + eloqstore::KvOptions opts; + REQUIRE(opts.LoadFromIni(write_ini("ok8k.ini", "8KB").c_str()) == 0); + REQUIRE(opts.data_page_size == 8192); + } + + CleanupTestDir(test_dir); +} + TEST_CASE( "EloqStore ValidateOptions rejects bad pinned-memory / GC-pool configs", "[eloq_store]")