From 93cb3a1b0db5e867ac8dc115cb62882a6f3e9fb7 Mon Sep 17 00:00:00 2001 From: Hannah Lyons <7751607+babyhuey@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:40:52 -0400 Subject: [PATCH] fix(libkernel): compare the full WAIT_UINT value and throw on _umtx_op operation 1 --- .../libkernel/SyncOnAddress/src/SyncOnAddress.cpp | 13 +++++++++---- core/libs/tests/GuestUmtxOp.cpp | 11 +++++++++-- docs/dev/TechnicalDebt.md | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp b/core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp index 021cc029c..8cf478bed 100644 --- a/core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp +++ b/core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp @@ -155,7 +155,7 @@ int UmtxDeadline(const void* sizeArgument, const void* timeArgument, std::option } template -int UmtxWait(void* object, TValue expected, const void* sizeArgument, const void* timeArgument, const void* caller) { +int UmtxWait(void* object, std::uint64_t expected, const void* sizeArgument, const void* timeArgument, const void* caller) { std::optional deadline; if (const int error = UmtxDeadline(sizeArgument, timeArgument, deadline); error != 0) { return error; @@ -166,7 +166,10 @@ int UmtxWait(void* object, TValue expected, const void* sizeArgument, const void if (!IsAlignedAddress(reinterpret_cast(object))) { APS5_INVALID_ARG_EX; } - return WaitOnAddressUntil(static_cast(object), expected, deadline, caller) ? 0 : GUEST_ETIMEDOUT; + if (expected > std::numeric_limits::max()) { + return 0; + } + return WaitOnAddressUntil(static_cast(object), static_cast(expected), deadline, caller) ? 0 : GUEST_ETIMEDOUT; } int UmtxWake(void* object, std::uint64_t count, bool isPrivate) { @@ -240,7 +243,7 @@ int APS5_VABI _umtx_op_nid_postfix(void* object, int operation, std::uint64_t va break; case UMTX_OP_WAIT_UINT: case UMTX_OP_WAIT_UINT_PRIVATE: - error = UmtxWait(object, static_cast(value), sizeArgument, timeArgument, caller); + error = UmtxWait(object, value, sizeArgument, timeArgument, caller); break; case UMTX_OP_WAKE: case UMTX_OP_WAKE_PRIVATE: @@ -249,7 +252,9 @@ int APS5_VABI _umtx_op_nid_postfix(void* object, int operation, std::uint64_t va default: if (operation < 0 || operation >= UMTX_OP_MAX) { error = GUEST_EINVAL; - } else if (operation <= UMTX_OP_RESERVED1) { + } else if (operation == UMTX_OP_RESERVED1) { + NotImplemented_nid_no_patch("_umtx_op operation 1"); + } else if (operation < UMTX_OP_RESERVED1) { error = GUEST_EOPNOTSUPP; } else { NotImplemented_nid_no_patch(("_umtx_op operation " + std::to_string(operation)).c_str()); diff --git a/core/libs/tests/GuestUmtxOp.cpp b/core/libs/tests/GuestUmtxOp.cpp index 9755aba0b..1c33f00d7 100644 --- a/core/libs/tests/GuestUmtxOp.cpp +++ b/core/libs/tests/GuestUmtxOp.cpp @@ -80,7 +80,11 @@ static void WaitComparesALongAndWaitUintAnInt() { auto* low = reinterpret_cast(&value); Require(FailsWith(Umtx(low, UMTX_OP_WAIT_UINT, 1, nullptr, &none), GUEST_ETIMEDOUT)); - Require(FailsWith(Umtx(low, UMTX_OP_WAIT_UINT_PRIVATE, 0x700000001ULL, nullptr, &none), GUEST_ETIMEDOUT)); + Require(Umtx(low, UMTX_OP_WAIT_UINT_PRIVATE, 0x700000001ULL, nullptr, &none) == 0); + Require(Umtx(low, UMTX_OP_WAIT_UINT, 0x700000001ULL, nullptr, &none) == 0); + std::uint32_t all = 0xffffffffu; + Require(Umtx(&all, UMTX_OP_WAIT_UINT, 0xffffffffffffffffULL, nullptr, &none) == 0); + Require(FailsWith(Umtx(&all, UMTX_OP_WAIT_UINT, 0xffffffffULL, nullptr, &none), GUEST_ETIMEDOUT)); } static void TimesOutWhileTheValueMatches() { @@ -205,11 +209,14 @@ static void RejectsBadObjectsAndOperations() { std::uint64_t value = 0; Require(FailsWith(Umtx(&value, 0, 0), GUEST_EOPNOTSUPP)); - Require(FailsWith(Umtx(&value, 1, 0), GUEST_EOPNOTSUPP)); Require(FailsWith(Umtx(&value, UMTX_OP_MAX, 0), GUEST_EINVAL)); Require(FailsWith(Umtx(&value, -1, 0), GUEST_EINVAL)); bool threw = false; + try { Umtx(&value, 1, 0); } + catch (const std::runtime_error&) { threw = true; } + Require(threw); + threw = false; try { Umtx(&value, UMTX_OP_MUTEX_LOCK, 0); } catch (const std::runtime_error&) { threw = true; } Require(threw); diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 432cb918a..c48690f27 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -166,7 +166,7 @@ Throughout the project, every function at every stage either **does exactly what - [sceKernelGetModuleInfo](../../core/libs/prx/libkernel/Module/src/ModuleInfo.cpp) (libkernel) - the 0x160-byte layout (`st_size`, `name[256]`, 4 segments, `segment_count`, `fingerprint[20]`) follows [fpPS4](https://github.com/red-prig/fpPS4/blob/04cefd43e6fddd1ab033e7980cd356d14c964905/sys/sys_types.pas#L319-L337) and [shadPS4](https://github.com/shadps4-emu/shadPS4/blob/0fe263a4760dfbfa973366890061749b4af0de97/src/core/module.h#L15-L31). Only that `st_size` is accepted, as in shadPS4; other sizes return `EINVAL`, a null pointer `EFAULT` and a handle `sceKernelGetModuleList` does not list `ESRCH`. Name and segments are the ones `sceKernelGetModuleInfoFromAddr` reports, on Windows from the same PE sections, so `prot` uses the read/write/execute bits 1/2/4, while fpPS4 and shadPS4 store the ELF `p_flags`. The fingerprint is left zero, as the host image does not carry it - [sceKernelReleaseFlexibleMemory](../../core/libs/prx/libkernel/DirectMemory/Export.cpp) (libkernel) - unmaps the range like sceKernelMunmap, as fpPS4 and prosper do; whether the console keeps the virtual range reserved after releasing its pages is unverified - [Memory protection bits 0x100 and 0x200](../../core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp) (libkernel) - taken as audio coprocessor (ACP) read and write, as in the [KytyPS5](https://github.com/KytyPS5/KytyPS5) and [etaHEN](https://github.com/etaHEN/etaHEN) headers; the coprocessor work runs on the host CPU, so they grant host read and read/write access like the GPU bits -- [_umtx_op](../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - `UMTX_OP_WAIT`, `UMTX_OP_WAKE`, `UMTX_OP_WAIT_UINT`, `UMTX_OP_WAIT_UINT_PRIVATE` and `UMTX_OP_WAKE_PRIVATE` follow FreeBSD 11's [kern_umtx.c](https://github.com/freebsd/freebsd-src/blob/stable/11/sys/kern/kern_umtx.c): a timeout whose size argument is at most 16 is a relative `timespec`, a larger one an `_umtx_time`, and a wake of 0 or fewer wakes one waiter. Waiters share the queue of `sceKernelSyncOnAddressWait`. Operations 0 and 1 fail with `EOPNOTSUPP` and operations above 26 with `EINVAL`, as in FreeBSD 11; the other operations throw, and so does a misaligned object. PPSA12544's `libc.prx` passes `UMTX_OP_WAIT` and `UMTX_OP_WAIT_UINT` a pointer to a 4-byte microsecond timeout with size argument 0, which FreeBSD reads as a `timespec`, and waits for a running call_once with operation 1; its `PS5Util.prx` passes a `timespec`. Whether the console reads a microsecond timeout there, and what operation 1 does on it, is unverified +- [_umtx_op](../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - `UMTX_OP_WAIT`, `UMTX_OP_WAKE`, `UMTX_OP_WAIT_UINT`, `UMTX_OP_WAIT_UINT_PRIVATE` and `UMTX_OP_WAKE_PRIVATE` follow FreeBSD 11's [kern_umtx.c](https://github.com/freebsd/freebsd-src/blob/stable/11/sys/kern/kern_umtx.c): a timeout whose size argument is at most 16 is a relative `timespec`, a larger one an `_umtx_time`, and a wake of 0 or fewer wakes one waiter. Waiters share the queue of `sceKernelSyncOnAddressWait`. Operation 0 fails with `EOPNOTSUPP` and operations above 26 with `EINVAL`, as in FreeBSD 11; `UMTX_OP_WAIT_UINT` with a value that does not fit in 32 bits returns 0 at once, as FreeBSD's zero-extended compare does. Operation 1 throws, since PPSA12544's call_once uses it in a way FreeBSD rejects; the other operations throw, and so does a misaligned object. PPSA12544's `libc.prx` passes `UMTX_OP_WAIT` and `UMTX_OP_WAIT_UINT` a pointer to a 4-byte microsecond timeout with size argument 0, which FreeBSD reads as a `timespec`, and waits for a running call_once with operation 1; its `PS5Util.prx` passes a `timespec`. Whether the console reads a microsecond timeout there, and what operation 1 does on it, is unverified - [sceKernelSyncOnAddressWait](../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - the only known caller passes a null timeout and a name string as the fourth argument; the timeout is assumed to point to microseconds like the other kernel waits, and the name is ignored - [sceKernelSyncOnAddressWait8 / sceKernelSyncOnAddressWait16 / sceKernelSyncOnAddressWait32 / sceKernelSyncOnAddressWait64](../../core/libs/prx/libkernel/SyncOnAddress/src/SyncOnAddress.cpp) (libkernel) - libc's `__std_atomic_wait_direct_8/16/32/64` pass a microsecond timeout or null and ignore the result; the by-value comparand, return values and the 2- and 8-byte alignment of Wait16 and Wait64 are assumed - [scePthreadRwlockDestroy, pthread_rwlock_destroy](../../core/libs/prx/libkernel/Pthread/src/Rwlock.cpp) (libkernel) - a rwlock held for reading or writing returns `EBUSY` and keeps its handle, as [KytyPS5](https://github.com/KytyPS5/KytyPS5) does; FreeBSD's [libthr](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_rwlock.c) and shadPS4 free it without checking, and the console's answer is unverified. Destroying a rwlock while another thread is acquiring it is not guarded