From 30cab94840e1524e6dde75abf9d4a313ca6471e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 19 Oct 2026 21:27:21 +0200 Subject: [PATCH] SaveState: Fixes from a review of the kernel savestates - A partition memory block restored its allocator through BlockAllocatorFromID, whose privilege check asked the thread current before the load, already deleted with the kernel objects. Loading a kernel-partition block under a user thread left it without an allocator, crashing when it was freed. The saved id maps straight back now, and the current thread is cleared along with the objects. - The thread LoadExec leaves waiting (with no timeout) for plugins and VSH modules to start wasn't saved, so a state saved while one was starting hung the game thread after loading. The VSH modules still to start weren't saved either, nor cleared. - A state without the exit callback id left the previous session's. - From a broken or crafted state: thread ids that GetFast trusts (current, idle and ready), interrupt handler numbers, FPL and TLSPL indices, and a MIPS call or action slot that isn't there. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/HLE/HLE.cpp | 2 +- Core/HLE/ThreadQueueList.h | 15 +++++++++++++++ Core/HLE/sceKernel.cpp | 9 +++++++-- Core/HLE/sceKernelInterrupt.cpp | 4 ++++ Core/HLE/sceKernelMemory.cpp | 18 +++++++++++++++++- Core/HLE/sceKernelModule.cpp | 20 ++++++++++++++++++-- Core/HLE/sceKernelThread.cpp | 27 +++++++++++++++++++++++++++ Core/HLE/sceKernelThread.h | 2 ++ 8 files changed, 91 insertions(+), 6 deletions(-) diff --git a/Core/HLE/HLE.cpp b/Core/HLE/HLE.cpp index f3e12f4d05..ff0808b005 100644 --- a/Core/HLE/HLE.cpp +++ b/Core/HLE/HLE.cpp @@ -885,7 +885,7 @@ void HLEReturnFromMipsCall() { return; } - if (stackData->actionIndex != 0xFFFFFFFF && stackData->actionIndex < (u32)mipsCallActions.size()) { + if (stackData->actionIndex != 0xFFFFFFFF && stackData->actionIndex < (u32)mipsCallActions.size() && mipsCallActions[stackData->actionIndex] != nullptr) { PSPAction *&action = mipsCallActions[stackData->actionIndex]; VERBOSE_LOG(Log::HLE, "Executing action for HLE mips call at %08x, sp=%08x", stackData->func, sp); diff --git a/Core/HLE/ThreadQueueList.h b/Core/HLE/ThreadQueueList.h index bedd7de15f..125eee976c 100644 --- a/Core/HLE/ThreadQueueList.h +++ b/Core/HLE/ThreadQueueList.h @@ -61,6 +61,21 @@ struct ThreadQueueList { clear(); } + // Whether func is true for every queued thread. + template + bool all_of(F func) const { + for (int i = 0; i < NUM_QUEUES; ++i) { + const Queue *cur = &queues[i]; + if (cur->data == nullptr) + continue; + for (int j = cur->first; j < cur->end; ++j) { + if (!func(cur->data[j])) + return false; + } + } + return true; + } + // Only for debugging, returns priority level. int contains(const SceUID uid) { for (int i = 0; i < NUM_QUEUES; ++i) { diff --git a/Core/HLE/sceKernel.cpp b/Core/HLE/sceKernel.cpp index 43f32907c5..aaf5214cf2 100644 --- a/Core/HLE/sceKernel.cpp +++ b/Core/HLE/sceKernel.cpp @@ -250,8 +250,11 @@ void __KernelDoState(PointerWrap &p) Do(p, kernelRunning); kernelObjects.DoState(p); - if (s >= 2) + if (s >= 2) { Do(p, registeredExitCbId); + } else if (p.mode == p.MODE_READ) { + registeredExitCbId = 0; + } } { @@ -583,7 +586,9 @@ void KernelObjectPool::DoState(PointerWrap &p) { } if (p.mode == p.MODE_READ) { - hleCurrentThreadName = nullptr; + // The current thread is deleted with the rest. Until sceKernelThread loads the new one, + // nothing should look at it. + __KernelForgetCurrentThread(); kernelObjects.Clear(); } diff --git a/Core/HLE/sceKernelInterrupt.cpp b/Core/HLE/sceKernelInterrupt.cpp index f1a6b6492a..b76c09e727 100644 --- a/Core/HLE/sceKernelInterrupt.cpp +++ b/Core/HLE/sceKernelInterrupt.cpp @@ -218,6 +218,10 @@ void IntrHandler::DoState(PointerWrap &p) Do(p, intrNumber); Do(p, subIntrHandlers); + if (p.mode == p.MODE_READ && (intrNumber < 0 || intrNumber >= PSP_NUMBER_INTERRUPTS)) { + ERROR_LOG(Log::sceIntc, "Savestate failure: invalid interrupt handler %d", intrNumber); + p.SetError(p.ERROR_FAILURE); + } } void PendingInterrupt::DoState(PointerWrap &p) diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index c129a8307f..3ae2e0c000 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -117,6 +117,10 @@ void FPL::DoState(PointerWrap &p) { FplWaitingThread dv = { 0 }; Do(p, waitingThreads, dv); Do(p, pausedWaits); + // It's taken modulo the block count, but a negative one stays negative. + if (p.mode == p.MODE_READ && nextBlock < 0) { + p.SetError(p.ERROR_FAILURE); + } } void SceKernelVplHeader::Init(u32 ptr, u32 size) { @@ -834,7 +838,14 @@ public: if (s >= 2) { int allocType = BlockAllocatorToID(alloc); Do(p, allocType); - alloc = BlockAllocatorFromID(allocType); + // From BlockAllocatorToID, so not a game's partition id: map it back without + // BlockAllocatorFromID's privilege check, which asks about whatever thread is current. + switch (allocType) { + case KERNEL_PARTITION_ID: alloc = &kernelMemory; break; + case USER_PARTITION_ID: alloc = &userMemory; break; + case VSHELL_PARTITION_ID: alloc = &volatileMemory; break; + default: alloc = nullptr; break; + } } } @@ -1699,6 +1710,11 @@ struct TLSPL : public KernelObject { Do(p, waitingThreads); Do(p, next); Do(p, usage); + // Both index arrays. + if (p.mode == p.MODE_READ && (ntls.index < 0 || ntls.index >= TLSPL_NUM_INDEXES || usage.size() != (size_t)ntls.totalBlocks)) { + ERROR_LOG(Log::SaveState, "TLSPL: bad index %d or block count %d (%d used) in savestate", ntls.index, ntls.totalBlocks, (int)usage.size()); + p.SetError(p.ERROR_FAILURE); + } } NativeTlspl ntls; diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index a48c46ba6a..2bdda32fe1 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -239,7 +239,7 @@ u32 PSPModule::GetMissingErrorCode() { } void PSPModule::DoState(PointerWrap &p) { - auto s = p.Section("Module", 1, 6); + auto s = p.Section("Module", 1, 7); if (!s) return; @@ -285,6 +285,14 @@ void PSPModule::DoState(PointerWrap &p) { if (s >= 5) { Do(p, modulePtr.ptr); } + if (s >= 7) { + // LoadExec's thread waits for plugins (and VSH modules) to start, with no timeout. + Do(p, pluginWaitingThread); + Do(p, startingPlugins); + } else if (p.mode == p.MODE_READ) { + pluginWaitingThread = 0; + startingPlugins.clear(); + } ModuleWaitingThread mwt = { 0 }; Do(p, waitingThreads, mwt); @@ -555,7 +563,7 @@ std::vector> __KernelModuleCodeRanges() { } void __KernelModuleDoState(PointerWrap &p) { - auto s = p.Section("sceKernelModule", 1, 5); + auto s = p.Section("sceKernelModule", 1, 6); if (!s) return; @@ -596,6 +604,14 @@ void __KernelModuleDoState(PointerWrap &p) { } CoreTiming::RestoreRegisterEvent(eventModuleLoadDone, "LoadModuleDone", &__KernelLoadModuleDone); + if (s >= 6) { + Do(p, g_vshModulesToStart); + Do(p, g_vshModuleStarting); + } else if (p.mode == p.MODE_READ) { + g_vshModulesToStart.clear(); + g_vshModuleStarting = 0; + } + if (p.mode == p.MODE_READ) { u32 error; // We process these late, since they depend on loadedModules for interlinking. diff --git a/Core/HLE/sceKernelThread.cpp b/Core/HLE/sceKernelThread.cpp index 5bd4a67f7f..ae972af596 100644 --- a/Core/HLE/sceKernelThread.cpp +++ b/Core/HLE/sceKernelThread.cpp @@ -647,6 +647,10 @@ inline void __SetCurrentThread(PSPThread *thread, SceUID threadID, const char *n hleCurrentThreadName = name; } +void __KernelForgetCurrentThread() { + __SetCurrentThread(nullptr, 0, nullptr); +} + u32 __KernelCallbackReturnAddress() { return cbReturnHackAddr; } @@ -945,6 +949,25 @@ void __KernelThreadingDoState(PointerWrap &p) g_chanceAccumulator = 50; } + if (p.mode == p.MODE_READ) { + // GetFast trusts all of these to be threads. + u32 error; + auto isThread = [&](SceUID id) { + return id == 0 || kernelObjects.Get(id, error) != nullptr; + }; + bool valid = isThread(currentThread) && threadReadyQueue.all_of(isThread); + for (SceUID id : threadIdleID) { + valid = valid && isThread(id); + } + if (!valid) { + ERROR_LOG(Log::SaveState, "Unable to load state: current, idle or ready thread isn't a thread"); + threadReadyQueue.clear(); + currentThread = 0; + p.SetError(p.ERROR_FAILURE); + return; + } + } + __SetCurrentThread(kernelObjects.GetFast(currentThread), currentThread, __KernelGetThreadName(currentThread)); lastSwitchCycles = CoreTiming::GetTicks(currentMIPS); @@ -3574,6 +3597,10 @@ void __KernelReturnFromMipsCall() { u32 callId = cur->currentMipscallId; MipsCall *call = mipsCalls.pop(callId); + if (!call) { + ERROR_LOG(Log::sceKernel, "__KernelReturnFromMipsCall(): No call %d", callId); + return hleNoLogVoid(); + } // Value returned by the callback function u32 retVal = currentMIPS->r[MIPS_REG_V0]; diff --git a/Core/HLE/sceKernelThread.h b/Core/HLE/sceKernelThread.h index 35d3996aa3..a6cd9522da 100644 --- a/Core/HLE/sceKernelThread.h +++ b/Core/HLE/sceKernelThread.h @@ -352,6 +352,8 @@ KernelObject *__KernelThreadObject(); KernelObject *__KernelCallbackObject(); SceUID __KernelGetCurThread(); +// Before a load deletes the kernel objects. +void __KernelForgetCurrentThread(); int KernelCurThreadPriority(); bool KernelChangeThreadPriority(SceUID threadID, int priority); // Whether the running thread belongs to a kernel module. Privilege on the PSP is a property of