From 58a4330a712fe27d3af6aee16817f9f1ad876343 Mon Sep 17 00:00:00 2001 From: Qin Li Date: Tue, 3 Dec 2019 15:23:10 -0800 Subject: [PATCH] fixed VirtualMemoryLogging::logRecords overflow (#308) when VirtualMemoryLogging::recordNumber increments from LONG_MAX, it became negative number, and the result of i % MaxRecords became a number from -127 to 0. When that happens we will ovewrite CRITICAL_SECTION virtual_critsec which are stored in bss right before logRecords with garbage data. Then most likely the process will have a GC hang with one or more GC threads stuck trying to enter or leave critical section. The fix is to ensure ULONG value are passed to modulo operation. --- src/coreclr/src/pal/src/map/virtual.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/src/pal/src/map/virtual.cpp b/src/coreclr/src/pal/src/map/virtual.cpp index 8530ea0..6902153 100644 --- a/src/coreclr/src/pal/src/map/virtual.cpp +++ b/src/coreclr/src/pal/src/map/virtual.cpp @@ -107,7 +107,7 @@ namespace VirtualMemoryLogging // An entry in the in-memory log struct LogRecord { - LONG RecordId; + ULONG RecordId; DWORD Operation; LPVOID CurrentThread; LPVOID RequestedAddress; @@ -118,14 +118,14 @@ namespace VirtualMemoryLogging }; // Maximum number of records in the in-memory log - const LONG MaxRecords = 128; + const ULONG MaxRecords = 128; // Buffer used to store the logged data volatile LogRecord logRecords[MaxRecords]; // Current record number. Use (recordNumber % MaxRecords) to determine // the current position in the circular buffer. - volatile LONG recordNumber = 0; + volatile ULONG recordNumber = 0; // Record an entry in the in-memory log void LogVaOperation( @@ -137,7 +137,7 @@ namespace VirtualMemoryLogging IN LPVOID returnedAddress, IN BOOL result) { - LONG i = InterlockedIncrement(&recordNumber) - 1; + ULONG i = (ULONG)InterlockedIncrement((LONG *)&recordNumber) - 1; LogRecord* curRec = (LogRecord*)&logRecords[i % MaxRecords]; curRec->RecordId = i; -- 2.7.4