From 3dd4ca73494a09006f83d361d5d5b9c6d1b22665 Mon Sep 17 00:00:00 2001 From: Anthony Date: Thu, 25 Feb 2021 21:08:15 +1300 Subject: [PATCH] Implement IsEmuHandle by keeping a lookup of EmuHandle objects Fixes a crash in Amped on loading a map after having exited one Amped requests an invalid handle 0xFDFDFDFD IsEmuHandle return a false positive and we'd crash trying to call NtClose() --- src/core/kernel/exports/EmuKrnlNt.cpp | 6 +-- src/core/kernel/support/EmuFile.cpp | 60 +++++++++++++++++++-------- src/core/kernel/support/EmuFile.h | 16 ++++++- 3 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/core/kernel/exports/EmuKrnlNt.cpp b/src/core/kernel/exports/EmuKrnlNt.cpp index 9a04a8995..6b70378ec 100644 --- a/src/core/kernel/exports/EmuKrnlNt.cpp +++ b/src/core/kernel/exports/EmuKrnlNt.cpp @@ -142,7 +142,7 @@ XBSYSAPI EXPORTNUM(187) xbox::ntstatus_xt NTAPI xbox::NtClose NTSTATUS ret = xbox::status_success; - if (IsEmuHandle(Handle)) + if (EmuHandle::IsEmuHandle(Handle)) { // delete 'special' handles EmuHandle *iEmuHandle = HandleToEmuHandle(Handle); @@ -682,7 +682,7 @@ XBSYSAPI EXPORTNUM(197) xbox::ntstatus_xt NTAPI xbox::NtDuplicateObject NTSTATUS ret = xbox::status_success; - if (IsEmuHandle(SourceHandle)) { + if (EmuHandle::IsEmuHandle(SourceHandle)) { EmuHandle* iEmuHandle = HandleToEmuHandle(SourceHandle); ret = iEmuHandle->NtDuplicateObject(TargetHandle, Options); /* @@ -739,7 +739,7 @@ XBSYSAPI EXPORTNUM(198) xbox::ntstatus_xt NTAPI xbox::NtFlushBuffersFile LOG_FUNC_END; NTSTATUS ret = xbox::status_success; - if (IsEmuHandle(FileHandle)) + if (EmuHandle::IsEmuHandle(FileHandle)) LOG_UNIMPLEMENTED(); else ret = NtDll::NtFlushBuffersFile(FileHandle, (NtDll::IO_STATUS_BLOCK*)IoStatusBlock); diff --git a/src/core/kernel/support/EmuFile.cpp b/src/core/kernel/support/EmuFile.cpp index c5c6113e0..83b3d6f8b 100644 --- a/src/core/kernel/support/EmuFile.cpp +++ b/src/core/kernel/support/EmuFile.cpp @@ -255,9 +255,50 @@ EmuHandle::EmuHandle(EmuNtObject* ntObject) NtObject = ntObject; } +std::unordered_map EmuHandle::EmuHandleLookup = {}; +std::mutex EmuHandle::EmuHandleLookupLock = {}; + +EmuHandle* EmuHandle::CreateEmuHandle(EmuNtObject* ntObject) { + auto emuHandle = new EmuHandle(ntObject); + + // Register EmuHandle + EmuHandleLookupLock.lock(); + EmuHandleLookup.emplace(EmuHandleToHandle(emuHandle), emuHandle); + EmuHandleLookupLock.unlock(); + + return emuHandle; +} + NTSTATUS EmuHandle::NtClose() { - return NtObject->NtClose(); + auto status = NtObject->NtClose(); + + // Unregister the handle + if (status == STATUS_SUCCESS) { + EmuHandleLookupLock.lock(); + EmuHandleLookup.erase(EmuHandleToHandle(this)); + EmuHandleLookupLock.unlock(); + } + + return status; +} + +bool EmuHandle::IsEmuHandle(HANDLE Handle) +{ + auto iter = EmuHandleLookup.find(Handle); + return !(iter == EmuHandleLookup.end()); +} + +EmuHandle* HandleToEmuHandle(HANDLE Handle) +{ + return (EmuHandle*)((uint32_t)Handle & 0x7FFFFFFF); +} + +HANDLE EmuHandleToHandle(EmuHandle* emuHandle) +{ + // Set the high bit + // Avoids collisions with real Xbox handles..? + return (HANDLE)((uint32_t)emuHandle | 0x80000000); } NTSTATUS EmuHandle::NtDuplicateObject(PHANDLE TargetHandle, DWORD Options) @@ -274,7 +315,7 @@ EmuNtObject::EmuNtObject() HANDLE EmuNtObject::NewHandle() { RefCount++; - return EmuHandleToHandle(new EmuHandle(this)); + return EmuHandle::CreateEmuHandle(this); } NTSTATUS EmuNtObject::NtClose() @@ -292,21 +333,6 @@ EmuNtObject* EmuNtObject::NtDuplicateObject(DWORD Options) return this; } -bool IsEmuHandle(HANDLE Handle) -{ - return ((uint32_t)Handle > 0x80000000) && ((uint32_t)Handle < 0xFFFFFFFE); -} - -EmuHandle* HandleToEmuHandle(HANDLE Handle) -{ - return (EmuHandle*)((uint32_t)Handle & 0x7FFFFFFF); -} - -HANDLE EmuHandleToHandle(EmuHandle* emuHandle) -{ - return (HANDLE)((uint32_t)emuHandle | 0x80000000); -} - std::wstring string_to_wstring(std::string const & src) { std::wstring result = std::wstring(src.length(), L' '); diff --git a/src/core/kernel/support/EmuFile.h b/src/core/kernel/support/EmuFile.h index 8690f6970..c8be90781 100644 --- a/src/core/kernel/support/EmuFile.h +++ b/src/core/kernel/support/EmuFile.h @@ -30,6 +30,8 @@ #include #include #include +#include +#include // ****************************************************************** // * prevent name collisions @@ -160,10 +162,21 @@ NTSTATUS CxbxConvertFilePath(std::string RelativeXboxPath, OUT std::wstring &Rel class EmuHandle { public: - EmuHandle(EmuNtObject* ntObject); NTSTATUS NtClose(); NTSTATUS NtDuplicateObject(PHANDLE TargetHandle, DWORD Options); EmuNtObject* NtObject; + + static EmuHandle* CreateEmuHandle(EmuNtObject* ntObject); + static bool IsEmuHandle(HANDLE Handle); +private: + EmuHandle(EmuNtObject* ntObject); + // Keep track of every EmuHandle wrapper + // If we remember what EmuHandle objects we hand out, we can tell the difference between an EmuHandle and garbage + // We used to rely on the high bit to differentiatean EmuHandles + // But titles may attempt to operate on invalid handles with the high bit set + // Test case: Amped sets a handle value to 0xFDFDFDFD (coincidentally a VS debugger guard value) + static std::unordered_map EmuHandleLookup; + static std::mutex EmuHandleLookupLock; }; // ****************************************************************** @@ -207,7 +220,6 @@ struct XboxDevice { // ****************************************************************** // * is Handle a 'special' emulated handle? // ****************************************************************** -bool IsEmuHandle(HANDLE Handle); EmuHandle* HandleToEmuHandle(HANDLE Handle); HANDLE EmuHandleToHandle(EmuHandle* emuHandle);