diff --git a/x64_dbg_dbg/addrinfo.cpp b/x64_dbg_dbg/addrinfo.cpp index 2475333c..30198904 100644 --- a/x64_dbg_dbg/addrinfo.cpp +++ b/x64_dbg_dbg/addrinfo.cpp @@ -68,7 +68,7 @@ void dbload() return; } FILE* jsonFile = 0; - if(_wfopen_s(&jsonFile, wdbpath.c_str(), L"rb")) + if(_wfopen_s(&jsonFile, wdbpath.c_str(), L"rb") != 0) { dputs("\nfailed to open database file!"); return; diff --git a/x64_dbg_dbg/module.cpp b/x64_dbg_dbg/module.cpp index 73593658..bb7eeac4 100644 --- a/x64_dbg_dbg/module.cpp +++ b/x64_dbg_dbg/module.cpp @@ -33,8 +33,8 @@ bool ModLoad(uint Base, uint Size, const char* FullPath) if(extensionPos) { + strcpy_s(info.extension, extensionPos); extensionPos[0] = '\0'; - strcpy_s(info.extension, extensionPos + 1); } } @@ -98,6 +98,7 @@ bool ModUnload(uint Base) // Unload everything from TitanEngine StaticFileUnloadW(nullptr, false, found->second.Handle, found->second.FileMapSize, found->second.MapHandle, found->second.FileMapVA); + EXCLUSIVE_RELEASE(); // Update symbols SymUpdateModuleList(); diff --git a/x64_dbg_dbg/threading.cpp b/x64_dbg_dbg/threading.cpp index 3475196e..2f421b3e 100644 --- a/x64_dbg_dbg/threading.cpp +++ b/x64_dbg_dbg/threading.cpp @@ -28,10 +28,10 @@ bool waitislocked(WAIT_ID id) return waitarray[id]; } -bool ExclusiveSectionLocker::m_Initialized = false; -SRWLOCK ExclusiveSectionLocker::m_Locks[SectionLock::LockLast]; +bool SectionLockerGlobal::m_Initialized = false; +SRWLOCK SectionLockerGlobal::m_Locks[SectionLock::LockLast]; -void ExclusiveSectionLocker::Initialize() +void SectionLockerGlobal::Initialize() { if(m_Initialized) return; @@ -45,7 +45,7 @@ void ExclusiveSectionLocker::Initialize() m_Initialized = true; } -void ExclusiveSectionLocker::Deinitialize() +void SectionLockerGlobal::Deinitialize() { if(!m_Initialized) return; @@ -63,38 +63,7 @@ void ExclusiveSectionLocker::Deinitialize() m_Initialized = false; } -ExclusiveSectionLocker::ExclusiveSectionLocker(SectionLock LockIndex) -{ - m_Lock = &m_Locks[LockIndex]; - m_LockCount = 0; - - Lock(); -} - -ExclusiveSectionLocker::~ExclusiveSectionLocker() -{ - if(m_LockCount > 0) - Unlock(); - - // TODO: Assert that the lock count is zero on destructor -#ifdef _DEBUG - if(m_LockCount > 0) - __debugbreak(); -#endif -} - -void ExclusiveSectionLocker::Lock() -{ - AcquireSRWLockExclusive(m_Lock); - m_LockCount++; -} - -void ExclusiveSectionLocker::Unlock() -{ - m_LockCount--; - ReleaseSRWLockExclusive(m_Lock); -} - +/* SharedSectionLocker::SharedSectionLocker(SectionLock LockIndex) : ExclusiveSectionLocker(LockIndex) { @@ -111,4 +80,5 @@ void SharedSectionLocker::Unlock() { m_LockCount--; ReleaseSRWLockShared(m_Lock); -} \ No newline at end of file +} +*/ \ No newline at end of file diff --git a/x64_dbg_dbg/threading.h b/x64_dbg_dbg/threading.h index eb029d9b..3e758d3b 100644 --- a/x64_dbg_dbg/threading.h +++ b/x64_dbg_dbg/threading.h @@ -2,7 +2,6 @@ #include "_global.h" -//enums enum WAIT_ID { WAITID_RUN, @@ -23,10 +22,13 @@ bool waitislocked(WAIT_ID id); // Better, but requires VISTA+ // https://msdn.microsoft.com/en-us/library/windows/desktop/aa904937%28v=vs.85%29.aspx // -#define EXCLUSIVE_ACQUIRE(Index) ExclusiveSectionLocker __ThreadLock(SectionLock::##Index); +#define CriticalSectionLocker +#define locker(x) EXCLUSIVE_ACQUIRE(x) + +#define EXCLUSIVE_ACQUIRE(Index) SectionLocker __ThreadLock(SectionLock::##Index); #define EXCLUSIVE_RELEASE() __ThreadLock.Unlock(); -#define SHARED_ACQUIRE(Index) SharedSectionLocker __SThreadLock(SectionLock::##Index); +#define SHARED_ACQUIRE(Index) SectionLocker __SThreadLock(SectionLock::##Index); #define SHARED_RELEASE() __SThreadLock.Unlock(); enum SectionLock @@ -53,32 +55,56 @@ enum SectionLock LockLast }; -class ExclusiveSectionLocker +class SectionLockerGlobal { + template friend class SectionLocker; + public: static void Initialize(); static void Deinitialize(); - ExclusiveSectionLocker(SectionLock LockIndex); - ~ExclusiveSectionLocker(); - - void Lock(); - void Unlock(); - -private: +protected: static bool m_Initialized; static SRWLOCK m_Locks[SectionLock::LockLast]; - -protected: - SRWLOCK* m_Lock; - BYTE m_LockCount; }; -class SharedSectionLocker : public ExclusiveSectionLocker +template +class SectionLocker { public: - SharedSectionLocker(SectionLock LockIndex); + SectionLocker(SectionLock LockIndex) + { + m_Lock = &SectionLockerGlobal::m_Locks[LockIndex]; + m_LockCount = 0; - void Lock(); - void Unlock(); + Lock(); + } + + ~SectionLocker() + { + if(m_LockCount > 0) + Unlock(); + + // TODO: Assert that the lock count is zero on destructor +#ifdef _DEBUG + if(m_LockCount > 0) + __debugbreak(); +#endif + } + + inline void Lock() + { + AcquireSRWLockExclusive(m_Lock); + m_LockCount++; + } + + inline void Unlock() + { + m_LockCount--; + ReleaseSRWLockExclusive(m_Lock); + } + +protected: + PSRWLOCK m_Lock; + BYTE m_LockCount; }; \ No newline at end of file diff --git a/x64_dbg_dbg/variable.cpp b/x64_dbg_dbg/variable.cpp index f71905a0..fe4765c8 100644 --- a/x64_dbg_dbg/variable.cpp +++ b/x64_dbg_dbg/variable.cpp @@ -20,16 +20,23 @@ static void varsetvalue(VAR* var, VAR_VALUE* value) static bool varset(const char* name, VAR_VALUE* value, bool setreadonly) { - CriticalSectionLocker locker(LockVariables); + EXCLUSIVE_ACQUIRE(LockVariables); + String name_; if(*name != '$') name_ = "$"; name_ += name; VariableMap::iterator found = variables.find(name_); - if(found == variables.end()) //not found + if(found == variables.end()) //not found return false; if(found->second.alias.length()) + { + // Release the lock (potential deadlock here) + EXCLUSIVE_RELEASE(); + return varset(found->second.alias.c_str(), value, setreadonly); + } + if(!setreadonly && (found->second.type == VAR_READONLY || found->second.type == VAR_HIDDEN)) return false; varsetvalue(&found->second, value); @@ -69,9 +76,11 @@ VAR* vargetptr() bool varnew(const char* name, uint value, VAR_TYPE type) { - CriticalSectionLocker locker(LockVariables); if(!name) return false; + + CriticalSectionLocker locker(LockVariables); + std::vector names = StringUtils::Split(name, '\1'); String firstName; for(int i = 0; i < (int)names.size(); i++) @@ -100,7 +109,8 @@ bool varnew(const char* name, uint value, VAR_TYPE type) static bool varget(const char* name, VAR_VALUE* value, int* size, VAR_TYPE* type) { - CriticalSectionLocker locker(LockVariables); + EXCLUSIVE_ACQUIRE(LockVariables); + String name_; if(*name != '$') name_ = "$"; @@ -109,7 +119,12 @@ static bool varget(const char* name, VAR_VALUE* value, int* size, VAR_TYPE* type if(found == variables.end()) //not found return false; if(found->second.alias.length()) + { + // Release the lock (potential deadlock here) + EXCLUSIVE_RELEASE(); + return varget(found->second.alias.c_str(), value, size, type); + } if(type) *type = found->second.type; if(size) @@ -184,7 +199,8 @@ bool varset(const char* name, const char* string, bool setreadonly) bool vardel(const char* name, bool delsystem) { - CriticalSectionLocker locker(LockVariables); + EXCLUSIVE_ACQUIRE(LockVariables); + String name_; if(*name != '$') name_ = "$"; @@ -193,7 +209,13 @@ bool vardel(const char* name, bool delsystem) if(found == variables.end()) //not found return false; if(found->second.alias.length()) + { + // Release the lock (potential deadlock here) + EXCLUSIVE_RELEASE(); + return vardel(found->second.alias.c_str(), delsystem); + } + if(!delsystem && found->second.type != VAR_USER) return false; found = variables.begin(); diff --git a/x64_dbg_dbg/x64_dbg.cpp b/x64_dbg_dbg/x64_dbg.cpp index ff2b7dba..eea09dde 100644 --- a/x64_dbg_dbg/x64_dbg.cpp +++ b/x64_dbg_dbg/x64_dbg.cpp @@ -236,6 +236,7 @@ extern "C" DLL_EXPORT const char* _dbg_dbginit() return "Invalid TITAN_ENGINE_CONTEXT_t alignment!"; if(sizeof(TITAN_ENGINE_CONTEXT_t) != sizeof(REGISTERCONTEXT)) return "Invalid REGISTERCONTEXT alignment!"; + SectionLockerGlobal::Initialize(); dbginit(); dbgfunctionsinit(); json_set_alloc_funcs(emalloc_json, efree_json); @@ -314,7 +315,8 @@ extern "C" DLL_EXPORT void _dbg_dbgexitsignal() } else DeleteFileA(alloctrace); - CriticalSectionLocker::Deinitialize(); + + SectionLockerGlobal::Deinitialize(); } extern "C" DLL_EXPORT bool _dbg_dbgcmddirectexec(const char* cmd)