From f11501d0cef4422eab371a35d0d45fa2810ce577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrizio=20Hern=C3=A1ndez?= Date: Wed, 9 Sep 2026 06:38:46 -0600 Subject: [PATCH] fix(storage): validate page ranges handed out by the allocator A page index becomes a file offset on the write path exactly as it does on the read path, but only reads validated it (#946). Nothing checked the ranges the allocator returned, so a corrupted free page list silently extended the data file instead of failing: on the database from #843, PageManager returned page 212,650,604 of a 2884-page file and a checkpoint column flush wrote there, leaving an 11.8 MB database reporting 892.2 GiB of apparent size (#948). Allocated blocks barely move, so nothing looks wrong until a copy or backup materialises the sparse hole, which filled a 926 GiB volume here. PageManager::allocatePageRange now rejects a range that is not inside the data file, naming the range and the file's page count. The free list is deserialized from the database file, so a corrupted entry surfaces here first, and nothing shrinks the data file, so a legitimate free entry is always inside it. The existing DASSERT on the extension path is kept and backed by a real check, since DASSERT is stripped in release builds. Zero-page allocations are exempt: they write nothing, and their start index legitimately sits at the current end of the file. Verified against the #948 reproducer (a content-scrubbed copy of the #843 database, https://github.com/fabzter/ladybug-948-repro): Page allocation from the free page list returned pages [212650604, 212650605), which are out of bounds for a data file with 2884 pages. The free page list may be corrupted. The file stays 11.8 MB instead of reaching 892 GiB, and the error identifies the corrupted free list rather than surfacing as a SIGSEGV. Checks for false positives: 2000-cycle insert+CHECKPOINT soaks on a clean store carrying FTS and HNSW indexes pass on two seeds, and a smoke covering DDL, 3000 inserts with strings up to 3 KB, deletes that free pages, reuse of those freed pages through the free list, and EXPORT/IMPORT all pass. --- src/storage/page_manager.cpp | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/storage/page_manager.cpp b/src/storage/page_manager.cpp index 3218243174..8c78b0dff8 100644 --- a/src/storage/page_manager.cpp +++ b/src/storage/page_manager.cpp @@ -1,23 +1,60 @@ #include "storage/page_manager.h" +#include "common/exception/runtime.h" #include "common/uniq_lock.h" #include "storage/file_handle.h" #include "storage/storage_manager.h" +#include namespace lbug::storage { static constexpr bool ENABLE_FSM = true; +namespace { +// An allocated page index is turned straight into a file offset by the write path, so a range +// that does not belong to this file silently extends it rather than failing (issue #948: page +// 233,877,254 of a 2884-page database left an 11.8 MB file reporting 892 GiB of apparent size, +// which only surfaces once a copy or backup materialises the sparse hole). The free list is +// deserialized from the database file, so a corrupted entry lands here first: fail at the +// source, where the offending range can still be named. +// +// The file does shrink, via FreeSpaceManager::handleLastPageRange -> +// FileHandle::removePageIdxAndTruncateIfNecessary, but that path truncates to the start of the +// trailing free range and drops that range instead of re-adding it, and every surviving entry +// sorts below it, so a legitimate free entry stays inside the file. +void validateAllocatedPageRange(const PageRange& range, const FileHandle& fileHandle, + const char* source) { + if (range.numPages == 0) { + // A zero-page allocation writes nothing, and its start index legitimately sits at the + // current end of the file. + return; + } + const auto numPages = fileHandle.getNumPages(); + if (range.startPageIdx >= numPages || range.numPages > numPages - range.startPageIdx) { + throw common::RuntimeException( + std::format("Page allocation from {} returned pages [{}, {}), which are out of " + "bounds for a data file with {} pages. The database file may be " + "corrupted.", + source, range.startPageIdx, + static_cast(range.startPageIdx) + range.numPages, numPages)); + } +} +} // namespace + PageRange PageManager::allocatePageRange(common::page_idx_t numPages) { if constexpr (ENABLE_FSM) { common::UniqLock lck{mtx}; auto allocatedFreeChunk = freeSpaceManager->popFreePages(numPages); if (allocatedFreeChunk.has_value()) { + validateAllocatedPageRange(*allocatedFreeChunk, *fileHandle, "the free page list"); version.fetch_add(1, std::memory_order_relaxed); return {*allocatedFreeChunk}; } } auto startPageIdx = fileHandle->addNewPages(numPages); + // DASSERT alone is stripped in release builds, and this invariant guards a file offset. DASSERT(fileHandle->getNumPages() >= startPageIdx + numPages); + validateAllocatedPageRange(PageRange(startPageIdx, numPages), *fileHandle, + "the page extension path"); return PageRange(startPageIdx, numPages); }