From 5674c789efb549d3279004e9355c394e07c527e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 28 Sep 2026 15:25:05 -0600 Subject: [PATCH] sceUmd: Match hardware's parameter checks and wait timeouts - A timeout of 0 to sceUmdWaitDriveStatWithTimer/CB means no timeout, not a tiny one (or 8ms for the CB version). - Timeouts round like the event flag wait does. - A wait with no timeout no longer times out right after a callback. - sceUmdRegisterUMDCallBack only accepts callbacks. - sceUmdActivate requires the name to be exactly "disc0:", and it and sceUmdDeactivate/sceUmdGetDiscInfo reject kernel pointers. - sceUmdDeactivate needs a name in mode 2. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/HLE/sceUmd.cpp | 54 ++++++++++++++++++++++++++++----------------- pspautotests | 2 +- test.py | 1 + 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Core/HLE/sceUmd.cpp b/Core/HLE/sceUmd.cpp index d9fc257606..fec369bbc0 100644 --- a/Core/HLE/sceUmd.cpp +++ b/Core/HLE/sceUmd.cpp @@ -252,7 +252,9 @@ static void __UmdEndCallback(SceUID threadID, SceUID prevCallbackId) else { _dbg_assert_msg_(umdStatTimeoutEvent != -1, "Must have a umd timer"); - CoreTiming::ScheduleEvent(cyclesLeft, umdStatTimeoutEvent, __KernelGetCurThread()); + // A deadline of 0 means the wait has no timeout. + if (waitDeadline != 0) + CoreTiming::ScheduleEvent(cyclesLeft, umdStatTimeoutEvent, __KernelGetCurThread()); umdWaitingThreads.push_back(threadID); @@ -268,10 +270,15 @@ static int sceUmdCheckMedium() { return hleLogDebug(Log::sceKernel, retVal); } +// A user mode caller can't pass a kernel address. (mediaman.prx checks these with k1.) +static bool IsUserAddress(u32 addr) { + return (addr & 0x80000000) == 0; +} + static u32 sceUmdGetDiscInfo(u32 infoAddr) { DEBUG_LOG(Log::sceIo, "sceUmdGetDiscInfo(%08x)", infoAddr); - if (Memory::IsValidAddress(infoAddr)) { + if (Memory::IsValidRange(infoAddr, 8) && IsUserAddress(infoAddr) && IsUserAddress(infoAddr + 4)) { auto info = PSPPointer::Create(infoAddr); if (info->size != 8) return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); @@ -283,9 +290,13 @@ static u32 sceUmdGetDiscInfo(u32 infoAddr) { } } -static int sceUmdActivate(u32 mode, const char *name) { +static int sceUmdActivate(u32 mode, u32 namePtr) { if (mode < 1 || mode > 2) return hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); + // The firmware compares the name before checking the pointer, so a bad one crashes there. + const char *name = namePtr != 0 && Memory::IsValidAddress(namePtr) ? Memory::GetCharPointer(namePtr) : nullptr; + if (!name || strncmp(name, "disc0:", 7) != 0 || !IsUserAddress(namePtr)) + return hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad name"); __KernelUmdActivate(); @@ -295,11 +306,14 @@ static int sceUmdActivate(u32 mode, const char *name) { return hleLogDebug(Log::sceIo, 0); } -static int sceUmdDeactivate(u32 mode, const char *name) +static int sceUmdDeactivate(u32 mode, u32 namePtr) { // Why 18? No idea. if (mode > 18) return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT); + // Unlike sceUmdActivate(), the name isn't compared, and only mode 2 requires one. + if ((mode == 2 && namePtr == 0) || !IsUserAddress(namePtr)) + return hleLogError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad name"); __KernelUmdDeactivate(); @@ -314,8 +328,7 @@ static u32 sceUmdRegisterUMDCallBack(u32 cbId) { int retVal = 0; - // TODO: If the callback is invalid, return SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT. - if (!kernelObjects.IsValid(cbId)) { + if (!kernelObjects.Is(cbId)) { retVal = SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT; } else { // There's only ever one. @@ -363,13 +376,18 @@ static void __UmdStatTimeout(u64 userdata, int cyclesLate) HLEKernel::RemoveWaitingThread(umdWaitingThreads, threadID); } -static void __UmdWaitStat(u32 timeout) +// The firmware waits on an event flag holding the drive state (mediaman.prx), and passes no +// timeout at all for 0, so that waits forever. +static void __UmdWaitStat(u32 timeout, bool callbacks) { - // This happens to be how the hardware seems to time things. - if (timeout <= 4) - timeout = 15; - else if (timeout <= 215) - timeout = 250; + if (timeout == 0) + return; + + // Measured on hardware. Oddly, the CB version doesn't have the shortest step. + if (timeout <= 1 && !callbacks) + timeout = 25; + else if (timeout <= 209) + timeout = 240; CoreTiming::ScheduleEvent(usToCycles((int) timeout), umdStatTimeoutEvent, __KernelGetCurThread()); } @@ -416,7 +434,7 @@ static int sceUmdWaitDriveStatWithTimer(u32 stat, u32 timeout) { hleEatCycles(520); if ((stat & __KernelUmdGetState()) == 0) { - __UmdWaitStat(timeout); + __UmdWaitStat(timeout, false); umdWaitingThreads.push_back(__KernelGetCurThread()); __KernelWaitCurThread(WAITTYPE_UMD, 1, stat, 0, false, "umd stat waited with timer"); return hleLogDebug(Log::sceIo, 0, "waiting"); @@ -441,11 +459,7 @@ static int sceUmdWaitDriveStatCB(u32 stat, u32 timeout) { hleEatCycles(520); hleCheckCurrentCallbacks(); if ((stat & __KernelUmdGetState()) == 0) { - if (timeout == 0) { - timeout = 8000; - } - - __UmdWaitStat(timeout); + __UmdWaitStat(timeout, true); umdWaitingThreads.push_back(__KernelGetCurThread()); __KernelWaitCurThread(WAITTYPE_UMD, 1, stat, 0, true, "umd stat waited"); return hleLogDebug(Log::sceIo, 0, "waiting"); @@ -519,10 +533,10 @@ static u32 sceUmdReplacePermit() { const HLEFunction sceUmdUser[] = { - {0XC6183D47, &WrapI_UC, "sceUmdActivate", 'i', "is"}, + {0XC6183D47, &WrapI_UU, "sceUmdActivate", 'i', "is"}, {0X6B4A146C, &WrapU_V, "sceUmdGetDriveStat", 'x', "" }, {0X46EBB729, &WrapI_V, "sceUmdCheckMedium", 'i', "" }, - {0XE83742BA, &WrapI_UC, "sceUmdDeactivate", 'i', "xs"}, + {0XE83742BA, &WrapI_UU, "sceUmdDeactivate", 'i', "xs"}, {0X8EF08FCE, &WrapI_U, "sceUmdWaitDriveStat", 'i', "x" }, {0X56202973, &WrapI_UU, "sceUmdWaitDriveStatWithTimer", 'i', "xx"}, {0X4A9E5E29, &WrapI_UU, "sceUmdWaitDriveStatCB", 'i', "xx"}, diff --git a/pspautotests b/pspautotests index 33e59e29f2..fa87488fb7 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 33e59e29f2aad3c7440e5b1afeab1715268d72dc +Subproject commit fa87488fb7f034ce006490285297ca290d8d23b7 diff --git a/test.py b/test.py index 237ec26b82..dbf08f80d1 100755 --- a/test.py +++ b/test.py @@ -450,6 +450,7 @@ tests_good = [ "utility/savedata/getsize", "utility/savedata/makedata", "utility/systemparam/systemparam", + "umd/api/api", "umd/callbacks/umd", "umd/wait/wait", "umd/register",