Skip to content

Commit

Permalink
Harden the memory reading implementation
Browse files Browse the repository at this point in the history
Adapt to a sporadically observed problem in memory reading: On 2020-02-25 and 2020-03-04 observed issue caused the whole memory reading to fail with a `System.OverflowException` out of `ReadBytes`, in both cases called via `ReadActiveDictionaryEntriesFromDictionaryAddress`.
See the added image for an example of the error.

Improve the dictionary reading:
+ Avoiding invoking `ReadBytes` with an invalid argument
+ Avoid stalling the whole reading process when we happen to read garbage in a single dictionary.

For the report from 2020-03-04, see the discussion with McMahon on Discord.
  • Loading branch information
Viir committed Mar 10, 2020
1 parent b6eb860 commit edae247
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 9 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
29 changes: 20 additions & 9 deletions implement/read-memory-64-bit/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace read_memory_64_bit
{
class Program
{
static string AppVersionId => "2020-02-06";
static string AppVersionId => "2020-03-10";

static int Main(string[] args)
{
Expand Down Expand Up @@ -451,12 +451,12 @@ void logLine(string lineText)

if (readContent)
{
int bytesRead = 0;
UIntPtr bytesRead = UIntPtr.Zero;
var regionContentBuffer = new byte[(long)m.RegionSize];

WinApi.ReadProcessMemory(processHandle, regionBaseAddress, regionContentBuffer, regionContentBuffer.Length, ref bytesRead);
WinApi.ReadProcessMemory(processHandle, regionBaseAddress, regionContentBuffer, (UIntPtr)regionContentBuffer.LongLength, ref bytesRead);

if (bytesRead != regionContentBuffer.Length)
if (bytesRead.ToUInt64() != (ulong)regionContentBuffer.LongLength)
throw new Exception($"Failed to ReadProcessMemory at 0x{regionBaseAddress:X}: Only read " + bytesRead + " bytes.");

regionContent = regionContentBuffer;
Expand Down Expand Up @@ -876,6 +876,12 @@ PyDictEntry[] ReadActiveDictionaryEntriesFromDictionaryAddress(ulong dictionaryA

var numberOfSlots = (int)ma_mask + 1;

if (numberOfSlots < 0 || 10_000 < numberOfSlots)
{
// Avoid stalling the whole reading process when a single dictionary contains garbage.
return null;
}

var slotsMemorySize = numberOfSlots * 8 * 3;

var slotsMemory = memoryReader.ReadBytes(ma_table, slotsMemorySize);
Expand Down Expand Up @@ -1189,18 +1195,23 @@ public byte[] ReadBytes(ulong startAddress, int length)
{
var buffer = new byte[length];

int numberOfBytesRead = 0;
UIntPtr numberOfBytesReadAsPtr = UIntPtr.Zero;

if (!WinApi.ReadProcessMemory(processHandle, startAddress, buffer, buffer.Length, ref numberOfBytesRead))
if (!WinApi.ReadProcessMemory(processHandle, startAddress, buffer, (UIntPtr)buffer.LongLength, ref numberOfBytesReadAsPtr))
return null;

var numberOfBytesRead = numberOfBytesReadAsPtr.ToUInt64();

if (numberOfBytesRead == 0)
return null;

if (numberOfBytesRead == buffer.Length)
if (int.MaxValue < numberOfBytesRead)
return null;

if (numberOfBytesRead == (ulong)buffer.LongLength)
return buffer;

return buffer.AsSpan(0, numberOfBytesRead).ToArray();
return buffer.AsSpan(0, (int)numberOfBytesRead).ToArray();
}
}

Expand All @@ -1213,7 +1224,7 @@ static public class WinApi
static public extern int VirtualQueryEx(IntPtr hProcess, IntPtr lpAddress, out MEMORY_BASIC_INFORMATION64 lpBuffer, uint dwLength);

[DllImport("kernel32.dll")]
static public extern bool ReadProcessMemory(IntPtr hProcess, ulong lpBaseAddress, byte[] lpBuffer, int dwSize, ref int lpNumberOfBytesRead);
static public extern bool ReadProcessMemory(IntPtr hProcess, ulong lpBaseAddress, byte[] lpBuffer, UIntPtr nSize, ref UIntPtr lpNumberOfBytesRead);

[DllImport("kernel32.dll", SetLastError = true)]
static public extern bool CloseHandle(IntPtr hHandle);
Expand Down

0 comments on commit edae247

Please sign in to comment.