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

Commit

Permalink
fixed VirtualMemoryLogging::logRecords overflow
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tqinli committed Nov 21, 2019
1 parent a9f3fc1 commit 8281f09
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/pal/src/map/virtual.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -137,7 +137,9 @@ namespace VirtualMemoryLogging
IN LPVOID returnedAddress,
IN BOOL result)
{
LONG i = InterlockedIncrement(&recordNumber) - 1;
ULONG i = InterlockedIncrement((LONG *)&recordNumber) - 1;

// Converting to ULONG before modulo to ensure result is positive
LogRecord* curRec = (LogRecord*)&logRecords[i % MaxRecords];

curRec->RecordId = i;
Expand Down

0 comments on commit 8281f09

Please sign in to comment.