Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alloc/dealloc mismatches and some buffer overruns found by ASAN #80189

Merged
merged 3 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/ilasm/asmman.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class AsmMan
~AsmMan()
{
if(m_pAssembly) delete m_pAssembly;
if(m_szScopeName) delete m_szScopeName;
if(m_szScopeName) delete[] m_szScopeName;
if(m_pGUID) delete m_pGUID;
};
void SetErrorReporter(ErrorReporter* rpt) { report = rpt; };
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/md/ceefilegen/pesectionman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ HRESULT PESection::addSectReloc(unsigned offset, PESection *relativeTo,
}

memcpy(relocNew, m_relocStart, sizeof(PESectionReloc)*curLen);
delete m_relocStart;
delete[] m_relocStart;
m_relocStart = relocNew;
m_relocCur = &m_relocStart[curLen];
m_relocEnd = &m_relocStart[newLen];
Expand Down Expand Up @@ -403,7 +403,7 @@ HRESULT PESection::cloneInstance(PESection *destination) {
newSize = (INT32)(m_relocCur-m_relocStart);

if (newSize>(destination->m_relocEnd - destination->m_relocStart)) {
delete destination->m_relocStart;
delete[] destination->m_relocStart;

destination->m_relocStart = new (nothrow) PESectionReloc[newSize];
if (destination->m_relocStart == NULL)
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/md/runtime/mdcolumndescriptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,11 @@ const BYTE CMiniMdBase::s_GenericParamConstraintCol[] = {1,
};
#ifdef FEATURE_METADATA_EMIT_PORTABLE_PDB
// Dummy descriptors to fill the gap to 0x30
const BYTE CMiniMdBase::s_Dummy1Col[] = { NULL };
const BYTE CMiniMdBase::s_Dummy2Col[] = { NULL };
const BYTE CMiniMdBase::s_Dummy3Col[] = { NULL };
// Our parsing process assumes that each colum def will have at least 1 entry,
// so add enough bytes for a full table descriptor (1 + sizeof(CMiniColDef) bytes)
const BYTE CMiniMdBase::s_Dummy1Col[] = { 0, 0, 0, 0 };
const BYTE CMiniMdBase::s_Dummy2Col[] = { 0, 0, 0, 0 };
const BYTE CMiniMdBase::s_Dummy3Col[] = { 0, 0, 0, 0 };
// Actual portable PDB tables descriptors
const BYTE CMiniMdBase::s_DocumentCol[] = { 2,
103,0,2, 102,2,2, 103,4,2, 102,6,2,
Expand Down
18 changes: 4 additions & 14 deletions src/coreclr/vm/threadstatics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void ThreadLocalBlock::FreeTLM(SIZE_T i, BOOL isThreadShuttingdown)
pThreadLocalModule->m_pDynamicClassTable[k].m_pDynamicEntry = NULL;
}
}
delete pThreadLocalModule->m_pDynamicClassTable;
delete[] pThreadLocalModule->m_pDynamicClassTable;
pThreadLocalModule->m_pDynamicClassTable = NULL;
}

Expand Down Expand Up @@ -525,33 +525,23 @@ void ThreadLocalModule::AllocateDynamicClass(MethodTable *pMT)
{
if (pDynamicStatics == NULL)
{
SIZE_T dynamicEntrySize;
// If these allocations fail, we will throw
if (pMT->Collectible())
{
dynamicEntrySize = sizeof(CollectibleDynamicEntry);
pDynamicStatics = new CollectibleDynamicEntry(pMT->GetLoaderAllocator());
}
else
{
dynamicEntrySize = DynamicEntry::GetOffsetOfDataBlob() + dwStaticBytes;
pDynamicStatics = new({dwStaticBytes}) NormalDynamicEntry();
}

// If this allocation fails, we will throw
pDynamicStatics = (DynamicEntry*)new BYTE[dynamicEntrySize];

#ifdef FEATURE_64BIT_ALIGNMENT
// The memory block has be aligned at MAX_PRIMITIVE_FIELD_SIZE to guarantee alignment of statics
static_assert_no_msg(sizeof(NormalDynamicEntry) % MAX_PRIMITIVE_FIELD_SIZE == 0);
_ASSERTE(IS_ALIGNED(pDynamicStatics, MAX_PRIMITIVE_FIELD_SIZE));
#endif

// Zero out the new DynamicEntry
memset((BYTE*)pDynamicStatics, 0, dynamicEntrySize);

if (pMT->Collectible())
{
((CollectibleDynamicEntry*)pDynamicStatics)->m_pLoaderAllocator = pMT->GetLoaderAllocator();
}

// Save the DynamicEntry in the DynamicClassTable
m_pDynamicClassTable[dwID].m_pDynamicEntry = pDynamicStatics;
}
Expand Down
22 changes: 22 additions & 0 deletions src/coreclr/vm/threadstatics.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ struct ThreadLocalModule

struct CollectibleDynamicEntry : public DynamicEntry
{
CollectibleDynamicEntry(PTR_LoaderAllocator pLoaderAllocator)
:m_pLoaderAllocator(pLoaderAllocator)
{
LIMITED_METHOD_CONTRACT;
}

LOADERHANDLE m_hGCStatics;
LOADERHANDLE m_hNonGCStatics;
PTR_LoaderAllocator m_pLoaderAllocator;
Expand Down Expand Up @@ -133,6 +139,22 @@ struct ThreadLocalModule
SUPPORTS_DAC;
return dac_cast<PTR_BYTE>(this);
}

struct DynamicEntryStaticBytes
{
DWORD m_bytes;
};

static void* operator new(size_t) = delete;

static void* operator new(size_t baseSize, DynamicEntryStaticBytes dataBlobSize)
{
void* memory = ::operator new(baseSize + dataBlobSize.m_bytes);
// We want to zero out the data blob memory as the NormalDynamicEntry constructor
// will not zero it as it is outsize of the object.
memset((int8_t*)memory + baseSize, 0, dataBlobSize.m_bytes);
return memory;
}
};
typedef DPTR(NormalDynamicEntry) PTR_NormalDynamicEntry;

Expand Down