From 3384270b5319371e6aec05f3ce5e5dd63c99c8a0 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Tue, 11 Jul 2017 10:40:05 +0900 Subject: [PATCH 01/12] Add FEATURE_LOADER_HEAP_GUARD feature --- clrdefinitions.cmake | 3 +++ src/utilcode/loaderheap.cpp | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake index 7fb65377312f..503e5d251931 100644 --- a/clrdefinitions.cmake +++ b/clrdefinitions.cmake @@ -136,6 +136,9 @@ if(FEATURE_INTERPRETER) add_definitions(-DFEATURE_INTERPRETER) endif(FEATURE_INTERPRETER) add_definitions(-DFEATURE_ISYM_READER) +if (FEATURE_LOADER_HEAP_GUARD) + add_definitions(-DFEATURE_LOADER_HEAP_GUARD) +endif(FEATURE_LOADER_HEAP_GUARD) add_definitions(-DFEATURE_LOADER_OPTIMIZATION) if (CLR_CMAKE_PLATFORM_LINUX OR WIN32) add_definitions(-DFEATURE_MANAGED_ETW) diff --git a/src/utilcode/loaderheap.cpp b/src/utilcode/loaderheap.cpp index 3f1063ce8e3d..4c323f17aa9e 100644 --- a/src/utilcode/loaderheap.cpp +++ b/src/utilcode/loaderheap.cpp @@ -10,6 +10,12 @@ #define DONOT_DEFINE_ETW_CALLBACK #include "eventtracebase.h" +#ifdef FEATURE_LOADER_HEAP_GUARD +#define LOADER_HEAP_REDZONE_VALUE 0xcc +#else +#define LOADER_HEAP_REDZONE_VALUE 0 +#endif + #ifndef DACCESS_COMPILE INDEBUG(DWORD UnlockedLoaderHeap::s_dwNumInstancesOfLoaderHeaps = 0;) @@ -723,6 +729,9 @@ struct LoaderHeapFreeBlock } #endif +#if LOADER_HEAP_REDZONE_VALUE != 0 + memset(pMem, LOADER_HEAP_REDZONE_VALUE, dwTotalSize); +#endif INDEBUG(memset(pMem, 0xcc, dwTotalSize);) LoaderHeapFreeBlock *pNewBlock = (LoaderHeapFreeBlock*)pMem; pNewBlock->m_pNext = *ppHead; @@ -1344,6 +1353,10 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize if (pData) { +#if LOADER_HEAP_REDZONE_VALUE != 0 + memset((BYTE *)pData, 0x00, dwSize); +#endif + #ifdef _DEBUG BYTE *pAllocatedBytes = (BYTE *)pData; @@ -1529,7 +1542,7 @@ void UnlockedLoaderHeap::UnlockedBackoutMem(void *pMem, { // Cool. This was the last block allocated. We can just undo the allocation instead // of going to the freelist. - memset(pMem, 0, dwSize); // Must zero init this memory as AllocMem expect it + memset(pMem, LOADER_HEAP_REDZONE_VALUE, dwSize); // Fill freed region with LOADER_HEAP_REDZONE_VALUE m_pAllocPtr = (BYTE*)pMem; } else @@ -1639,6 +1652,11 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz ((BYTE*&)pResult) += extra; + +#if LOADER_HEAP_REDZONE_VALUE != 0 + memset((BYTE *)pResult, 0x00, dwRequestedSize); +#endif + #ifdef _DEBUG BYTE *pAllocatedBytes = (BYTE *)pResult; #if LOADER_HEAP_DEBUG_BOUNDARY > 0 From 48e667bad2f0f380a5c28204f1c66193bb3fbed0 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Tue, 18 Jul 2017 08:43:43 +0900 Subject: [PATCH 02/12] Invoke memset only for reclaimed regions --- src/utilcode/loaderheap.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/utilcode/loaderheap.cpp b/src/utilcode/loaderheap.cpp index 4c323f17aa9e..bf79a2ba1187 100644 --- a/src/utilcode/loaderheap.cpp +++ b/src/utilcode/loaderheap.cpp @@ -1354,7 +1354,12 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize if (pData) { #if LOADER_HEAP_REDZONE_VALUE != 0 - memset((BYTE *)pData, 0x00, dwSize); + { + BYTE *pAllocatedBytes = (BYTE *)pData; + + if (pAllocatedBytes[0] != 0x00) + memset(pAllocatedBytes, 0x00, dwSize); + } #endif #ifdef _DEBUG @@ -1654,7 +1659,12 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz ((BYTE*&)pResult) += extra; #if LOADER_HEAP_REDZONE_VALUE != 0 - memset((BYTE *)pResult, 0x00, dwRequestedSize); + { + BYTE *pAllocatedBytes = (BYTE *)pResult; + + if (pAllocatedBytes[0] != 0x00) + memset(pAllocatedBytes, 0x00, dwRequestedSize); + } #endif #ifdef _DEBUG From 87b228715848eecc781ad3303c5cc7b1261a9dca Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Tue, 18 Jul 2017 15:26:34 +0900 Subject: [PATCH 03/12] Enable FEATURE_LOADER_HEAP_GUARD by default --- clrdefinitions.cmake | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake index 503e5d251931..1e9d7c92b568 100644 --- a/clrdefinitions.cmake +++ b/clrdefinitions.cmake @@ -136,9 +136,7 @@ if(FEATURE_INTERPRETER) add_definitions(-DFEATURE_INTERPRETER) endif(FEATURE_INTERPRETER) add_definitions(-DFEATURE_ISYM_READER) -if (FEATURE_LOADER_HEAP_GUARD) - add_definitions(-DFEATURE_LOADER_HEAP_GUARD) -endif(FEATURE_LOADER_HEAP_GUARD) +add_definitions(-DFEATURE_LOADER_HEAP_GUARD) add_definitions(-DFEATURE_LOADER_OPTIMIZATION) if (CLR_CMAKE_PLATFORM_LINUX OR WIN32) add_definitions(-DFEATURE_MANAGED_ETW) From 66939d10c57b8e2715fd984420db89b2bccda53e Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Wed, 19 Jul 2017 10:39:54 +0900 Subject: [PATCH 04/12] Insert trap inside UMEntryThunk::Terminate --- clrdefinitions.cmake | 1 - src/inc/loaderheap.h | 25 +++++++++------ src/utilcode/loaderheap.cpp | 59 +++++++++++++++--------------------- src/vm/dllimportcallback.cpp | 12 ++------ src/vm/dllimportcallback.h | 4 --- src/vm/loaderallocator.cpp | 5 +-- src/vm/loaderallocator.hpp | 2 +- src/vm/loaderallocator.inl | 2 +- 8 files changed, 47 insertions(+), 63 deletions(-) diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake index 1e9d7c92b568..7fb65377312f 100644 --- a/clrdefinitions.cmake +++ b/clrdefinitions.cmake @@ -136,7 +136,6 @@ if(FEATURE_INTERPRETER) add_definitions(-DFEATURE_INTERPRETER) endif(FEATURE_INTERPRETER) add_definitions(-DFEATURE_ISYM_READER) -add_definitions(-DFEATURE_LOADER_HEAP_GUARD) add_definitions(-DFEATURE_LOADER_OPTIMIZATION) if (CLR_CMAKE_PLATFORM_LINUX OR WIN32) add_definitions(-DFEATURE_MANAGED_ETW) diff --git a/src/inc/loaderheap.h b/src/inc/loaderheap.h index 7d4c48f5e86f..ae3965c09233 100644 --- a/src/inc/loaderheap.h +++ b/src/inc/loaderheap.h @@ -217,7 +217,7 @@ class UnlockedLoaderHeap size_t * m_pPrivatePerfCounter_LoaderBytes; - DWORD m_flProtect; + DWORD m_Options; LoaderHeapFreeBlock *m_pFirstFreeBlock; @@ -288,7 +288,8 @@ class UnlockedLoaderHeap SIZE_T dwReservedRegionSize, size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, - BOOL fMakeExecutable = FALSE); + BOOL fMakeExecutable = FALSE, + BOOL fMakeRelaxed = FALSE); ~UnlockedLoaderHeap(); #endif @@ -398,10 +399,10 @@ class UnlockedLoaderHeap return m_dwTotalAlloc; } - BOOL IsExecutable() - { - return (PAGE_EXECUTE_READWRITE == m_flProtect); - } + BOOL IsExecutable(); + // Relaxed Loader Heap does not guarntee that allocated chunk is + // zero-initialized. + BOOL IsRelaxed(); public: @@ -447,14 +448,16 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout DWORD dwCommitBlockSize, size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, - BOOL fMakeExecutable = FALSE + BOOL fMakeExecutable = FALSE, + BOOL fMakeRelaxed = FALSE ) : UnlockedLoaderHeap(dwReserveBlockSize, dwCommitBlockSize, NULL, 0, pPrivatePerfCounter_LoaderBytes, pRangeList, - fMakeExecutable) + fMakeExecutable, + fMakeRelaxed) { WRAPPER_NO_CONTRACT; m_CriticalSection = NULL; @@ -469,7 +472,8 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout SIZE_T dwReservedRegionSize, size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, - BOOL fMakeExecutable = FALSE + BOOL fMakeExecutable = FALSE, + BOOL fMakeRelaxed = FALSE ) : UnlockedLoaderHeap(dwReserveBlockSize, dwCommitBlockSize, @@ -477,7 +481,8 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout dwReservedRegionSize, pPrivatePerfCounter_LoaderBytes, pRangeList, - fMakeExecutable) + fMakeExecutable, + fMakeRelaxed) { WRAPPER_NO_CONTRACT; m_CriticalSection = NULL; diff --git a/src/utilcode/loaderheap.cpp b/src/utilcode/loaderheap.cpp index bf79a2ba1187..6c238bb51ce8 100644 --- a/src/utilcode/loaderheap.cpp +++ b/src/utilcode/loaderheap.cpp @@ -10,11 +10,8 @@ #define DONOT_DEFINE_ETW_CALLBACK #include "eventtracebase.h" -#ifdef FEATURE_LOADER_HEAP_GUARD -#define LOADER_HEAP_REDZONE_VALUE 0xcc -#else -#define LOADER_HEAP_REDZONE_VALUE 0 -#endif +#define LHF_EXECUTABLE 0x1 +#define LHF_RELAXED 0x2 #ifndef DACCESS_COMPILE @@ -729,9 +726,6 @@ struct LoaderHeapFreeBlock } #endif -#if LOADER_HEAP_REDZONE_VALUE != 0 - memset(pMem, LOADER_HEAP_REDZONE_VALUE, dwTotalSize); -#endif INDEBUG(memset(pMem, 0xcc, dwTotalSize);) LoaderHeapFreeBlock *pNewBlock = (LoaderHeapFreeBlock*)pMem; pNewBlock->m_pNext = *ppHead; @@ -912,7 +906,8 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize, SIZE_T dwReservedRegionSize, size_t *pPrivatePerfCounter_LoaderBytes, RangeList *pRangeList, - BOOL fMakeExecutable) + BOOL fMakeExecutable, + BOOL fMakeRelaxed) { CONTRACTL { @@ -948,10 +943,11 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize, m_pPrivatePerfCounter_LoaderBytes = pPrivatePerfCounter_LoaderBytes; + m_Options = 0; if (fMakeExecutable) - m_flProtect = PAGE_EXECUTE_READWRITE; - else - m_flProtect = PAGE_READWRITE; + m_Options |= LHF_EXECUTABLE; + if (fMakeRelaxed) + m_Options |= LHF_RELAXED; m_pFirstFreeBlock = NULL; @@ -1142,7 +1138,7 @@ BOOL UnlockedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit) } // Commit first set of pages, since it will contain the LoaderHeapBlock - void *pTemp = ClrVirtualAlloc(pData, dwSizeToCommit, MEM_COMMIT, m_flProtect); + void *pTemp = ClrVirtualAlloc(pData, dwSizeToCommit, MEM_COMMIT, (m_Options & LHF_EXECUTABLE) ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE); if (pTemp == NULL) { //_ASSERTE(!"Unable to ClrVirtualAlloc commit in a loaderheap"); @@ -1234,7 +1230,7 @@ BOOL UnlockedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize) dwSizeToCommit = ALIGN_UP(dwSizeToCommit, GetOsPageSize()); // Yes, so commit the desired number of reserved pages - void *pData = ClrVirtualAlloc(m_pPtrToEndOfCommittedRegion, dwSizeToCommit, MEM_COMMIT, m_flProtect); + void *pData = ClrVirtualAlloc(m_pPtrToEndOfCommittedRegion, dwSizeToCommit, MEM_COMMIT, (m_Options & LHF_EXECUTABLE) ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE); if (pData == NULL) return FALSE; @@ -1353,15 +1349,6 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize if (pData) { -#if LOADER_HEAP_REDZONE_VALUE != 0 - { - BYTE *pAllocatedBytes = (BYTE *)pData; - - if (pAllocatedBytes[0] != 0x00) - memset(pAllocatedBytes, 0x00, dwSize); - } -#endif - #ifdef _DEBUG BYTE *pAllocatedBytes = (BYTE *)pData; @@ -1371,7 +1358,7 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize #endif if (dwRequestedSize > 0) { - _ASSERTE_MSG(pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0, + _ASSERTE_MSG((m_Options & LHF_RELAXED) || ( pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), "LoaderHeap must return zero-initialized memory"); } @@ -1547,7 +1534,8 @@ void UnlockedLoaderHeap::UnlockedBackoutMem(void *pMem, { // Cool. This was the last block allocated. We can just undo the allocation instead // of going to the freelist. - memset(pMem, LOADER_HEAP_REDZONE_VALUE, dwSize); // Fill freed region with LOADER_HEAP_REDZONE_VALUE + if ((m_Options & LHF_RELAXED) == 0) + memset(pMem, 0x00, dwSize); // Fill freed region with 0 m_pAllocPtr = (BYTE*)pMem; } else @@ -1658,15 +1646,6 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz ((BYTE*&)pResult) += extra; -#if LOADER_HEAP_REDZONE_VALUE != 0 - { - BYTE *pAllocatedBytes = (BYTE *)pResult; - - if (pAllocatedBytes[0] != 0x00) - memset(pAllocatedBytes, 0x00, dwRequestedSize); - } -#endif - #ifdef _DEBUG BYTE *pAllocatedBytes = (BYTE *)pResult; #if LOADER_HEAP_DEBUG_BOUNDARY > 0 @@ -1676,7 +1655,7 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz if (dwRequestedSize != 0) { - _ASSERTE_MSG(pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0, + _ASSERTE_MSG((m_Options & LHF_RELAXED) || (pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), "LoaderHeap must return zero-initialized memory"); } @@ -1794,6 +1773,16 @@ void *UnlockedLoaderHeap::UnlockedAllocMemForCode_NoThrow(size_t dwHeaderSize, s #endif // #ifndef DACCESS_COMPILE +BOOL UnlockedLoaderHeap::IsExecutable() +{ + return (m_Options & LHF_EXECUTABLE); +} + +BOOL UnlockedLoaderHeap::IsRelaxed() +{ + return (m_Options & LHF_RELAXED); +} + #ifdef DACCESS_COMPILE void UnlockedLoaderHeap::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) diff --git a/src/vm/dllimportcallback.cpp b/src/vm/dllimportcallback.cpp index 90c01a496bc1..698aa343dfcb 100644 --- a/src/vm/dllimportcallback.cpp +++ b/src/vm/dllimportcallback.cpp @@ -1111,13 +1111,8 @@ UMEntryThunk* UMEntryThunk::CreateUMEntryThunk() UMEntryThunk * p; -#ifdef FEATURE_WINDOWSPHONE // On the phone, use loader heap to save memory commit of regular executable heap p = (UMEntryThunk *)(void *)SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->AllocMem(S_SIZE_T(sizeof(UMEntryThunk))); -#else - p = new (executable) UMEntryThunk; - memset (p, 0, sizeof(*p)); -#endif RETURN p; } @@ -1126,11 +1121,10 @@ void UMEntryThunk::Terminate() { WRAPPER_NO_CONTRACT; -#ifdef FEATURE_WINDOWSPHONE SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->BackoutMem(this, sizeof(UMEntryThunk)); -#else - DeleteExecutable(this); -#endif + + _ASSERTE(SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->IsRelaxed()); + FillMemory(&m_code, sizeof(m_code), 0xcc); } VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p) diff --git a/src/vm/dllimportcallback.h b/src/vm/dllimportcallback.h index af2a0b1d927d..e79c5f03ef99 100644 --- a/src/vm/dllimportcallback.h +++ b/src/vm/dllimportcallback.h @@ -326,10 +326,6 @@ class UMEntryThunk { DestroyLongWeakHandle(GetObjectHandle()); } - -#ifdef _DEBUG - FillMemory(this, sizeof(*this), 0xcc); -#endif } void Terminate(); diff --git a/src/vm/loaderallocator.cpp b/src/vm/loaderallocator.cpp index 1a05bf2c059d..38589b6469e5 100644 --- a/src/vm/loaderallocator.cpp +++ b/src/vm/loaderallocator.cpp @@ -893,7 +893,7 @@ void LoaderAllocator::ActivateManagedTracking() #define COLLECTIBLE_CODEHEAP_SIZE (7 * GetOsPageSize()) #define COLLECTIBLE_VIRTUALSTUBDISPATCH_HEAP_SPACE (5 * GetOsPageSize()) -void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory) +void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory, BOOL fRelaxed) { STANDARD_VM_CONTRACT; @@ -1005,7 +1005,8 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory) dwExecutableHeapReserveSize, LOADERHEAP_PROFILE_COUNTER, NULL, - TRUE /* Make heap executable */); + TRUE /* Make heap executable */, + fRelaxed); initReservedMem += dwExecutableHeapReserveSize; } diff --git a/src/vm/loaderallocator.hpp b/src/vm/loaderallocator.hpp index 72fa59857d04..0630bf7807c7 100644 --- a/src/vm/loaderallocator.hpp +++ b/src/vm/loaderallocator.hpp @@ -396,7 +396,7 @@ class LoaderAllocator BaseDomain *GetDomain() { LIMITED_METHOD_CONTRACT; return m_pDomain; } virtual BOOL CanUnload() = 0; BOOL IsDomainNeutral(); - void Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory = NULL); + void Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory = NULL, BOOL fRelaxed = FALSE); void Terminate(); SIZE_T EstimateSize(); diff --git a/src/vm/loaderallocator.inl b/src/vm/loaderallocator.inl index 3f23ac9c8c14..50051b51a4b5 100644 --- a/src/vm/loaderallocator.inl +++ b/src/vm/loaderallocator.inl @@ -19,7 +19,7 @@ inline LOADERALLOCATORREF LoaderAllocator::GetExposedObject() inline void GlobalLoaderAllocator::Init(BaseDomain *pDomain) { - LoaderAllocator::Init(pDomain, m_ExecutableHeapInstance); + LoaderAllocator::Init(pDomain, m_ExecutableHeapInstance, TRUE); } inline void AppDomainLoaderAllocator::Init(AppDomain *pAppDomain) From 0c1395905aa85588e39b84c434c84a740c5577b0 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Wed, 19 Jul 2017 14:09:10 +0900 Subject: [PATCH 05/12] Make all exectuable heaps not to zero-initialize itself Use fZeroInit (instead of fMakeRelazed) --- src/inc/loaderheap.h | 14 ++++++-------- src/utilcode/loaderheap.cpp | 22 +++++++++++----------- src/vm/dllimportcallback.cpp | 16 +++++++++++++--- src/vm/loaderallocator.cpp | 4 ++-- src/vm/loaderallocator.hpp | 2 +- src/vm/loaderallocator.inl | 2 +- 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/inc/loaderheap.h b/src/inc/loaderheap.h index ae3965c09233..4333505e8398 100644 --- a/src/inc/loaderheap.h +++ b/src/inc/loaderheap.h @@ -289,7 +289,7 @@ class UnlockedLoaderHeap size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, BOOL fMakeExecutable = FALSE, - BOOL fMakeRelaxed = FALSE); + BOOL fZeroInit = TRUE); ~UnlockedLoaderHeap(); #endif @@ -400,9 +400,7 @@ class UnlockedLoaderHeap } BOOL IsExecutable(); - // Relaxed Loader Heap does not guarntee that allocated chunk is - // zero-initialized. - BOOL IsRelaxed(); + BOOL IsZeroInit(); public: @@ -449,7 +447,7 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, BOOL fMakeExecutable = FALSE, - BOOL fMakeRelaxed = FALSE + BOOL fZeroInit = TRUE ) : UnlockedLoaderHeap(dwReserveBlockSize, dwCommitBlockSize, @@ -457,7 +455,7 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout pPrivatePerfCounter_LoaderBytes, pRangeList, fMakeExecutable, - fMakeRelaxed) + fZeroInit) { WRAPPER_NO_CONTRACT; m_CriticalSection = NULL; @@ -473,7 +471,7 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout size_t *pPrivatePerfCounter_LoaderBytes = NULL, RangeList *pRangeList = NULL, BOOL fMakeExecutable = FALSE, - BOOL fMakeRelaxed = FALSE + BOOL fZeroInit = TRUE ) : UnlockedLoaderHeap(dwReserveBlockSize, dwCommitBlockSize, @@ -482,7 +480,7 @@ class LoaderHeap : public UnlockedLoaderHeap, public ILoaderHeapBackout pPrivatePerfCounter_LoaderBytes, pRangeList, fMakeExecutable, - fMakeRelaxed) + fZeroInit) { WRAPPER_NO_CONTRACT; m_CriticalSection = NULL; diff --git a/src/utilcode/loaderheap.cpp b/src/utilcode/loaderheap.cpp index 6c238bb51ce8..5dc489ae37c8 100644 --- a/src/utilcode/loaderheap.cpp +++ b/src/utilcode/loaderheap.cpp @@ -11,7 +11,7 @@ #include "eventtracebase.h" #define LHF_EXECUTABLE 0x1 -#define LHF_RELAXED 0x2 +#define LHF_ZEROINIT 0x2 #ifndef DACCESS_COMPILE @@ -907,7 +907,7 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize, size_t *pPrivatePerfCounter_LoaderBytes, RangeList *pRangeList, BOOL fMakeExecutable, - BOOL fMakeRelaxed) + BOOL fZeroInit) { CONTRACTL { @@ -946,8 +946,8 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize, m_Options = 0; if (fMakeExecutable) m_Options |= LHF_EXECUTABLE; - if (fMakeRelaxed) - m_Options |= LHF_RELAXED; + if (fZeroInit) + m_Options |= LHF_ZEROINIT; m_pFirstFreeBlock = NULL; @@ -1356,9 +1356,9 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize // Don't fill the memory we allocated - it is assumed to be zeroed - fill the memory after it memset(pAllocatedBytes + dwRequestedSize, 0xEE, LOADER_HEAP_DEBUG_BOUNDARY); #endif - if (dwRequestedSize > 0) + if ((dwRequestedSize > 0) && (m_Options & LHF_ZEROINIT)) { - _ASSERTE_MSG((m_Options & LHF_RELAXED) || ( pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), + _ASSERTE_MSG((pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), "LoaderHeap must return zero-initialized memory"); } @@ -1534,7 +1534,7 @@ void UnlockedLoaderHeap::UnlockedBackoutMem(void *pMem, { // Cool. This was the last block allocated. We can just undo the allocation instead // of going to the freelist. - if ((m_Options & LHF_RELAXED) == 0) + if (m_Options & LHF_ZEROINIT) memset(pMem, 0x00, dwSize); // Fill freed region with 0 m_pAllocPtr = (BYTE*)pMem; } @@ -1653,9 +1653,9 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz memset(pAllocatedBytes + dwRequestedSize, 0xee, LOADER_HEAP_DEBUG_BOUNDARY); #endif - if (dwRequestedSize != 0) + if ((dwRequestedSize != 0) && (m_Options & LHF_ZEROINIT)) { - _ASSERTE_MSG((m_Options & LHF_RELAXED) || (pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), + _ASSERTE_MSG((pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), "LoaderHeap must return zero-initialized memory"); } @@ -1778,9 +1778,9 @@ BOOL UnlockedLoaderHeap::IsExecutable() return (m_Options & LHF_EXECUTABLE); } -BOOL UnlockedLoaderHeap::IsRelaxed() +BOOL UnlockedLoaderHeap::IsZeroInit() { - return (m_Options & LHF_RELAXED); + return (m_Options & LHF_ZEROINIT); } #ifdef DACCESS_COMPILE diff --git a/src/vm/dllimportcallback.cpp b/src/vm/dllimportcallback.cpp index 698aa343dfcb..4a2963837d6e 100644 --- a/src/vm/dllimportcallback.cpp +++ b/src/vm/dllimportcallback.cpp @@ -1117,16 +1117,26 @@ UMEntryThunk* UMEntryThunk::CreateUMEntryThunk() RETURN p; } +#if defined(_TARGET_X86_) || defined(_TARGET_AMD64_) +#define REDZONE_VALUE 0xcc +#elif defined(_TARGET_ARM_) +#define REDZONE_VALUE 0xBE +#else +#define REDZONE_VALUE 0x00 +#endif + void UMEntryThunk::Terminate() { WRAPPER_NO_CONTRACT; - SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->BackoutMem(this, sizeof(UMEntryThunk)); + _ASSERTE(!SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->IsZeroInit()); + FillMemory(&m_code, sizeof(m_code), REDZONE_VALUE); - _ASSERTE(SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->IsRelaxed()); - FillMemory(&m_code, sizeof(m_code), 0xcc); + SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->BackoutMem(this, sizeof(UMEntryThunk)); } +#undef REDZONE_VALUE + VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p) { CONTRACTL diff --git a/src/vm/loaderallocator.cpp b/src/vm/loaderallocator.cpp index 38589b6469e5..7b1259e3b836 100644 --- a/src/vm/loaderallocator.cpp +++ b/src/vm/loaderallocator.cpp @@ -893,7 +893,7 @@ void LoaderAllocator::ActivateManagedTracking() #define COLLECTIBLE_CODEHEAP_SIZE (7 * GetOsPageSize()) #define COLLECTIBLE_VIRTUALSTUBDISPATCH_HEAP_SPACE (5 * GetOsPageSize()) -void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory, BOOL fRelaxed) +void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory) { STANDARD_VM_CONTRACT; @@ -1006,7 +1006,7 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory, BOO LOADERHEAP_PROFILE_COUNTER, NULL, TRUE /* Make heap executable */, - fRelaxed); + FALSE); initReservedMem += dwExecutableHeapReserveSize; } diff --git a/src/vm/loaderallocator.hpp b/src/vm/loaderallocator.hpp index 0630bf7807c7..72fa59857d04 100644 --- a/src/vm/loaderallocator.hpp +++ b/src/vm/loaderallocator.hpp @@ -396,7 +396,7 @@ class LoaderAllocator BaseDomain *GetDomain() { LIMITED_METHOD_CONTRACT; return m_pDomain; } virtual BOOL CanUnload() = 0; BOOL IsDomainNeutral(); - void Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory = NULL, BOOL fRelaxed = FALSE); + void Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory = NULL); void Terminate(); SIZE_T EstimateSize(); diff --git a/src/vm/loaderallocator.inl b/src/vm/loaderallocator.inl index 50051b51a4b5..3f23ac9c8c14 100644 --- a/src/vm/loaderallocator.inl +++ b/src/vm/loaderallocator.inl @@ -19,7 +19,7 @@ inline LOADERALLOCATORREF LoaderAllocator::GetExposedObject() inline void GlobalLoaderAllocator::Init(BaseDomain *pDomain) { - LoaderAllocator::Init(pDomain, m_ExecutableHeapInstance, TRUE); + LoaderAllocator::Init(pDomain, m_ExecutableHeapInstance); } inline void AppDomainLoaderAllocator::Init(AppDomain *pAppDomain) From 7c63a690a1e20631282d772f3adb75327ea99612 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Wed, 19 Jul 2017 14:31:16 +0900 Subject: [PATCH 06/12] Add comment --- src/vm/loaderallocator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/loaderallocator.cpp b/src/vm/loaderallocator.cpp index 7b1259e3b836..f5c67e12cab0 100644 --- a/src/vm/loaderallocator.cpp +++ b/src/vm/loaderallocator.cpp @@ -1006,7 +1006,7 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory) LOADERHEAP_PROFILE_COUNTER, NULL, TRUE /* Make heap executable */, - FALSE); + FALSE /* Do NOT zero-initialize */); initReservedMem += dwExecutableHeapReserveSize; } From cd6d5ecc1361aa406e707ca8696d2ec996a90fc6 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Wed, 19 Jul 2017 14:32:31 +0900 Subject: [PATCH 07/12] Revert unnecessary changes --- src/utilcode/loaderheap.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilcode/loaderheap.cpp b/src/utilcode/loaderheap.cpp index 5dc489ae37c8..2603a8ec333b 100644 --- a/src/utilcode/loaderheap.cpp +++ b/src/utilcode/loaderheap.cpp @@ -1358,7 +1358,7 @@ void *UnlockedLoaderHeap::UnlockedAllocMem_NoThrow(size_t dwSize #endif if ((dwRequestedSize > 0) && (m_Options & LHF_ZEROINIT)) { - _ASSERTE_MSG((pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), + _ASSERTE_MSG(pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0, "LoaderHeap must return zero-initialized memory"); } @@ -1655,7 +1655,7 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz if ((dwRequestedSize != 0) && (m_Options & LHF_ZEROINIT)) { - _ASSERTE_MSG((pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0), + _ASSERTE_MSG(pAllocatedBytes[0] == 0 && memcmp(pAllocatedBytes, pAllocatedBytes + 1, dwRequestedSize - 1) == 0, "LoaderHeap must return zero-initialized memory"); } From 63dcb89eab605ee96ba3065da55c43243e7e28b7 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Thu, 20 Jul 2017 08:21:46 +0900 Subject: [PATCH 08/12] Add and use 'Poison' method to insert a trap --- src/vm/amd64/cgenamd64.cpp | 13 +++++++++++++ src/vm/amd64/cgencpu.h | 1 + src/vm/arm/cgencpu.h | 1 + src/vm/arm/stubs.cpp | 6 ++++++ src/vm/arm64/cgencpu.h | 1 + src/vm/arm64/stubs.cpp | 4 ++++ src/vm/dllimportcallback.cpp | 12 +----------- src/vm/i386/cgencpu.h | 1 + src/vm/i386/cgenx86.cpp | 7 +++++++ 9 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/vm/amd64/cgenamd64.cpp b/src/vm/amd64/cgenamd64.cpp index 497abcd502d5..15572947ba6f 100644 --- a/src/vm/amd64/cgenamd64.cpp +++ b/src/vm/amd64/cgenamd64.cpp @@ -670,6 +670,19 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) _ASSERTE(DbgIsExecutable(&m_movR10[0], &m_jmpRAX[3]-&m_movR10[0])); } +void UMEntryThunkCode::Poison() +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + } + CONTRACTL_END; + + m_movR10[0] = 0xCC; +} + UMEntryThunk* UMEntryThunk::Decode(LPVOID pCallback) { LIMITED_METHOD_CONTRACT; diff --git a/src/vm/amd64/cgencpu.h b/src/vm/amd64/cgencpu.h index 64a6501dc0ea..b74e3ca7d3ea 100644 --- a/src/vm/amd64/cgencpu.h +++ b/src/vm/amd64/cgencpu.h @@ -472,6 +472,7 @@ struct DECLSPEC_ALIGN(8) UMEntryThunkCode BYTE m_padding2[5]; void Encode(BYTE* pTargetCode, void* pvSecretParam); + void Poison(); LPCBYTE GetEntryPoint() const { diff --git a/src/vm/arm/cgencpu.h b/src/vm/arm/cgencpu.h index 181d5f10eb97..6f128f653455 100644 --- a/src/vm/arm/cgencpu.h +++ b/src/vm/arm/cgencpu.h @@ -988,6 +988,7 @@ struct DECLSPEC_ALIGN(4) UMEntryThunkCode TADDR m_pvSecretParam; void Encode(BYTE* pTargetCode, void* pvSecretParam); + void Poison(); LPCBYTE GetEntryPoint() const { diff --git a/src/vm/arm/stubs.cpp b/src/vm/arm/stubs.cpp index 2e8bb19d499f..a5113587f9bf 100644 --- a/src/vm/arm/stubs.cpp +++ b/src/vm/arm/stubs.cpp @@ -2522,6 +2522,12 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) FlushInstructionCache(GetCurrentProcess(),&m_code,sizeof(m_code)); } +void UMEntryThunkCode::Poison() +{ + m_code[0] = 0xbebe; + FlushInstructionCache(GetCurrentProcess(),&m_code,sizeof(m_code)); +} + ///////////////////////////// UNIMPLEMENTED ////////////////////////////////// #ifndef DACCESS_COMPILE diff --git a/src/vm/arm64/cgencpu.h b/src/vm/arm64/cgencpu.h index d8bbcf7d1d61..5c522c573515 100644 --- a/src/vm/arm64/cgencpu.h +++ b/src/vm/arm64/cgencpu.h @@ -481,6 +481,7 @@ struct DECLSPEC_ALIGN(16) UMEntryThunkCode TADDR m_pvSecretParam; void Encode(BYTE* pTargetCode, void* pvSecretParam); + void Poison(); LPCBYTE GetEntryPoint() const { diff --git a/src/vm/arm64/stubs.cpp b/src/vm/arm64/stubs.cpp index 40d274959f8e..df2124db4484 100644 --- a/src/vm/arm64/stubs.cpp +++ b/src/vm/arm64/stubs.cpp @@ -1244,6 +1244,10 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) FlushInstructionCache(GetCurrentProcess(),&m_code,sizeof(m_code)); } +void UMEntryThunkCode::Poison() +{ + +} #ifdef PROFILING_SUPPORTED #include "proftoeeinterfaceimpl.h" diff --git a/src/vm/dllimportcallback.cpp b/src/vm/dllimportcallback.cpp index 4a2963837d6e..8684c12167b4 100644 --- a/src/vm/dllimportcallback.cpp +++ b/src/vm/dllimportcallback.cpp @@ -1117,26 +1117,16 @@ UMEntryThunk* UMEntryThunk::CreateUMEntryThunk() RETURN p; } -#if defined(_TARGET_X86_) || defined(_TARGET_AMD64_) -#define REDZONE_VALUE 0xcc -#elif defined(_TARGET_ARM_) -#define REDZONE_VALUE 0xBE -#else -#define REDZONE_VALUE 0x00 -#endif - void UMEntryThunk::Terminate() { WRAPPER_NO_CONTRACT; _ASSERTE(!SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->IsZeroInit()); - FillMemory(&m_code, sizeof(m_code), REDZONE_VALUE); + m_code.Poison(); SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap()->BackoutMem(this, sizeof(UMEntryThunk)); } -#undef REDZONE_VALUE - VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p) { CONTRACTL diff --git a/src/vm/i386/cgencpu.h b/src/vm/i386/cgencpu.h index ff76d992fc2e..e4a623b715ce 100644 --- a/src/vm/i386/cgencpu.h +++ b/src/vm/i386/cgencpu.h @@ -504,6 +504,7 @@ struct DECLSPEC_ALIGN(4) UMEntryThunkCode const BYTE * m_execstub; // pointer to destination code // make sure the backpatched portion is dword aligned. void Encode(BYTE* pTargetCode, void* pvSecretParam); + void Poison(); LPCBYTE GetEntryPoint() const { diff --git a/src/vm/i386/cgenx86.cpp b/src/vm/i386/cgenx86.cpp index c75490babdfe..e315ffb1e6e9 100644 --- a/src/vm/i386/cgenx86.cpp +++ b/src/vm/i386/cgenx86.cpp @@ -1588,6 +1588,13 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) FlushInstructionCache(GetCurrentProcess(),GetEntryPoint(),sizeof(UMEntryThunkCode)); } +void UMEntryThunkCode::Poison() +{ + LIMITED_METHOD_CONTRACT; + + m_movEAX = X86_INSTR_INT3; +} + UMEntryThunk* UMEntryThunk::Decode(LPVOID pCallback) { LIMITED_METHOD_CONTRACT; From 03e2ed1150ec2e881e108400b6dcfbf3c792b024 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Thu, 20 Jul 2017 12:59:09 +0900 Subject: [PATCH 09/12] Do NOT invoke FlushInstructionCache --- src/vm/arm/stubs.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vm/arm/stubs.cpp b/src/vm/arm/stubs.cpp index a5113587f9bf..e54e9b429290 100644 --- a/src/vm/arm/stubs.cpp +++ b/src/vm/arm/stubs.cpp @@ -2525,7 +2525,6 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) void UMEntryThunkCode::Poison() { m_code[0] = 0xbebe; - FlushInstructionCache(GetCurrentProcess(),&m_code,sizeof(m_code)); } ///////////////////////////// UNIMPLEMENTED ////////////////////////////////// From 4029d1cd37ab0cc3c77f4f7c7c5f69e1fb763ee0 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Thu, 20 Jul 2017 13:01:21 +0900 Subject: [PATCH 10/12] Update comment --- src/vm/loaderallocator.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vm/loaderallocator.cpp b/src/vm/loaderallocator.cpp index f5c67e12cab0..ff54277efdca 100644 --- a/src/vm/loaderallocator.cpp +++ b/src/vm/loaderallocator.cpp @@ -1006,7 +1006,8 @@ void LoaderAllocator::Init(BaseDomain *pDomain, BYTE *pExecutableHeapMemory) LOADERHEAP_PROFILE_COUNTER, NULL, TRUE /* Make heap executable */, - FALSE /* Do NOT zero-initialize */); + FALSE /* Disable zero-initialization (needed by UMEntryThunkCode::Poison) */ + ); initReservedMem += dwExecutableHeapReserveSize; } From c986ba457e0fe41917a43c9dec74978f76fd1041 Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Mon, 31 Jul 2017 15:04:11 +0900 Subject: [PATCH 11/12] Add comment on ARM Poisoning instruction --- src/vm/arm/stubs.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vm/arm/stubs.cpp b/src/vm/arm/stubs.cpp index e54e9b429290..39b5937129a3 100644 --- a/src/vm/arm/stubs.cpp +++ b/src/vm/arm/stubs.cpp @@ -2524,6 +2524,7 @@ void UMEntryThunkCode::Encode(BYTE* pTargetCode, void* pvSecretParam) void UMEntryThunkCode::Poison() { + // Insert 'bkpt 0x00be' at the entry point m_code[0] = 0xbebe; } From 9c97b74a262965c2d51bfae9a774d371dd62003c Mon Sep 17 00:00:00 2001 From: Jonghyun Park Date: Mon, 31 Jul 2017 15:04:29 +0900 Subject: [PATCH 12/12] Use X86_INSTR_INT3 instead of 0xCC --- src/vm/amd64/cgenamd64.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm/amd64/cgenamd64.cpp b/src/vm/amd64/cgenamd64.cpp index 15572947ba6f..20dca22e36bc 100644 --- a/src/vm/amd64/cgenamd64.cpp +++ b/src/vm/amd64/cgenamd64.cpp @@ -680,7 +680,7 @@ void UMEntryThunkCode::Poison() } CONTRACTL_END; - m_movR10[0] = 0xCC; + m_movR10[0] = X86_INSTR_INT3; } UMEntryThunk* UMEntryThunk::Decode(LPVOID pCallback)