mirror of
https://github.com/smartcmd/MinecraftConsoles.git
synced 2026-08-20 09:57:09 +00:00
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.
This commit is contained in:
parent
0545e15c48
commit
8583e99d94
|
|
@ -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();
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue