From 8583e99d947ad29d559305d3c8f62563cbe3ca5c Mon Sep 17 00:00:00 2001 From: MrTheShy <49885496+MrTheShy@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:23:05 +0100 Subject: [PATCH] Fix crash in WriteHeader when save buffer is too small for header table When a player enters a new region, RegionFile's constructor calls createFile which adds a FileEntry with length 0 to the file table. This increases the header table size (appended at the end of the save buffer) by sizeof(FileEntrySaveData) per entry, but since no actual data is written to the file, MoveDataBeyond is never called and the committed virtual memory pages are never grown to match. On the next autosave tick, saveLevelData writes level.dat first (before chunkSource->save which would have grown the buffer). If level.dat doesn't need to grow, finalizeWrite calls WriteHeader which tries to memcpy the now-larger header table past the end of committed memory, causing an access violation. This is especially likely in splitscreen where two players exploring at the same time can create multiple new RegionFile entries within a single tick, quickly exhausting the page-alignment slack in the buffer (yes i am working at splitscreen in the meanwhile :) ) The fix was deduced by tracing the crash callstack through the save system: FileHeader, ConsoleSaveFileOriginal, the stream chain, and the RegionFile/RegionFileCache layer. The root cause turned out to be a gap between createFile (which grows the header table) and MoveDataBeyond (the only place that grows the buffer), with finalizeWrite sitting right in between unprotected. The buffer growth check added here mirrors the exact same VirtualAlloc pattern already used in MoveDataBeyond (line 484-497) and in the constructor's decompression path (line 176-190), so it integrates naturally with the existing code. Same types, same page rounding, same error handling. The fast path (no new entries, buffer already big enough) is a single DWORD comparison that doesn't get taken, so there is zero overhead in the common case. This is the right place for the fix because finalizeWrite is the sole caller of WriteHeader, meaning every code path that writes the header (closeHandle, PrepareForWrite, deleteFile, Flush) is now protected by a single check point. --- Minecraft.World/ConsoleSaveFileOriginal.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Minecraft.World/ConsoleSaveFileOriginal.cpp b/Minecraft.World/ConsoleSaveFileOriginal.cpp index 3a2b5ef11..d4db4e25e 100644 --- a/Minecraft.World/ConsoleSaveFileOriginal.cpp +++ b/Minecraft.World/ConsoleSaveFileOriginal.cpp @@ -463,6 +463,23 @@ BOOL ConsoleSaveFileOriginal::closeHandle( FileEntry *file ) void ConsoleSaveFileOriginal::finalizeWrite() { LockSaveAccess(); + + // Ensure buffer is large enough for the full file including header table. + // New file entries (e.g. from RegionFile creation) increase GetFileSize() + // without triggering MoveDataBeyond, so the committed pages may be short. + DWORD currentHeapSize = pagesCommitted * CSF_PAGE_SIZE; + DWORD desiredSize = header.GetFileSize(); + if( desiredSize > currentHeapSize ) + { + unsigned int pagesRequired = ( desiredSize + (CSF_PAGE_SIZE - 1 ) ) / CSF_PAGE_SIZE; + void *pvRet = VirtualAlloc(pvHeap, pagesRequired * CSF_PAGE_SIZE, COMMIT_ALLOCATION, PAGE_READWRITE); + if( pvRet == NULL ) + { + __debugbreak(); + } + pagesCommitted = pagesRequired; + } + header.WriteHeader( pvSaveMem ); ReleaseSaveAccess(); }