From e97b18e244e4d51a7434b4d18a77bd1da6203e99 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Mon, 5 Jan 2026 01:20:02 +0000 Subject: [PATCH 1/3] InvalidationTracker: Fix potential race when reprotecting RWX ranges Theoretically, another thread could run after the reprotection but before the invalidation, write some code and jump to it but end up jumping to old code as the invalidation is yet to occur. Fix this by locking the invalidation mutex, invalidating and only then reprotecting. --- Source/Windows/Common/InvalidationTracker.cpp | 31 +++++++++++-------- Source/Windows/Common/InvalidationTracker.h | 2 ++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Source/Windows/Common/InvalidationTracker.cpp b/Source/Windows/Common/InvalidationTracker.cpp index 5bd2930e0..77c980177 100644 --- a/Source/Windows/Common/InvalidationTracker.cpp +++ b/Source/Windows/Common/InvalidationTracker.cpp @@ -168,22 +168,22 @@ void InvalidationTracker::ReprotectRWXIntervals(uint64_t Address, uint64_t Size) bool InvalidationTracker::HandleRWXAccessViolation(FEXCore::Core::InternalThreadState* Thread, uint64_t HostPc, uint64_t FaultAddress) { const bool NeedsInvalidate = [&](uint64_t Address) { std::shared_lock Lock(IntervalsLock); - const bool Enclosed = RWXIntervals.Query(Address).Enclosed; - // Invalidate just the single faulting page - if (!Enclosed) { - return false; - } - - ULONG TmpProt; - void* TmpAddress = reinterpret_cast(Address); - SIZE_T TmpSize = 1; - NtProtectVirtualMemory(NtCurrentProcess(), &TmpAddress, &TmpSize, PAGE_EXECUTE_READWRITE, &TmpProt); - return true; + return RWXIntervals.Query(Address).Enclosed; }(FaultAddress); if (NeedsInvalidate) { // IntervalsLock cannot be held during invalidation - InvalidateIntervalInternal(FaultAddress & FEXCore::Utils::FEX_PAGE_MASK, FEXCore::Utils::FEX_PAGE_SIZE); + { + std::scoped_lock Lock(CTX.GetCodeInvalidationMutex()); + + InvalidateIntervalInternalLocked(FaultAddress & FEXCore::Utils::FEX_PAGE_MASK, FEXCore::Utils::FEX_PAGE_SIZE); + + // Invalidate, then unprotect the faulting page with the compilation lock held to ensure that any racing invalidations are not dropped. + ULONG TmpProt; + void* TmpAddress = reinterpret_cast(FaultAddress); + SIZE_T TmpSize = 1; + NtProtectVirtualMemory(NtCurrentProcess(), &TmpAddress, &TmpSize, PAGE_EXECUTE_READWRITE, &TmpProt); + } DetectMonoBackpatcherBlock(Thread, HostPc); return true; } @@ -254,7 +254,12 @@ void InvalidationTracker::DisableSMCDetection() { } void InvalidationTracker::InvalidateIntervalInternal(uint64_t Address, uint64_t Size) { - std::scoped_lock Lock(CTX.GetCodeInvalidationMutex()); + std::scoped_lock CodeLock(CTX.GetCodeInvalidationMutex()); + InvalidateIntervalInternalLocked(Address, Size); +} + +void InvalidationTracker::InvalidateIntervalInternalLocked(uint64_t Address, uint64_t Size) { + // NOTE: This assumes CodeInvalidationMutex is locked by the caller CTX.InvalidateCodeBuffersCodeRange(Address, Size); for (auto Thread : Threads) { CTX.InvalidateThreadCachedCodeRange(Thread.second, Address, Size); diff --git a/Source/Windows/Common/InvalidationTracker.h b/Source/Windows/Common/InvalidationTracker.h index 88c2ee1af..92e49d1a2 100644 --- a/Source/Windows/Common/InvalidationTracker.h +++ b/Source/Windows/Common/InvalidationTracker.h @@ -39,6 +39,8 @@ private: void DetectMonoBackpatcherBlock(FEXCore::Core::InternalThreadState* Thread, uint64_t HostPC); void DisableSMCDetection(); void InvalidateIntervalInternal(uint64_t Address, uint64_t Size); + // NOTE: This assumed CodeInvalidationMutex is locked by the caller + void InvalidateIntervalInternalLocked(uint64_t Address, uint64_t Size); FEXCore::IntervalList XIntervals; From ec13e5d503e212a37c3c641e30cf31abd8285de4 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Mon, 5 Jan 2026 02:28:15 +0000 Subject: [PATCH 2/3] Windows: Use FrontendPtr for extra TLS space on WOW64/ARM64EC We have run out of EmulatorData slots on ARM64EC, so introduce this for less-frequently accessed TLS data for which the additional cost doesn't matter. --- Source/Windows/ARM64EC/Module.cpp | 12 ++++++++++++ Source/Windows/WOW64/Module.cpp | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/Source/Windows/ARM64EC/Module.cpp b/Source/Windows/ARM64EC/Module.cpp index c2c9be484..ab07af93f 100644 --- a/Source/Windows/ARM64EC/Module.cpp +++ b/Source/Windows/ARM64EC/Module.cpp @@ -126,6 +126,10 @@ struct ThreadCPUArea { } }; +struct FrontendThreadData { + bool InLockedRWXRead {}; +}; + namespace { fextl::unique_ptr CTX; fextl::unique_ptr SignalDelegator; @@ -150,6 +154,10 @@ ThreadCPUArea GetCPUArea() { return ThreadCPUArea(NtCurrentTeb()); } +FrontendThreadData* GetFrontendThreadData(FEXCore::Core::InternalThreadState* Thread) { + return static_cast(Thread->FrontendPtr); +} + bool IsEmulatorStackAddress(const ThreadCPUArea CPUArea, uint64_t Address) { return Address <= CPUArea.EmulatorStackBase() && Address >= CPUArea.EmulatorStackLimit(); } @@ -928,6 +936,8 @@ NTSTATUS ThreadInit() { .AMD64_ControlWord = 0x27f}; Exception::LoadStateFromECContext(Thread, CPUArea.ContextAmd64().AMD64_Context); + Thread->FrontendPtr = new FrontendThreadData(); + { auto ThreadTID = GetCurrentThreadId(); Threads.emplace(ThreadTID, Thread); @@ -983,6 +993,8 @@ NTSTATUS ThreadTerm(HANDLE Thread, LONG ExitCode) { } auto ThreadState = CPUArea.ThreadState(); + delete GetFrontendThreadData(ThreadState); + // GDT and LDT are mirrored, only free one. delete[] ThreadState->CurrentFrame->State.segment_arrays[FEXCore::Core::CPUState::SEGMENT_ARRAY_INDEX_GDT]; diff --git a/Source/Windows/WOW64/Module.cpp b/Source/Windows/WOW64/Module.cpp index 23dbf679b..39800fa51 100644 --- a/Source/Windows/WOW64/Module.cpp +++ b/Source/Windows/WOW64/Module.cpp @@ -107,6 +107,10 @@ struct TLS { } }; +struct FrontendThreadData { + bool InLockedRWXRead {}; +}; + class WowSyscallHandler; namespace { @@ -142,6 +146,10 @@ TLS GetTLS() { return TLS {NtCurrentTeb()}; } +FrontendThreadData* GetFrontendThreadData(FEXCore::Core::InternalThreadState* Thread) { + return static_cast(Thread->FrontendPtr); +} + uint64_t GetWowTEB(void* TEB) { static constexpr size_t WowTEBOffsetMemberOffset {0x180c}; return static_cast( @@ -595,6 +603,8 @@ void BTCpuThreadInit() { TLS.ThreadState() = Thread; TLS.ControlWord().fetch_or(ControlBits::WOW_CPU_AREA_DIRTY, std::memory_order::relaxed); + Thread->FrontendPtr = new FrontendThreadData(); + auto ThreadTID = GetCurrentThreadId(); Threads.emplace(ThreadTID, Thread); if (StatAllocHandler) { @@ -641,6 +651,8 @@ void BTCpuThreadTerm(HANDLE Thread, LONG ExitCode) { } auto ThreadState = TLS.ThreadState(); + delete GetFrontendThreadData(ThreadState); + // GDT and LDT are mirrored, only free one. delete[] ThreadState->CurrentFrame->State.segment_arrays[FEXCore::Core::CPUState::SEGMENT_ARRAY_INDEX_GDT]; From 8e9f593c393b05ab26ef08d152a801808a08e571 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Mon, 5 Jan 2026 02:30:36 +0000 Subject: [PATCH 3/3] Windows: Handle NtReadFile calls to RX protected RWX memory Handling this safely requires blocking all compilation for the duration of the read, as fault-based tracking doesn't work when wine's unix side itself is the one performing the write into the RWX region (we cannot catch unix faults). Fixes a startup crash in Persona 5. --- Source/Windows/ARM64EC/Module.cpp | 25 +++++++- Source/Windows/Common/InvalidationTracker.cpp | 63 ++++++++++++------- Source/Windows/Common/InvalidationTracker.h | 8 +++ Source/Windows/WOW64/Module.cpp | 20 +++++- 4 files changed, 93 insertions(+), 23 deletions(-) diff --git a/Source/Windows/ARM64EC/Module.cpp b/Source/Windows/ARM64EC/Module.cpp index ab07af93f..9015a55c7 100644 --- a/Source/Windows/ARM64EC/Module.cpp +++ b/Source/Windows/ARM64EC/Module.cpp @@ -881,7 +881,30 @@ void BTCpu64NotifyMemoryDirty(void* Address, SIZE_T Size) { InvalidationTracker->InvalidateAlignedInterval(reinterpret_cast(Address), static_cast(Size), false); } -void BTCpu64NotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After, NTSTATUS Status) {} +void BTCpu64NotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After, NTSTATUS Status) { + auto* ThreadState = GetCPUArea().ThreadState(); + if (!InvalidationTracker || !ThreadState) { + return; + } + + auto& InLockedRWXRead = GetFrontendThreadData(ThreadState)->InLockedRWXRead; + if (!After) { + ThreadCreationMutex.lock(); + CTX->GetCodeInvalidationMutex().lock(); + if (InvalidationTracker->BeginUntrackedWriteLocked(reinterpret_cast(Address), static_cast(Size))) { + InLockedRWXRead = true; + } else { + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } + } else { + if (InLockedRWXRead) { + InLockedRWXRead = false; + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } + } +} NTSTATUS ThreadInit() { std::scoped_lock Lock(ThreadCreationMutex); diff --git a/Source/Windows/Common/InvalidationTracker.cpp b/Source/Windows/Common/InvalidationTracker.cpp index 77c980177..8f888b761 100644 --- a/Source/Windows/Common/InvalidationTracker.cpp +++ b/Source/Windows/Common/InvalidationTracker.cpp @@ -142,27 +142,7 @@ void InvalidationTracker::InvalidateAlignedInterval(uint64_t Address, uint64_t S } void InvalidationTracker::ReprotectRWXIntervals(uint64_t Address, uint64_t Size) { - const auto End = Address + Size; - std::shared_lock Lock(IntervalsLock); - - if (SMCDetectionDisabled) { - return; - } - - do { - const auto Query = RWXIntervals.Query(Address); - if (Query.Enclosed) { - void* TmpAddress = reinterpret_cast(Address); - SIZE_T TmpSize = static_cast(std::min(End, Address + Query.Size) - Address); - ULONG TmpProt; - NtProtectVirtualMemory(NtCurrentProcess(), &TmpAddress, &TmpSize, PAGE_EXECUTE_READ, &TmpProt); - } else if (!Query.Size) { - // No more regions past `Address` in the interval list - break; - } - - Address += Query.Size; - } while (Address < End); + ProtectRWXIntervalsInternal(Address, Size, false); } bool InvalidationTracker::HandleRWXAccessViolation(FEXCore::Core::InternalThreadState* Thread, uint64_t HostPc, uint64_t FaultAddress) { @@ -190,6 +170,10 @@ bool InvalidationTracker::HandleRWXAccessViolation(FEXCore::Core::InternalThread return false; } +bool InvalidationTracker::BeginUntrackedWriteLocked(uint64_t Address, uint64_t Size) { + return ProtectRWXIntervalsInternal(Address, Size, true); +} + FEXCore::HLE::ExecutableRangeInfo InvalidationTracker::QueryExecutableRange(uint64_t Address) { std::shared_lock Lock(IntervalsLock); const auto XResult = XIntervals.Query(Address); @@ -266,4 +250,41 @@ void InvalidationTracker::InvalidateIntervalInternalLocked(uint64_t Address, uin } } +bool InvalidationTracker::ProtectRWXIntervalsInternal(uint64_t Address, uint64_t Size, bool ForWriteLocked) { + const auto End = Address + Size; + std::shared_lock Lock(IntervalsLock); + + if (SMCDetectionDisabled) { + return false; + } + + bool HitRWXInterval = false; + do { + const auto Query = RWXIntervals.Query(Address); + if (Query.Enclosed) { + if (!HitRWXInterval) { + if (ForWriteLocked) { + // If we are protecting as writable, then the entire range must be invalidated before any protections are + // applied and the invalidation mutex must be locked throughout. + // Do this lazily only when an RWX region is actually hit. + // NOTE: This assumes CodeInvalidationMutex is locked by the caller + InvalidateIntervalInternalLocked(Address, Size); + } + HitRWXInterval = true; + } + void* TmpAddress = reinterpret_cast(Address); + SIZE_T TmpSize = static_cast(std::min(End, Address + Query.Size) - Address); + ULONG TmpProt; + NtProtectVirtualMemory(NtCurrentProcess(), &TmpAddress, &TmpSize, ForWriteLocked ? PAGE_EXECUTE_READWRITE : PAGE_EXECUTE_READ, &TmpProt); + } else if (!Query.Size) { + // No more regions past `Address` in the interval list + break; + } + + Address += Query.Size; + } while (Address < End); + + return HitRWXInterval; +} + } // namespace FEX::Windows diff --git a/Source/Windows/Common/InvalidationTracker.h b/Source/Windows/Common/InvalidationTracker.h index 92e49d1a2..73b08b915 100644 --- a/Source/Windows/Common/InvalidationTracker.h +++ b/Source/Windows/Common/InvalidationTracker.h @@ -33,6 +33,11 @@ public: void InvalidateAlignedInterval(uint64_t Address, uint64_t Size, bool Free); void ReprotectRWXIntervals(uint64_t Address, uint64_t Size); bool HandleRWXAccessViolation(FEXCore::Core::InternalThreadState* Thread, uint64_t HostPC, uint64_t FaultAddress); + + // Unprotects any RWX intervals in the input interval and invalidates code + // NOTE: CodeInvalidationMutex must be locked when calling this, and if true is returned, kept locked until the write ends. + bool BeginUntrackedWriteLocked(uint64_t Address, uint64_t Size); + FEXCore::HLE::ExecutableRangeInfo QueryExecutableRange(uint64_t Address); private: @@ -42,6 +47,9 @@ private: // NOTE: This assumed CodeInvalidationMutex is locked by the caller void InvalidateIntervalInternalLocked(uint64_t Address, uint64_t Size); + // NOTE: If ForWriteLocked is true then this assumes CodeInvalidationMutex is locked by the caller, + // and any code in the range will be invalidated before protection as RWX, otherwise protects as RX if false. + bool ProtectRWXIntervalsInternal(uint64_t Address, uint64_t Size, bool ForWriteLocked); FEXCore::IntervalList XIntervals; FEXCore::IntervalList RWXIntervals; diff --git a/Source/Windows/WOW64/Module.cpp b/Source/Windows/WOW64/Module.cpp index 39800fa51..ea18a93cb 100644 --- a/Source/Windows/WOW64/Module.cpp +++ b/Source/Windows/WOW64/Module.cpp @@ -1004,7 +1004,25 @@ void BTCpuNotifyUnmapViewOfSection(void* Address, BOOL After, ULONG Status) { } } -void BTCpuNotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After, NTSTATUS Status) {} +void BTCpuNotifyReadFile(HANDLE Handle, void* Address, SIZE_T Size, BOOL After, NTSTATUS Status) { + auto& InLockedRWXRead = GetFrontendThreadData(GetTLS().ThreadState())->InLockedRWXRead; + if (!After) { + ThreadCreationMutex.lock(); + CTX->GetCodeInvalidationMutex().lock(); + if (InvalidationTracker->BeginUntrackedWriteLocked(reinterpret_cast(Address), static_cast(Size))) { + InLockedRWXRead = true; + } else { + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } + } else { + if (InLockedRWXRead) { + InLockedRWXRead = false; + CTX->GetCodeInvalidationMutex().unlock(); + ThreadCreationMutex.unlock(); + } + } +} BOOLEAN WINAPI BTCpuIsProcessorFeaturePresent(UINT Feature) { return CPUFeatures->IsFeaturePresent(Feature) ? TRUE : FALSE;