fix(libkernel): skip the host-only pages around a guest thread stack in mprotect on Linux

core/libs/prx/libkernel/DirectMemory/DirectMemory.cpp

@@ -4,6 +4,7 @@
 #include "prx/libc/include/GuestArena.hpp"
 #include "prx/libc/include/GuestWriteWatch.hpp"
 #include <algorithm>
+#include <array>
 #include <cerrno>
 #include <iterator>
 #include <map>
@@ -1481,29 +1482,33 @@ static int FindImagesOverlapping(dl_phdr_info* image, std::size_t, void* data) {
 }
 #endif
 
-int DoMprotect(const void* addr, size_t len, int prot, bool requireMappedStart) {
-    Trace("protect %p+0x%zx prot=0x%x", addr, len, prot);
-    const auto address = reinterpret_cast<std::uintptr_t>(addr);
+static bool OnGuestThreadStack(std::uintptr_t address, std::size_t len, int prot, std::uintptr_t* stackStart, std::uintptr_t* stackEnd) {
+    return (prot & GuestProtGpuReadWrite) == 0 && GuestThreadStack(address, stackStart, stackEnd) && address >= *stackStart && address + len <= *stackEnd;
+}
+
+static bool HostStackPage(std::uintptr_t page) {
+    return !GuestThreadStackHostRanges(page, page + PS5_PAGE_SIZE).empty();
+}
+
+static void GuestPageBounds(std::uintptr_t address, std::size_t len, std::uintptr_t* first, std::uintptr_t* end) {
     constexpr auto pageMask = static_cast<std::uintptr_t>(PS5_PAGE_SIZE - 1);
-    const auto limit = std::numeric_limits<std::uintptr_t>::max();
-    if (address == 0 || len == 0 || len > limit - address || address + len > limit - pageMask) throw std::invalid_argument("Invalid guest memory protection range");
-    auto first = address & ~pageMask;
-    auto end = (address + len + pageMask) & ~pageMask;
+    *first = address & ~pageMask;
+    *end = (address + len + pageMask) & ~pageMask;
 #ifdef __APPLE__
-    GuestAllocations::GuestAllocationsGuestPages_nid_postfix(address, len, PS5_PAGE_SIZE, &first, &end);
+    GuestAllocations::GuestAllocationsGuestPages_nid_postfix(address, len, PS5_PAGE_SIZE, first, end);
 #endif
-    auto bytes = static_cast<std::size_t>(end - first);
-    const void* pointer = reinterpret_cast<const void*>(first);
+}
+
+static void ProtectStackRange(std::uintptr_t address, std::size_t len, int prot, std::uintptr_t stackStart, std::uintptr_t stackEnd) {
+    std::uintptr_t first = 0;
+    std::uintptr_t end = 0;
+    GuestPageBounds(address, len, &first, &end);
     const auto nativeProtection = LinuxProtFromSce(prot);
-    if (GuestThreadStackHostRangeOverlaps(address, address + len))
-        throw std::system_error(EACCES, std::generic_category(), "mprotect overlaps host-only guest thread stack pages");
-    std::uintptr_t stackStart = 0;
-    std::uintptr_t stackEnd = 0;
-    if ((prot & GuestProtGpuReadWrite) == 0 && GuestThreadStack(address, &stackStart, &stackEnd) && address >= stackStart && address + len <= stackEnd) {
+    {
         constexpr std::uintptr_t hostPageMask = 0xfff;
         const auto requested = std::max(first, (stackStart + hostPageMask) & ~hostPageMask);
         const auto insideEnd = std::min(end, stackEnd & ~hostPageMask);
-        if (requested >= insideEnd) return 0;
+        if (requested >= insideEnd) return;
         auto* requestedPointer = reinterpret_cast<void*>(requested);
         const auto requestedBytes = static_cast<std::size_t>(insideEnd - requested);
 #ifdef _WIN32
@@ -1512,7 +1517,7 @@ int DoMprotect(const void* addr, size_t len, int prot, bool requireMappedStart)
         const auto inside = hostStack && !reserveBottom ? AboveStackGuard(requested, insideEnd) : requested;
         if (inside == insideEnd) {
             RecordProtection(requestedPointer, requestedBytes, prot);
-            return 0;
+            return;
         }
 #else
         const auto inside = requested;
@@ -1540,35 +1545,48 @@ int DoMprotect(const void* addr, size_t len, int prot, bool requireMappedStart)
         NoteGuestStackProtection(inside, insideBytes, (prot & 3) == 3);
         GuestAllocations::GuestAllocationsInvalidate_nid_postfix(inside, insideBytes);
         RecordProtection(insidePointer, insideBytes, prot);
-        return 0;
     }
+}
+
+static int ProtectRegisteredRanges(const std::vector<std::pair<std::uintptr_t, std::uintptr_t>>& ranges, int prot, bool requireMappedStart) {
+    const auto nativeProtection = LinuxProtFromSce(prot);
+    if (requireMappedStart && ranges.empty()) return SCE_KERNEL_ERROR_EINVAL;
     GuestAllocations::Mutation mutation;
+    std::vector<std::pair<const void*, std::size_t>> spans;
+    for (std::size_t index = 0; index < ranges.size(); ++index) {
+        const auto [address, rangeEnd] = ranges[index];
+        std::uintptr_t first = 0;
+        std::uintptr_t end = 0;
+        GuestPageBounds(address, rangeEnd - address, &first, &end);
+        auto bytes = static_cast<std::size_t>(end - first);
+        const void* pointer = reinterpret_cast<const void*>(first);
 #ifdef _WIN32
-    for (auto cursor = first; cursor < end;) {
-        MEMORY_BASIC_INFORMATION view{};
-        if (VirtualQuery(reinterpret_cast<const void*>(cursor), &view, sizeof(view)) != sizeof(view)) throw std::runtime_error("Cannot query guest memory protection range");
-        if (view.Type == MEM_IMAGE) {
-            if (view.AllocationBase == GetModuleHandleW(nullptr)) mutation.RegisterMainImage();
-            else if (IsGuestModuleImage(view.AllocationBase)) mutation.RegisterImage(view.AllocationBase);
-            else if (cursor == first) throw std::invalid_argument("Memory protection of a foreign image is not supported");
+        for (auto cursor = first; cursor < end;) {
+            MEMORY_BASIC_INFORMATION view{};
+            if (VirtualQuery(reinterpret_cast<const void*>(cursor), &view, sizeof(view)) != sizeof(view)) throw std::runtime_error("Cannot query guest memory protection range");
+            if (view.Type == MEM_IMAGE) {
+                if (view.AllocationBase == GetModuleHandleW(nullptr)) mutation.RegisterMainImage();
+                else if (IsGuestModuleImage(view.AllocationBase)) mutation.RegisterImage(view.AllocationBase);
+                else if (cursor == first) throw std::invalid_argument("Memory protection of a foreign image is not supported");
+            }
+            if (view.Type == MEM_MAPPED && (nativeProtection & ~PosixRightsFromWin(view.AllocationProtect)) != 0)
+                throw std::system_error(EACCES, std::generic_category(), "mprotect above the maximum protection of a mapped view");
+            cursor = reinterpret_cast<std::uintptr_t>(view.BaseAddress) + view.RegionSize;
         }
-        if (view.Type == MEM_MAPPED && (nativeProtection & ~PosixRightsFromWin(view.AllocationProtect)) != 0)
-            throw std::system_error(EACCES, std::generic_category(), "mprotect above the maximum protection of a mapped view");
-        cursor = reinterpret_cast<std::uintptr_t>(view.BaseAddress) + view.RegionSize;
-    }
 #else
-    mutation.RegisterMainImage();
-    GuestImageSearch search{.start = first, .end = end, .pageSize = static_cast<std::uintptr_t>(::sysconf(_SC_PAGESIZE))};
-    dl_iterate_phdr(FindImagesOverlapping, &search);
-    for (const auto& image : search.images) {
-        if (image.guestModule) mutation.RegisterImage(reinterpret_cast<const void*>(image.base));
-        else if (image.containsStart) throw std::invalid_argument("Memory protection of a foreign image is not supported");
-    }
-    mutation.TrimToRegistered(PS5_PAGE_SIZE, pointer, bytes);
+        mutation.RegisterMainImage();
+        GuestImageSearch search{.start = first, .end = end, .pageSize = static_cast<std::uintptr_t>(::sysconf(_SC_PAGESIZE))};
+        dl_iterate_phdr(FindImagesOverlapping, &search);
+        for (const auto& image : search.images) {
+            if (image.guestModule) mutation.RegisterImage(reinterpret_cast<const void*>(image.base));
+            else if (image.containsStart) throw std::invalid_argument("Memory protection of a foreign image is not supported");
+        }
+        mutation.TrimToRegistered(PS5_PAGE_SIZE, pointer, bytes);
 #endif
-    std::vector<std::pair<const void*, std::size_t>> spans;
-    mutation.ForRegistered(pointer, bytes, [&](const void* span, std::size_t spanBytes) { spans.emplace_back(span, spanBytes); });
-    if (requireMappedStart && (spans.empty() || spans.front().first != pointer)) return SCE_KERNEL_ERROR_EINVAL;
+        const auto before = spans.size();
+        mutation.ForRegistered(pointer, bytes, [&](const void* span, std::size_t spanBytes) { spans.emplace_back(span, spanBytes); });
+        if (requireMappedStart && index == 0 && (spans.size() == before || spans[before].first != pointer)) return SCE_KERNEL_ERROR_EINVAL;
+    }
     for (const auto& [span, spanBytes] : spans) mutation.RequireUnpinned(span, spanBytes);
     for (const auto& [span, spanBytes] : spans) {
         mutation.Protect(span, spanBytes, (prot & 3) != 0, (prot & 2) != 0, (prot & GuestProtGpuReadWrite) != 0, [&] {
@@ -1584,6 +1602,45 @@ int DoMprotect(const void* addr, size_t len, int prot, bool requireMappedStart)
     return 0;
 }
 
+int DoMprotect(const void* addr, size_t len, int prot, bool requireMappedStart) {
+    Trace("protect %p+0x%zx prot=0x%x", addr, len, prot);
+    const auto address = reinterpret_cast<std::uintptr_t>(addr);
+    constexpr auto pageMask = static_cast<std::uintptr_t>(PS5_PAGE_SIZE - 1);
+    const auto limit = std::numeric_limits<std::uintptr_t>::max();
+    if (address == 0 || len == 0 || len > limit - address || address + len > limit - pageMask) throw std::invalid_argument("Invalid guest memory protection range");
+    std::uintptr_t first = 0;
+    std::uintptr_t end = 0;
+    GuestPageBounds(address, len, &first, &end);
+    std::vector<std::pair<std::uintptr_t, std::uintptr_t>> parts;
+    auto cursor = address;
+    for (const auto& [low, high] : GuestThreadStackHostRanges(first, end)) {
+        if (requireMappedStart && low <= address && address < high) return SCE_KERNEL_ERROR_EINVAL;
+        if (cursor < std::min(low, address + len)) parts.emplace_back(cursor, std::min(low, address + len));
+        cursor = std::max(cursor, high);
+    }
+    if (cursor < address + len) parts.emplace_back(cursor, address + len);
+    std::vector<std::pair<std::uintptr_t, std::uintptr_t>> registered;
+    std::vector<std::array<std::uintptr_t, 4>> stacks;
+    for (auto [low, high] : parts) {
+        std::uintptr_t stackStart = 0;
+        std::uintptr_t stackEnd = 0;
+        if (OnGuestThreadStack(low, high - low, prot, &stackStart, &stackEnd)) {
+            stacks.push_back({low, high, stackStart, stackEnd});
+            continue;
+        }
+        if (HostStackPage(low & ~pageMask)) {
+            if (requireMappedStart && low == address) return SCE_KERNEL_ERROR_EINVAL;
+            low = (low + pageMask) & ~pageMask;
+        }
+        if (HostStackPage((high - 1) & ~pageMask)) high = (high - 1) & ~pageMask;
+        if (low < high) registered.emplace_back(low, high);
+    }
+    if (!registered.empty() || stacks.empty())
+        if (const int result = ProtectRegisteredRanges(registered, prot, requireMappedStart && (registered.empty() || registered.front().first == address)); result != 0) return result;
+    for (const auto& [low, high, stackStart, stackEnd] : stacks) ProtectStackRange(low, high - low, prot, stackStart, stackEnd);
+    return 0;
+}
+
 int DoMtypeprotect(const void* addr, size_t len, int type, int prot, bool requireMappedStart) {
     const int result = DoMprotect(addr, len, prot, requireMappedStart);
     if (result != 0) return result;

core/libs/prx/libkernel/Pthread/include/Pthread.hpp

@@ -15,6 +15,8 @@
 #include <mutex>
 #include <string>
 #include <thread>
+#include <utility>
+#include <vector>
 #ifndef _WIN32
 #include <pthread.h>
 #endif
@@ -183,6 +185,7 @@ struct PthreadPrivate {
 };
 
 bool GuestThreadStack(std::uintptr_t address, std::uintptr_t* start, std::uintptr_t* end);
+std::vector<std::pair<std::uintptr_t, std::uintptr_t>> GuestThreadStackHostRanges(std::uintptr_t start, std::uintptr_t end);
 bool GuestThreadStackHostRangeOverlaps(std::uintptr_t start, std::uintptr_t end);
 void NoteGuestStackProtection(std::uintptr_t start, std::size_t bytes, bool readWrite);
 #ifdef _WIN32

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

@@ -532,22 +532,26 @@ bool GuestThreadStack(std::uintptr_t address, std::uintptr_t* start, std::uintpt
     return false;
 }
 
-bool GuestThreadStackHostRangeOverlaps(std::uintptr_t start, std::uintptr_t end) {
+std::vector<std::pair<std::uintptr_t, std::uintptr_t>> GuestThreadStackHostRanges(std::uintptr_t start, std::uintptr_t end) {
+    std::vector<std::pair<std::uintptr_t, std::uintptr_t>> ranges;
 #if defined(__linux__)
     std::lock_guard lock(guestStackMappingsLock);
     auto it = guestStackMappings.upper_bound(start);
     if (it != guestStackMappings.begin()) --it;
     for (; it != guestStackMappings.end() && it->first < end; ++it) {
-        const auto mappingStart = it->first;
         const auto& stack = it->second;
-        if (start < stack.stackStart && end > mappingStart) return true;
-        if (start < stack.mappingEnd && end > stack.stackEnd) return true;
+        for (const auto& [low, high] : {std::pair{it->first, stack.stackStart}, std::pair{stack.stackEnd, stack.mappingEnd}})
+            if (start < high && end > low) ranges.emplace_back(std::max(start, low), std::min(end, high));
     }
 #else
     (void)start;
     (void)end;
 #endif
-    return false;
+    return ranges;
+}
+
+bool GuestThreadStackHostRangeOverlaps(std::uintptr_t start, std::uintptr_t end) {
+    return !GuestThreadStackHostRanges(start, end).empty();
 }
 
 struct CpuClockRegistry {

core/libs/tests/GuestKernelStackProtect.cpp

@@ -8,6 +8,10 @@
 #include <cstdlib>
 #include <cstring>
 #include <initializer_list>
+#include <memory>
+#ifdef __linux__
+#include "prx/libc/include/GuestAllocations.hpp"
+#endif
 #ifdef _WIN32
 #ifndef NOMINMAX
 #define NOMINMAX
@@ -215,6 +219,46 @@ static void CheckPeerStackRemap() {
 }
 #endif
 
+#ifdef __linux__
+struct PinProbe {
+    std::shared_ptr<const GuestAllocations::Range> pin;
+    std::uintptr_t earlier = 0;
+    std::uint64_t earlierBefore = 0;
+    bool earlierChanged = false;
+    unsigned waits = 0;
+};
+
+static PinProbe pinProbe;
+
+static bool ReleaseStackPin(std::uintptr_t, std::size_t) {
+    ++pinProbe.waits;
+    if (QueryPage(pinProbe.earlier).raw != pinProbe.earlierBefore) pinProbe.earlierChanged = true;
+    pinProbe.pin.reset();
+    return true;
+}
+
+static void CheckPinnedLaterPart(std::uintptr_t low) {
+    auto stack = GuestAllocations::GuestAllocationsLookup_nid_postfix(low, false);
+    Require(stack != nullptr && stack->allocationAddress % Page == 0 && stack->allocationAddress > Page);
+    const auto earlier = static_cast<std::uintptr_t>(stack->allocationAddress) - Page;
+    Require(GuestAllocations::GuestAllocationsLookup_nid_postfix(earlier, false) == nullptr);
+    Require(mmap_nid_postfix(reinterpret_cast<void*>(earlier), Page, 3, 0x1012, -1, 0) == reinterpret_cast<void*>(earlier));
+    pinProbe.earlier = earlier;
+    pinProbe.earlierBefore = QueryPage(earlier).raw;
+    pinProbe.pin = std::move(stack);
+    GuestAllocations::GuestAllocationsSetPinWaiter_nid_postfix(&ReleaseStackPin);
+    const int protectedRange = mprotect_nid_postfix(reinterpret_cast<void*>(earlier), low + GuardBytes - earlier, 0x11);
+    GuestAllocations::GuestAllocationsSetPinWaiter_nid_postfix(nullptr);
+    Require(protectedRange == 0);
+    Require(pinProbe.waits != 0);
+    Require(!pinProbe.earlierChanged);
+    Require(!QueryPage(earlier).writable && !QueryPage(low).writable);
+    Require(mprotect_nid_postfix(reinterpret_cast<void*>(low), GuardBytes, 3) == 0);
+    Require(QueryPage(low).writable);
+    Require(munmap_nid_postfix(reinterpret_cast<void*>(earlier), Page) == 0);
+}
+#endif
+
 static void* APS5_VABI Worker(void* arg) {
     auto& passed = *static_cast<std::atomic<bool>*>(arg);
     int local = 0;
@@ -232,7 +276,7 @@ static void* APS5_VABI Worker(void* arg) {
     }
     Require(hostGuard != 0);
     const auto hostGuardBefore = QueryPage(hostGuard).raw;
-    Require(mprotect_nid_postfix(reinterpret_cast<void*>(hostGuard), Page, 3) == -1);
+    Require(mprotect_nid_postfix(reinterpret_cast<void*>(hostGuard), Page, 3) == 0);
     Require(QueryPage(hostGuard).raw == hostGuardBefore);
 #endif
     auto* guard = reinterpret_cast<volatile unsigned char*>(low);
@@ -255,19 +299,27 @@ static void* APS5_VABI Worker(void* arg) {
     static_cast<volatile unsigned char*>(start)[0] = 3;
     const auto outside = reinterpret_cast<std::uintptr_t>(start) & ~(Page - 1);
     if (outside >= Page) {
-        const int straddle = mprotect_nid_postfix(reinterpret_cast<void*>(outside - Page), 2 * Page, 3);
+        Require(mprotect_nid_postfix(reinterpret_cast<void*>(outside - Page), 2 * Page, 3) == 0);
         Require(QueryPage(below).raw == belowBefore);
-#ifndef __linux__
-        Require(straddle == 0);
-#else
-        (void)straddle;
+#ifdef __linux__
+        Require(mprotect_nid_postfix(reinterpret_cast<void*>(outside - Page), 3 * Page, 1) == 0);
+        Require(QueryPage(below).raw == belowBefore);
+        Require(!QueryPage(outside + Page).writable);
+        Require(mprotect_nid_postfix(start, GuardBytes, 3) == 0);
+        Require(QueryPage(outside + Page).writable);
 #endif
+        Require(mprotect_nid_postfix(reinterpret_cast<void*>(outside - Page), 2 * Page, 0x11) == 0);
+        Require(QueryPage(below).raw == belowBefore);
+        Require(mprotect_nid_postfix(start, GuardBytes, 3) == 0);
     }
     const auto stackBytes = reinterpret_cast<std::uintptr_t>(end) - reinterpret_cast<std::uintptr_t>(start);
     Require(mprotect_nid_postfix(start, stackBytes, 0xc2) == 0);
     Require(QueryPage(low).writable);
     static_cast<volatile unsigned char*>(start)[0] = 4;
     Require(mprotect_nid_postfix(start, stackBytes, 3) == 0);
+#ifdef __linux__
+    CheckPinnedLaterPart(low);
+#endif
 #ifdef _WIN32
     CheckFixedStackRemap(low, start);
 #endif