Merge pull request #5597 from JJenkx/fix/thread-stack-retirement-order

fix(libkernel): retire stack metadata before releasing mappings

core/libs/prx/libkernel/Pthread/src/Thread.cpp

@@ -125,8 +125,8 @@ static void AllocateGuestStack(PthreadPrivate* thread, std::size_t usableBytes)
 static void FreeGuestStack(PthreadPrivate* thread) {
     if (thread->guestStack == nullptr) return;
     void* const mapping = thread->guestStack;
-    DoMunmap(thread->guestStack, thread->guestStackBytes);
     UnregisterGuestStackMapping(mapping);
+    DoMunmap(thread->guestStack, thread->guestStackBytes);
     thread->guestStack = nullptr;
     thread->guestStackBytes = 0;
 }
@@ -152,8 +152,8 @@ static void ReapRetiredStacks() {
         }
     }
     for (const auto& stack : ready) {
-        DoMunmap(stack.address, stack.bytes);
         UnregisterGuestStackMapping(stack.address);
+        DoMunmap(stack.address, stack.bytes);
     }
 }
 

core/libs/tests/GuestPthreadStack.cpp

@@ -1,4 +1,14 @@
 #include "SceTypes.hpp"
+#ifdef __linux__
+#include "prx/libc/include/GuestAllocations.hpp"
+#include <atomic>
+#include <cstdio>
+#include <chrono>
+#include <thread>
+#include <string>
+#include <sys/syscall.h>
+#include <unistd.h>
+#endif
 #include <cstddef>
 #include <cstdint>
 #include <cstdlib>
@@ -12,6 +22,9 @@ int APS5_VABI scePthreadAttrGet(Pthread thread, PthreadAttr* attr);
 int APS5_VABI scePthreadAttrSetstacksize(PthreadAttr* attr, std::size_t stackSize);
 int APS5_VABI scePthreadAttrGetstack(const PthreadAttr* attr, void** stackAddress, std::size_t* stackSize);
 Pthread APS5_VABI scePthreadSelf();
+#ifdef __linux__
+int APS5_VABI scePthreadDetach(Pthread thread);
+#endif
 }
 
 static constexpr int SCE_OK = 0;
@@ -64,7 +77,41 @@ static void* APS5_VABI Worker(void* arg) {
     return arg;
 }
 
+#ifdef __linux__
+bool GuestThreadStackHostRangeOverlaps(std::uintptr_t start, std::uintptr_t end);
+static std::atomic<unsigned> observedUnmaps{0};
+static std::atomic<std::uintptr_t> retiredStackWord{0};
+static std::atomic<bool> retiredUnmapped{false};
+static std::atomic<bool> detachedDone{false};
+static int detachedTid = 0;
+
+static void* APS5_VABI DetachedWorker(void*) {
+    int local = 0;
+    detachedTid = static_cast<int>(syscall(SYS_gettid));
+    retiredStackWord.store(reinterpret_cast<std::uintptr_t>(&local), std::memory_order_relaxed);
+    detachedDone.store(true, std::memory_order_release);
+    return nullptr;
+}
+
+static void* APS5_VABI ReaperWorker(void*) {
+    return nullptr;
+}
+
+static void ObserveUnmap(std::uintptr_t address, std::size_t bytes) {
+    if (GuestThreadStackHostRangeOverlaps(address, address + bytes)) {
+        std::fprintf(stderr, "Retired stack still owns host-only pages after unmap\n");
+        std::abort();
+    }
+    observedUnmaps.fetch_add(1, std::memory_order_relaxed);
+    const auto retired = retiredStackWord.load(std::memory_order_relaxed);
+    if (retired >= address && retired - address < bytes) retiredUnmapped.store(true, std::memory_order_relaxed);
+}
+#endif
+
 int main() {
+#ifdef __linux__
+    GuestAllocations::GuestAllocationsSetUnmapObserver_nid_postfix(&ObserveUnmap);
+#endif
     for (auto stackSize : STACK_SIZES) {
         PthreadAttr attr = nullptr;
         Require(scePthreadAttrInit(&attr) == SCE_OK);
@@ -76,4 +123,25 @@ int main() {
         Require(scePthreadJoin(thread, &result) == SCE_OK);
         Require(result == &stackSize);
     }
+#ifdef __linux__
+    Pthread detached = nullptr;
+    Require(scePthreadCreate(&detached, nullptr, DetachedWorker, nullptr, nullptr) == SCE_OK);
+    Require(scePthreadDetach(detached) == SCE_OK);
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2);
+    while (!detachedDone.load(std::memory_order_acquire)) {
+        Require(std::chrono::steady_clock::now() < deadline);
+        std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    }
+    const auto task = "/proc/self/task/" + std::to_string(detachedTid);
+    while (access(task.c_str(), F_OK) == 0) {
+        Require(std::chrono::steady_clock::now() < deadline);
+        std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    }
+    Pthread reaper = nullptr;
+    Require(scePthreadCreate(&reaper, nullptr, ReaperWorker, nullptr, nullptr) == SCE_OK);
+    Require(scePthreadJoin(reaper, nullptr) == SCE_OK);
+    Require(retiredUnmapped.load(std::memory_order_relaxed));
+    GuestAllocations::GuestAllocationsSetUnmapObserver_nid_postfix(nullptr);
+    Require(observedUnmaps.load(std::memory_order_relaxed) != 0);
+#endif
 }