Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fill freed loader heap chunk with non-zero value #12731

Merged
merged 12 commits into from
Jul 31, 2017
25 changes: 15 additions & 10 deletions src/inc/loaderheap.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class UnlockedLoaderHeap

size_t * m_pPrivatePerfCounter_LoaderBytes;

DWORD m_flProtect;
DWORD m_Options;

LoaderHeapFreeBlock *m_pFirstFreeBlock;

Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this flag fZeroInit = TRUE to make it clear what it does.


~UnlockedLoaderHeap();
#endif
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand All @@ -469,15 +472,17 @@ 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,
dwReservedRegionAddress,
dwReservedRegionSize,
pPrivatePerfCounter_LoaderBytes,
pRangeList,
fMakeExecutable)
fMakeExecutable,
fMakeRelaxed)
{
WRAPPER_NO_CONTRACT;
m_CriticalSection = NULL;
Expand Down
35 changes: 26 additions & 9 deletions src/utilcode/loaderheap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#define DONOT_DEFINE_ETW_CALLBACK
#include "eventtracebase.h"

#define LHF_EXECUTABLE 0x1
#define LHF_RELAXED 0x2

#ifndef DACCESS_COMPILE

INDEBUG(DWORD UnlockedLoaderHeap::s_dwNumInstancesOfLoaderHeaps = 0;)
Expand Down Expand Up @@ -903,7 +906,8 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize,
SIZE_T dwReservedRegionSize,
size_t *pPrivatePerfCounter_LoaderBytes,
RangeList *pRangeList,
BOOL fMakeExecutable)
BOOL fMakeExecutable,
BOOL fMakeRelaxed)
{
CONTRACTL
{
Expand Down Expand Up @@ -939,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;

Expand Down Expand Up @@ -1133,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");
Expand Down Expand Up @@ -1225,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;

Expand Down Expand Up @@ -1353,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");
}

Expand Down Expand Up @@ -1529,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, 0, dwSize); // Must zero init this memory as AllocMem expect it
if ((m_Options & LHF_RELAXED) == 0)
memset(pMem, 0x00, dwSize); // Fill freed region with 0
m_pAllocPtr = (BYTE*)pMem;
}
else
Expand Down Expand Up @@ -1639,6 +1645,7 @@ void *UnlockedLoaderHeap::UnlockedAllocAlignedMem_NoThrow(size_t dwRequestedSiz


((BYTE*&)pResult) += extra;

#ifdef _DEBUG
BYTE *pAllocatedBytes = (BYTE *)pResult;
#if LOADER_HEAP_DEBUG_BOUNDARY > 0
Expand All @@ -1648,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");
}

Expand Down Expand Up @@ -1766,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)
Expand Down
12 changes: 3 additions & 9 deletions src/vm/dllimportcallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other thread can allocate the block at this point. This needs to be done before the call to BackoutMem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0xCC works well for x86 because of it is code for the breakpoint instruction. What does it do on arm?

}

VOID UMEntryThunk::FreeUMEntryThunk(UMEntryThunk* p)
Expand Down
4 changes: 0 additions & 4 deletions src/vm/dllimportcallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,6 @@ class UMEntryThunk
{
DestroyLongWeakHandle(GetObjectHandle());
}

#ifdef _DEBUG
FillMemory(this, sizeof(*this), 0xcc);
#endif
}

void Terminate();
Expand Down
5 changes: 3 additions & 2 deletions src/vm/loaderallocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should not take the flag.

{
STANDARD_VM_CONTRACT;

Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be false unconditionally.

initReservedMem += dwExecutableHeapReserveSize;
}

Expand Down
2 changes: 1 addition & 1 deletion src/vm/loaderallocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion src/vm/loaderallocator.inl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down