Skip to content

Commit 1f30b77

Browse files
committed
Fix corruption in virtual methods cache table
*** PR separately ***
1 parent 542bc41 commit 1f30b77

File tree

1 file changed

+86
-0
lines changed

1 file changed

+86
-0
lines changed

src/CLR/Core/Cache.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,59 @@ void CLR_RT_EventCache::VirtualMethodTable::Initialize()
196196
m_entriesMRU = (Link *)&g_scratchVirtualMethodTableLinkMRU[0];
197197
m_payloads = (Payload *)&g_scratchVirtualMethodPayload[0];
198198

199+
#ifdef DEBUG
200+
201+
// DIAGNOSTIC: Verify structure sizes and alignment
202+
CLR_Debug::Printf("\r\n========== VirtualMethodTable::Initialize DIAGNOSTICS ==========\r\n");
203+
CLR_Debug::Printf("sizeof(Link) = %u, alignof(Link) = %u\r\n", sizeof(Link), alignof(Link));
204+
CLR_Debug::Printf("sizeof(Payload) = %u, alignof(Payload) = %u\r\n", sizeof(Payload), alignof(Payload));
205+
CLR_Debug::Printf("sizeof(Payload::Key) = %u\r\n", sizeof(Payload::Key));
206+
CLR_Debug::Printf("LinkArraySize() = %u (expected: 641)\r\n", LinkArraySize());
207+
CLR_Debug::Printf("LinkMRUArraySize() = %u (expected: 513)\r\n", LinkMRUArraySize());
208+
CLR_Debug::Printf("PayloadArraySize() = %u (expected: 512)\r\n", PayloadArraySize());
209+
210+
// Verify array base addresses don't overlap
211+
uintptr_t entries_start = (uintptr_t)m_entries;
212+
uintptr_t entries_end = entries_start + (LinkArraySize() * sizeof(Link));
213+
uintptr_t entriesMRU_start = (uintptr_t)m_entriesMRU;
214+
uintptr_t entriesMRU_end = entriesMRU_start + (LinkMRUArraySize() * sizeof(Link));
215+
uintptr_t payloads_start = (uintptr_t)m_payloads;
216+
uintptr_t payloads_end = payloads_start + (PayloadArraySize() * sizeof(Payload));
217+
218+
CLR_Debug::Printf(
219+
"m_entries: 0x%08X - 0x%08X (%u bytes)\r\n",
220+
(unsigned int)entries_start,
221+
(unsigned int)entries_end,
222+
(unsigned int)(entries_end - entries_start));
223+
CLR_Debug::Printf(
224+
"m_entriesMRU: 0x%08X - 0x%08X (%u bytes)\r\n",
225+
(unsigned int)entriesMRU_start,
226+
(unsigned int)entriesMRU_end,
227+
(unsigned int)(entriesMRU_end - entriesMRU_start));
228+
CLR_Debug::Printf(
229+
"m_payloads: 0x%08X - 0x%08X (%u bytes)\r\n",
230+
(unsigned int)payloads_start,
231+
(unsigned int)payloads_end,
232+
(unsigned int)(payloads_end - payloads_start));
233+
234+
// Check for overlaps
235+
if (entries_end > entriesMRU_start && entries_start < entriesMRU_end)
236+
{
237+
CLR_Debug::Printf("*** WARNING: m_entries and m_entriesMRU OVERLAP! ***\r\n");
238+
}
239+
if (entries_end > payloads_start && entries_start < payloads_end)
240+
{
241+
CLR_Debug::Printf("*** WARNING: m_entries and m_payloads OVERLAP! ***\r\n");
242+
}
243+
if (entriesMRU_end > payloads_start && entriesMRU_start < payloads_end)
244+
{
245+
CLR_Debug::Printf("*** WARNING: m_entriesMRU and m_payloads OVERLAP! ***\r\n");
246+
}
247+
248+
CLR_Debug::Printf("================================================================\r\n\r\n");
249+
250+
#endif
251+
199252
//
200253
// Link all the entries to themselves => no elements in the lists.
201254
//
@@ -211,6 +264,7 @@ void CLR_RT_EventCache::VirtualMethodTable::Initialize()
211264
// Link all the entries to the following one => all the elements are in the MRU list.
212265
//
213266
_ASSERTE(LinkMRUArraySize() < 0xFFFF);
267+
214268
for (index = 0; index < LinkMRUArraySize(); index++)
215269
{
216270
Link &lnk = m_entriesMRU[index];
@@ -301,14 +355,36 @@ bool CLR_RT_EventCache::VirtualMethodTable::FindVirtualMethod(
301355

302356
for (index = m_entries[indexHead].m_next;; index = m_entries[index].m_next)
303357
{
358+
#ifdef DEBUG &&_WIN64
359+
CLR_Debug::Printf(" Loop: index=%u, indexHead=%u\r\n", index, indexHead);
360+
#endif
361+
362+
// validate index before using it to prevent crashes from corrupted data
363+
if (index >= LinkArraySize())
364+
{
365+
// !! corrupted index detected !!
366+
// // repair the hash chain and treat as cache miss
367+
m_entries[indexHead].m_next = indexHead;
368+
m_entries[indexHead].m_prev = indexHead;
369+
370+
index = indexHead;
371+
}
372+
304373
if (index != indexHead)
305374
{
375+
_ASSERTE(index < PayloadArraySize());
376+
306377
Payload &res = m_payloads[index];
307378

308379
if (res.m_key.m_mdVirtual.data != mdVirtualData)
380+
{
309381
continue;
382+
}
383+
310384
if (res.m_key.m_cls.data != clsData)
385+
{
311386
continue;
387+
}
312388

313389
md = res.m_md;
314390

@@ -317,10 +393,20 @@ bool CLR_RT_EventCache::VirtualMethodTable::FindVirtualMethod(
317393
else
318394
{
319395
if (g_CLR_RT_TypeSystem.FindVirtualMethodDef(cls, mdVirtual, md) == false)
396+
{
320397
return false;
398+
}
321399

322400
index = GetNewEntry();
323401

402+
#ifdef DEBUG &&_WIN64
403+
CLR_Debug::Printf(" GetNewEntry returned: %u\r\n", index);
404+
#endif
405+
406+
// initialize the entry's links before use to prevent corruption
407+
m_entries[index].m_next = index;
408+
m_entries[index].m_prev = index;
409+
324410
Payload &res = m_payloads[index];
325411

326412
res.m_md = md;

0 commit comments

Comments
 (0)