diff --git a/core/libs/prx/libkernel/CMakeLists.txt b/core/libs/prx/libkernel/CMakeLists.txt index 4961439df..32ac770fc 100644 --- a/core/libs/prx/libkernel/CMakeLists.txt +++ b/core/libs/prx/libkernel/CMakeLists.txt @@ -343,6 +343,16 @@ if(BUILD_TESTING) set_tests_properties(guest_cond_recursive_mutex PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() + add_test_executable(guest_cond_unowned_mutex_tests ${LIBS_INCLUDE_DIR}/tests/GuestCondUnownedMutex.cpp) + target_include_directories(guest_cond_unowned_mutex_tests PRIVATE ${LIBS_INCLUDE_DIR}) + target_link_libraries(guest_cond_unowned_mutex_tests PRIVATE libkernel libc) + configure_windows_unwind(guest_cond_unowned_mutex_tests) + add_test(NAME guest_cond_unowned_mutex COMMAND guest_cond_unowned_mutex_tests) + set_tests_properties(guest_cond_unowned_mutex PROPERTIES TIMEOUT 20) + if(WIN32) + set_tests_properties(guest_cond_unowned_mutex PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() add_test_executable(guest_cpu_time_clocks_tests ${LIBS_INCLUDE_DIR}/tests/GuestCpuTimeClocks.cpp) target_include_directories(guest_cpu_time_clocks_tests PRIVATE ${LIBS_INCLUDE_DIR}) target_link_libraries(guest_cpu_time_clocks_tests PRIVATE libkernel libc) diff --git a/core/libs/prx/libkernel/Pthread/src/Cond.cpp b/core/libs/prx/libkernel/Pthread/src/Cond.cpp index 66887e6a7..4cddf4d5b 100644 --- a/core/libs/prx/libkernel/Pthread/src/Cond.cpp +++ b/core/libs/prx/libkernel/Pthread/src/Cond.cpp @@ -3,6 +3,7 @@ #include "prx/libkernel/Pthread/include/Cond.hpp" #include "prx/libkernel/Pthread/include/Cancel.hpp" #include "prx/libkernel/Pthread/Posix/Common.hpp" +#include "prx/libkernel/KernelErrors.hpp" #include "prx/libkernel/Time/include/Time.hpp" #include "prx/libkernel/Time/include/TimedWait.hpp" #include @@ -22,6 +23,14 @@ PthreadCond destroyedCond() { return reinterpret_cast(std::uintptr_t{2}); } +PthreadMutex destroyedMutex() { + return reinterpret_cast(std::uintptr_t{2}); +} + +PthreadMutex adaptiveInitializer() { + return reinterpret_cast(std::uintptr_t{1}); +} + PthreadCond resolveCond(PthreadCond* cond) { if (!cond) throw std::invalid_argument("Condition variable pointer is null"); @@ -39,18 +48,23 @@ PthreadCond resolveCond(PthreadCond* cond) { return created; } -PthreadMutex lockedMutex(PthreadMutex* mutex) { - if (!mutex || !*mutex) +int mutexOwnership(PthreadMutex* mutex, PthreadMutex* owned) { + if (!mutex) throw std::invalid_argument("Mutex pointer is null"); - auto* current = *mutex; - if (current->_owner.load(std::memory_order_acquire) != std::this_thread::get_id()) - throw std::runtime_error("Condition wait mutex is not owned by the current thread"); - return current; + const auto current = std::atomic_ref(*mutex).load(std::memory_order_acquire); + if (current == destroyedMutex()) + return SCE_KERNEL_ERROR_EINVAL; + if (!current || current == adaptiveInitializer() || current->_owner.load(std::memory_order_acquire) != std::this_thread::get_id()) + return SCE_KERNEL_ERROR_EPERM; + *owned = current; + return 0; } int waitUntil(PthreadCond* cond, PthreadMutex* mutex, std::optional deadlineNanos, const void* caller) { auto* c = resolveCond(cond); - auto* m = lockedMutex(mutex); + PthreadMutex m = nullptr; + if (const int error = mutexOwnership(mutex, &m)) + return error; bool timedOut = false; const auto waitStart = std::chrono::steady_clock::now(); struct Trace { diff --git a/core/libs/tests/GuestCondUnownedMutex.cpp b/core/libs/tests/GuestCondUnownedMutex.cpp new file mode 100644 index 000000000..f68fe1739 --- /dev/null +++ b/core/libs/tests/GuestCondUnownedMutex.cpp @@ -0,0 +1,106 @@ +#include "SceTypes.hpp" +#include +#include +#include +#include + +extern "C" { +int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name); +int APS5_VABI scePthreadJoin(Pthread thread, void** retval); +int APS5_VABI scePthreadMutexInit(PthreadMutex* mutex, const PthreadMutexattr* attr, const char* name); +int APS5_VABI scePthreadMutexDestroy(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexLock(PthreadMutex* mutex); +int APS5_VABI scePthreadMutexUnlock(PthreadMutex* mutex); +int APS5_VABI scePthreadCondInit(PthreadCond* cond, const PthreadCondattr* attr, const char* name); +int APS5_VABI scePthreadCondDestroy(PthreadCond* cond); +int APS5_VABI scePthreadCondWait(PthreadCond* cond, PthreadMutex* mutex); +int APS5_VABI scePthreadCondTimedwait(PthreadCond* cond, PthreadMutex* mutex, unsigned int usec); +int APS5_VABI pthread_cond_wait_nid_postfix(PthreadCond* cond, PthreadMutex* mutex); +int APS5_VABI pthread_cond_timedwait_nid_postfix(PthreadCond* cond, PthreadMutex* mutex, const KernelTimespec* abstime); +int APS5_VABI clock_gettime_nid_postfix(int clockId, KernelTimespec* tp); +} + +static constexpr int SCE_OK = 0; +static constexpr int SCE_KERNEL_ERROR_EPERM = static_cast(0x80020001); +static constexpr int SCE_KERNEL_ERROR_EINVAL = static_cast(0x80020016); +static constexpr int POSIX_EPERM = 1; +static constexpr int POSIX_EINVAL = 22; +static constexpr std::uintptr_t ADAPTIVE_INITIALIZER = 1; + +static void Require(bool value) { if (!value) std::abort(); } + +static int PosixTimed(PthreadCond* cond, PthreadMutex* mutex) { + KernelTimespec deadline{}; + Require(clock_gettime_nid_postfix(0, &deadline) == 0); + deadline.tv_sec += 60; + return pthread_cond_timedwait_nid_postfix(cond, mutex, &deadline); +} + +struct Wait { + int (*run)(PthreadCond*, PthreadMutex*); + int notOwned; + int destroyed; +}; + +static int SceWait(PthreadCond* cond, PthreadMutex* mutex) { return scePthreadCondWait(cond, mutex); } +static int SceTimed(PthreadCond* cond, PthreadMutex* mutex) { return scePthreadCondTimedwait(cond, mutex, 60000000); } +static int PosixWait(PthreadCond* cond, PthreadMutex* mutex) { return pthread_cond_wait_nid_postfix(cond, mutex); } + +struct Holder { + PthreadMutex mutex = nullptr; + std::atomic locked{false}; + std::atomic release{false}; +}; + +static void* APS5_VABI Hold(void* arg) { + auto& holder = *static_cast(arg); + Require(scePthreadMutexLock(&holder.mutex) == SCE_OK); + holder.locked.store(true); + while (!holder.release.load()) + std::this_thread::yield(); + Require(scePthreadMutexUnlock(&holder.mutex) == SCE_OK); + return nullptr; +} + +static void Check(const Wait& wait) { + PthreadCond cond = nullptr; + Require(scePthreadCondInit(&cond, nullptr, nullptr) == SCE_OK); + + PthreadMutex unlocked = nullptr; + Require(scePthreadMutexInit(&unlocked, nullptr, nullptr) == SCE_OK); + Require(wait.run(&cond, &unlocked) == wait.notOwned); + Require(scePthreadMutexLock(&unlocked) == SCE_OK); + Require(scePthreadMutexUnlock(&unlocked) == SCE_OK); + Require(scePthreadMutexDestroy(&unlocked) == SCE_OK); + + PthreadMutex uninitialized = nullptr; + Require(wait.run(&cond, &uninitialized) == wait.notOwned); + + PthreadMutex adaptive = reinterpret_cast(ADAPTIVE_INITIALIZER); + Require(wait.run(&cond, &adaptive) == wait.notOwned); + + PthreadMutex destroyed = nullptr; + Require(scePthreadMutexInit(&destroyed, nullptr, nullptr) == SCE_OK); + Require(scePthreadMutexDestroy(&destroyed) == SCE_OK); + Require(wait.run(&cond, &destroyed) == wait.destroyed); + + Holder holder; + Require(scePthreadMutexInit(&holder.mutex, nullptr, nullptr) == SCE_OK); + Pthread thread = nullptr; + Require(scePthreadCreate(&thread, nullptr, Hold, &holder, nullptr) == SCE_OK); + while (!holder.locked.load()) + std::this_thread::yield(); + Require(wait.run(&cond, &holder.mutex) == wait.notOwned); + holder.release.store(true); + Require(scePthreadJoin(thread, nullptr) == SCE_OK); + Require(scePthreadMutexDestroy(&holder.mutex) == SCE_OK); + + Require(scePthreadCondDestroy(&cond) == SCE_OK); +} + +int main() { + Check({SceWait, SCE_KERNEL_ERROR_EPERM, SCE_KERNEL_ERROR_EINVAL}); + Check({SceTimed, SCE_KERNEL_ERROR_EPERM, SCE_KERNEL_ERROR_EINVAL}); + Check({PosixWait, POSIX_EPERM, POSIX_EINVAL}); + Check({PosixTimed, POSIX_EPERM, POSIX_EINVAL}); +} diff --git a/docs/dev/TechnicalDebt.md b/docs/dev/TechnicalDebt.md index 8c103b8c8..21a722572 100644 --- a/docs/dev/TechnicalDebt.md +++ b/docs/dev/TechnicalDebt.md @@ -150,7 +150,7 @@ Throughout the project, every function at every stage either **does exactly what - [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 -- [scePthreadCondWait / scePthreadCondTimedwait](../../core/libs/prx/libkernel/Pthread/src/Cond.cpp) (libkernel) - a recursive mutex locked several times is released completely for the wait and relocked to the same depth afterwards, as FreeBSD libthr does (`_mutex_cv_unlock`, `_mutex_cv_lock`); unverified on hardware +- [scePthreadCondWait / scePthreadCondTimedwait](../../core/libs/prx/libkernel/Pthread/src/Cond.cpp) (libkernel) - before waiting, FreeBSD libthr's [`cond_wait_common`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_cond.c#L1992-L2029) checks ownership through [`_mutex_owned`](https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_mutex.c#L1166-L1178): an unowned mutex returns `EPERM`, while its destroyed marker returns `EINVAL`. An untouched adaptive static initializer (slot value 1) therefore returns `EPERM`; the test covers this, an uninitialized slot, a mutex owned by another thread, and the destroyed marker for both timed and untimed sce and POSIX waits. A recursive mutex locked several times is released completely for the wait and relocked to the same depth afterwards, as FreeBSD libthr does (`_mutex_cv_unlock`, `_mutex_cv_lock`). These FreeBSD-derived behaviors remain unverified on console hardware - [sceKernelAioSubmitReadCommandsMultiple / sceKernelAioSubmitWriteCommandsMultiple / sceKernelAioWaitRequests / sceKernelAioPollRequests / sceKernelAioCancelRequest / sceKernelAioCancelRequests / sceKernelAioDeleteRequests](../../core/libs/prx/libkernel/Aio/src/Aio.cpp) (libkernel) - behaviour from shadPS4 (one id per request for the Multiple submits, cancel turns any request aborted and id 0 reports processing, wait mode 2 returns once one request completed, poll copies every state without waiting); the 128 requests or ids per batch limit is from SharpProspero, with an unknown error, so larger batches throw; wait modes other than 1 (and) and 2 (or) throw; null pointers, negative counts and invalid ids return the codes the single-request siblings use, checked before any state is written - [sceKernelGetAvailableCpumask](../../core/libs/prx/libkernel/Pthread/src/Thread.cpp) (libkernel) - returns the default thread affinity, 0x1FFF (CPUs 0 to 12), as KytyPS5 does. Titles pin threads within it (PPSA02664 passes 0x1FFB, Hades II PPSA36082 0x3 and 0x3F); whether CPU 13 is available to a title is not confirmed - [sceKernelGetCurrentCpu](../../core/libs/prx/libkernel/System/src/Process.cpp) (libkernel) - guest affinities are not applied to host threads, so the host CPU index is mapped into the calling thread's affinity clipped to `sceKernelGetAvailableCpumask` (the available mask when they do not overlap): host CPU n reports the (n modulo the CPU count)-th CPU of that mask, so a thread pinned to one CPU reports that CPU, as a FreeBSD `cpuset` guarantees on the console. Which CPU the console reports for a thread allowed on several is not known; shadPS4 returns 0