From 1ac3d3c5a60ec2465c2e711d72391eec2aa43005 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Tue, 12 Aug 2025 00:43:52 +0100 Subject: [PATCH] Windows: Extend user handle access masks in callbacks where necessary While most applications will just create ALL_ACCESS handles which support the additional operations FEX needs over the regular windows syscall (usually just QUERY_INFORMATION to lookup the thread object), it is valid for them to pass in the minimal set required for each operation and windows/wine will reject any other operations (e.g. querying the TEB base on a handle with only the SUSPEND right). Windows permits duplicating handles to extend their permissions given the process itself has such permissions, so do that as necessary for the operations FEX needs. --- Source/Windows/ARM64EC/Module.cpp | 10 +++--- Source/Windows/Common/Handle.h | 47 +++++++++++++++++++++++----- Source/Windows/WOW64/Module.cpp | 51 +++++++++++++++++++------------ Source/Windows/include/winternl.h | 1 + 4 files changed, 79 insertions(+), 30 deletions(-) diff --git a/Source/Windows/ARM64EC/Module.cpp b/Source/Windows/ARM64EC/Module.cpp index 800ec3ee8..2803110d6 100644 --- a/Source/Windows/ARM64EC/Module.cpp +++ b/Source/Windows/ARM64EC/Module.cpp @@ -964,8 +964,10 @@ NTSTATUS ThreadTerm(HANDLE Thread, LONG ExitCode) { return STATUS_ACCESS_DENIED; } + auto ThreadDup = FEX::Windows::DupHandle(Thread, THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME); + THREAD_BASIC_INFORMATION Info; - if (auto Err = NtQueryInformationThread(Thread, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { + if (auto Err = NtQueryInformationThread(*ThreadDup, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { return Err; } @@ -974,12 +976,12 @@ NTSTATUS ThreadTerm(HANDLE Thread, LONG ExitCode) { if (!Self) { CONTEXT TmpContext; // If we are suspending a thread that isn't ourselves, try to suspend it first so we know internal JIT locks aren't being held. - NtSuspendThread(Thread, NULL); + NtSuspendThread(*ThreadDup, NULL); // This will wait for the thread to be suspended - NtGetContextThread(Thread, &TmpContext); + NtGetContextThread(*ThreadDup, &TmpContext); } - const auto [Err, CPUArea] = GetThreadCPUArea(Thread); + const auto [Err, CPUArea] = GetThreadCPUArea(*ThreadDup); if (Err) { return Err; } diff --git a/Source/Windows/Common/Handle.h b/Source/Windows/Common/Handle.h index 20a2c2d57..1f856ad3e 100644 --- a/Source/Windows/Common/Handle.h +++ b/Source/Windows/Common/Handle.h @@ -4,13 +4,46 @@ #include namespace FEX::Windows { - bool ValidateHandleAccess(HANDLE Handle, ACCESS_MASK Access) { - OBJECT_BASIC_INFORMATION Info; +class ScopedHandle final { +public: + explicit ScopedHandle(HANDLE Handle) + : Handle(Handle) {} - if (NtQueryObject(Handle, ObjectBasicInformation, &Info, sizeof(Info), nullptr)) { - return false; - } - - return (Info.GrantedAccess & Access) == Access; + // Move-only type + ScopedHandle(const ScopedHandle&) = delete; + ScopedHandle& operator=(ScopedHandle&) = delete; + ScopedHandle(ScopedHandle&& rhs) + : Handle(rhs.Handle) { + rhs.Handle = INVALID_HANDLE_VALUE; } + + ~ScopedHandle() { + if (Handle != INVALID_HANDLE_VALUE) { + NtClose(Handle); + } + } + + HANDLE operator*() const { + return Handle; + } +private: + HANDLE Handle; +}; + + +bool ValidateHandleAccess(HANDLE Handle, ACCESS_MASK Access) { + OBJECT_BASIC_INFORMATION Info; + + if (NtQueryObject(Handle, ObjectBasicInformation, &Info, sizeof(Info), nullptr)) { + return false; + } + + return (Info.GrantedAccess & Access) == Access; } + +ScopedHandle DupHandle(HANDLE Handle, ACCESS_MASK Access) { + HANDLE Duplicated = INVALID_HANDLE_VALUE; + NtDuplicateObject(NtCurrentProcess(), Handle, NtCurrentProcess(), &Duplicated, Access, 0, 0); + return ScopedHandle {Duplicated}; +} +} // namespace FEX::Windows diff --git a/Source/Windows/WOW64/Module.cpp b/Source/Windows/WOW64/Module.cpp index 35870a1e3..7bb731958 100644 --- a/Source/Windows/WOW64/Module.cpp +++ b/Source/Windows/WOW64/Module.cpp @@ -581,8 +581,10 @@ void BTCpuThreadTerm(HANDLE Thread, LONG ExitCode) { return; } + auto ThreadDup = FEX::Windows::DupHandle(Thread, THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME); + THREAD_BASIC_INFORMATION Info; - if (auto Err = NtQueryInformationThread(Thread, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { + if (auto Err = NtQueryInformationThread(*ThreadDup, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { return; } @@ -590,10 +592,10 @@ void BTCpuThreadTerm(HANDLE Thread, LONG ExitCode) { bool Self = ThreadTID == GetCurrentThreadId(); if (!Self) { // If we are suspending a thread that isn't ourselves, try to suspend it first so we know internal JIT locks aren't being held. - RtlWow64SuspendThread(Thread, NULL); + RtlWow64SuspendThread(*ThreadDup, NULL); } - auto [Err, TLS] = GetThreadTLS(Thread); + auto [Err, TLS] = GetThreadTLS(*ThreadDup); if (Err) { return; } @@ -628,44 +630,53 @@ void* __wine_get_unix_opcode() { } NTSTATUS BTCpuGetContext(HANDLE Thread, HANDLE Process, void* Unknown, WOW64_CONTEXT* Context) { - auto [Err, TLS] = GetThreadTLS(Thread); + if (!FEX::Windows::ValidateHandleAccess(Thread, THREAD_GET_CONTEXT)) { + return STATUS_ACCESS_DENIED; + } + + auto ThreadDup = FEX::Windows::DupHandle(Thread, THREAD_QUERY_INFORMATION | THREAD_GET_CONTEXT | THREAD_SET_CONTEXT); + auto [Err, TLS] = GetThreadTLS(*ThreadDup); if (Err) { return Err; } if (!(TLS.ControlWord().load(std::memory_order::relaxed) & ControlBits::WOW_CPU_AREA_DIRTY)) { - if (Err = Context::FlushThreadStateContext(Thread); Err) { + if (Err = Context::FlushThreadStateContext(*ThreadDup); Err) { return Err; } } - return RtlWow64GetThreadContext(Thread, Context); + return RtlWow64GetThreadContext(*ThreadDup, Context); } NTSTATUS BTCpuSetContext(HANDLE Thread, HANDLE Process, void* Unknown, WOW64_CONTEXT* Context) { - auto [Err, TLS] = GetThreadTLS(Thread); + if (!FEX::Windows::ValidateHandleAccess(Thread, THREAD_SET_CONTEXT)) { + return STATUS_ACCESS_DENIED; + } + + auto ThreadDup = FEX::Windows::DupHandle(Thread, THREAD_QUERY_INFORMATION | THREAD_GET_CONTEXT | THREAD_SET_CONTEXT); + auto [Err, TLS] = GetThreadTLS(*ThreadDup); if (Err) { return Err; } - // Back-up the input context incase we've been passed the CPU area (the flush below would wipe it out otherwise) WOW64_CONTEXT TmpContext = *Context; if (!(TLS.ControlWord().load(std::memory_order::relaxed) & ControlBits::WOW_CPU_AREA_DIRTY)) { - if (Err = Context::FlushThreadStateContext(Thread); Err) { + if (Err = Context::FlushThreadStateContext(*ThreadDup); Err) { return Err; } } // Merge the input context into the CPU area then pass the full context into the JIT - if (Err = RtlWow64SetThreadContext(Thread, &TmpContext); Err) { + if (Err = RtlWow64SetThreadContext(*ThreadDup, &TmpContext); Err) { return Err; } TmpContext.ContextFlags = WOW64_CONTEXT_FULL | WOW64_CONTEXT_EXTENDED_REGISTERS; - if (Err = RtlWow64GetThreadContext(Thread, &TmpContext); Err) { + if (Err = RtlWow64GetThreadContext(*ThreadDup, &TmpContext); Err) { return Err; } @@ -712,8 +723,9 @@ NTSTATUS BTCpuSuspendLocalThread(HANDLE Thread, ULONG* Count) { return STATUS_ACCESS_DENIED; } + auto ThreadDup = FEX::Windows::DupHandle(Thread, THREAD_QUERY_INFORMATION | THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_SET_CONTEXT); THREAD_BASIC_INFORMATION Info; - if (NTSTATUS Err = NtQueryInformationThread(Thread, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { + if (NTSTATUS Err = NtQueryInformationThread(*ThreadDup, ThreadBasicInformation, &Info, sizeof(Info), nullptr); Err) { return Err; } @@ -723,17 +735,17 @@ NTSTATUS BTCpuSuspendLocalThread(HANDLE Thread, ULONG* Count) { // Mark the CPU area as dirty, to force the JIT context to be restored from it on entry as it may be changed using // SetThreadContext (which doesn't use the BTCpu API) if (!(GetTLS().ControlWord().fetch_or(ControlBits::WOW_CPU_AREA_DIRTY, std::memory_order::relaxed) & ControlBits::WOW_CPU_AREA_DIRTY)) { - if (NTSTATUS Err = Context::FlushThreadStateContext(Thread); Err) { + if (NTSTATUS Err = Context::FlushThreadStateContext(*ThreadDup); Err) { return Err; } } - return NtSuspendThread(Thread, Count); + return NtSuspendThread(*ThreadDup, Count); } LogMan::Msg::DFmt("Suspending thread: {:X}", ThreadTID); - auto [Err, TLS] = GetThreadTLS(Thread); + auto [Err, TLS] = GetThreadTLS(*ThreadDup); if (Err) { return Err; } @@ -742,7 +754,8 @@ NTSTATUS BTCpuSuspendLocalThread(HANDLE Thread, ULONG* Count) { // If the thread hasn't yet been initialized, suspend it without special handling as it wont yet have entered the JIT if (!Threads.contains(ThreadTID)) { - return NtSuspendThread(Thread, Count); + LogMan::Msg::DFmt("Thread suspended: {:X}", ThreadTID); + return NtSuspendThread(*ThreadDup, Count); } // If CONTROL_IN_JIT is unset at this point, then it can never be set (and thus the JIT cannot be reentered) as @@ -762,7 +775,7 @@ NTSTATUS BTCpuSuspendLocalThread(HANDLE Thread, ULONG* Count) { ; // The JIT has now been interrupted and the context stored in the thread's CPU area is up-to-date - if (Err = NtSuspendThread(Thread, Count); Err) { + if (Err = NtSuspendThread(*ThreadDup, Count); Err) { TLS.ControlWord().fetch_and(~ControlBits::PAUSED, std::memory_order::relaxed); return Err; } @@ -773,12 +786,12 @@ NTSTATUS BTCpuSuspendLocalThread(HANDLE Thread, ULONG* Count) { // NtSuspendThread may return before the thread is actually suspended, so a sync operation like NtGetContextThread // needs to be called to ensure it is before we unset CONTROL_PAUSED - std::ignore = NtGetContextThread(Thread, &TmpContext); + std::ignore = NtGetContextThread(*ThreadDup, &TmpContext); // Mark the CPU area as dirty, to force the JIT context to be restored from it on entry as it may be changed using // SetThreadContext (which doesn't use the BTCpu API) if (!(TLS.ControlWord().fetch_or(ControlBits::WOW_CPU_AREA_DIRTY, std::memory_order::relaxed) & ControlBits::WOW_CPU_AREA_DIRTY)) { - if (Err = Context::FlushThreadStateContext(Thread); Err) { + if (Err = Context::FlushThreadStateContext(*ThreadDup); Err) { return Err; } } diff --git a/Source/Windows/include/winternl.h b/Source/Windows/include/winternl.h index 8bf0cd48c..10aaf91fc 100644 --- a/Source/Windows/include/winternl.h +++ b/Source/Windows/include/winternl.h @@ -490,6 +490,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx(HANDLE, PVOID*, SIZE_T*, ULONG, ULONG, NTSTATUS WINAPI NtAllocateVirtualMemory(HANDLE, PVOID*, ULONG_PTR, SIZE_T*, ULONG, ULONG); NTSTATUS WINAPI NtContinue(PCONTEXT, BOOLEAN); NTSTATUS WINAPI NtCreateSection(HANDLE*, ACCESS_MASK, const OBJECT_ATTRIBUTES*, const LARGE_INTEGER*, ULONG, ULONG, HANDLE); +NTSTATUS WINAPI NtDuplicateObject(HANDLE, HANDLE, HANDLE, PHANDLE, ACCESS_MASK, ULONG, ULONG); NTSTATUS WINAPI NtFlushInstructionCache(HANDLE, LPCVOID, SIZE_T); NTSTATUS WINAPI NtFreeVirtualMemory(HANDLE, PVOID*, SIZE_T*, ULONG); NTSTATUS WINAPI NtGetContextThread(HANDLE, CONTEXT*);