Fix dispatcher crash/deadlock, instruction analyzer, cache equality, task leak, and redirection protection
- MainThreadDispatcher: guard DispatchHook with try/catch so exceptions never escape to native caller; drain and fault pending work on Dispose; synchronize Execute/ExecuteAsync/Dispose against race/dispose. - InstructionAnalyzer: require ModRM 0xEC for 0x83/0x81 sub-esp/rsp forms, rejecting unsafe RIP-relative or memory forms. - PatternScannerCache: implement value equality on CacheKey so repeated scans actually hit cache. - BackgroundTaskExecutor: add remote allocations to the free list immediately after VirtualAllocEx, before any write that could fail and leak. - redirect: capture and restore original page protection in Apply/Remove instead of leaving target RWX. - Regression tests for all six fixes. Tests: 198 passing, 4 integration/interactive skipped.
This commit is contained in:
@@ -128,6 +128,29 @@ public sealed class PatternScannerCache
|
||||
IntPtr End,
|
||||
IReadOnlyList<ProcessModule>? Modules = null) : IEquatable<CacheKey>
|
||||
{
|
||||
public bool Equals(CacheKey? other)
|
||||
{
|
||||
if (other is null)
|
||||
return false;
|
||||
if (Start != other.Start || End != other.End || Mask != other.Mask)
|
||||
return false;
|
||||
if (!Pattern.AsSpan().SequenceEqual(other.Pattern))
|
||||
return false;
|
||||
|
||||
if (Modules is null)
|
||||
return other.Modules is null;
|
||||
if (other.Modules is null || Modules.Count != other.Modules.Count)
|
||||
return false;
|
||||
|
||||
for (int i = 0; i < Modules.Count; i++)
|
||||
{
|
||||
if (Modules[i].BaseAddress != other.Modules[i].BaseAddress)
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
// Override GetHashCode to hash the contents, not references
|
||||
public override int GetHashCode()
|
||||
{
|
||||
|
||||
@@ -21,8 +21,10 @@ public sealed class MainThreadPump : IDisposable
|
||||
private readonly IntPtr _frameAddress;
|
||||
private readonly ConcurrentQueue<WorkItem> _queue = new();
|
||||
|
||||
private readonly object _gate = new();
|
||||
private Detour? _detour;
|
||||
private bool _installed;
|
||||
private bool _disposed;
|
||||
|
||||
/// <summary>
|
||||
/// Creates a pump that will hook the frame function at <paramref name="frameAddress"/>.
|
||||
@@ -52,14 +54,16 @@ public sealed class MainThreadPump : IDisposable
|
||||
/// </summary>
|
||||
public TResult Execute<TResult>(Func<TResult> work)
|
||||
{
|
||||
if (!_installed)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
"The main-thread pump is not installed. Call Install() first.");
|
||||
}
|
||||
|
||||
var tcs = new TaskCompletionSource<object?>();
|
||||
_queue.Enqueue(new WorkItem(() => work()!, tcs));
|
||||
lock (_gate)
|
||||
{
|
||||
if (_disposed)
|
||||
ThrowDisposed();
|
||||
if (!_installed)
|
||||
throw new InvalidOperationException("The main-thread pump is not installed. Call Install() first.");
|
||||
|
||||
_queue.Enqueue(new WorkItem(() => work()!, tcs));
|
||||
}
|
||||
|
||||
object? result = tcs.Task.GetAwaiter().GetResult();
|
||||
return (TResult)result!;
|
||||
@@ -70,45 +74,70 @@ public sealed class MainThreadPump : IDisposable
|
||||
/// </summary>
|
||||
public Task<TResult> ExecuteAsync<TResult>(Func<TResult> work)
|
||||
{
|
||||
if (!_installed)
|
||||
var tcs = new TaskCompletionSource<TResult>();
|
||||
lock (_gate)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
"The main-thread pump is not installed. Call Install() first.");
|
||||
if (_disposed)
|
||||
ThrowDisposed();
|
||||
if (!_installed)
|
||||
throw new InvalidOperationException("The main-thread pump is not installed. Call Install() first.");
|
||||
|
||||
object? Box() => work()!;
|
||||
_queue.Enqueue(new WorkItem(Box, r => tcs.SetResult((TResult)r!), ex => tcs.SetException(ex)));
|
||||
}
|
||||
|
||||
var tcs = new TaskCompletionSource<TResult>();
|
||||
object? Box() => work()!;
|
||||
_queue.Enqueue(new WorkItem(Box, r => tcs.SetResult((TResult)r!), ex => tcs.SetException(ex)));
|
||||
return tcs.Task;
|
||||
}
|
||||
|
||||
/// <summary>Removes the frame-function detour if it is installed.</summary>
|
||||
public void Dispose()
|
||||
{
|
||||
if (_installed && _detour is not null)
|
||||
lock (_gate)
|
||||
{
|
||||
_detour.Remove();
|
||||
if (_disposed)
|
||||
return;
|
||||
_disposed = true;
|
||||
|
||||
if (_installed && _detour is not null)
|
||||
_detour.Remove();
|
||||
_installed = false;
|
||||
}
|
||||
|
||||
// Fault any caller still blocked on queued work so Execute cannot hang forever.
|
||||
while (_queue.TryDequeue(out WorkItem? item))
|
||||
{
|
||||
item.SetException(new ObjectDisposedException(nameof(MainThreadPump)));
|
||||
}
|
||||
}
|
||||
|
||||
private static void ThrowDisposed()
|
||||
=> throw new ObjectDisposedException(nameof(MainThreadPump));
|
||||
|
||||
private int PumpHook()
|
||||
{
|
||||
while (_queue.TryDequeue(out WorkItem? item))
|
||||
try
|
||||
{
|
||||
try
|
||||
while (_queue.TryDequeue(out WorkItem? item))
|
||||
{
|
||||
object? result = item.Work();
|
||||
item.SetResult(result);
|
||||
try
|
||||
{
|
||||
object? result = item.Work();
|
||||
item.SetResult(result);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
item.SetException(ex);
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
item.SetException(ex);
|
||||
}
|
||||
}
|
||||
|
||||
// Call the original frame function so rendering/game logic continues.
|
||||
return _detour is null ? 0 : (int?)_detour.CallOriginal() ?? 0;
|
||||
// Call the original frame function so rendering/game logic continues.
|
||||
return _detour is null ? 0 : (int?)_detour.CallOriginal() ?? 0;
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Never let an exception escape back into the native frame caller.
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
|
||||
|
||||
@@ -298,6 +298,8 @@ public sealed class RemoteThreadExecutor
|
||||
$"Failed to allocate remote string memory: error {error}");
|
||||
}
|
||||
|
||||
allocations.Add(remote);
|
||||
|
||||
int written = _reader.WriteBytes(remote, buffer);
|
||||
if (written != buffer.Length)
|
||||
{
|
||||
@@ -305,7 +307,6 @@ public sealed class RemoteThreadExecutor
|
||||
$"Failed to write string bytes to the remote process (wrote {written} of {buffer.Length} bytes).");
|
||||
}
|
||||
|
||||
allocations.Add(remote);
|
||||
return (nuint)(nint)remote;
|
||||
}
|
||||
|
||||
@@ -351,6 +352,8 @@ public sealed class RemoteThreadExecutor
|
||||
$"Failed to allocate remote struct memory: error {error}");
|
||||
}
|
||||
|
||||
allocations.Add(remote);
|
||||
|
||||
int written = _reader.WriteBytes(remote, buffer);
|
||||
if (written != size)
|
||||
{
|
||||
@@ -358,7 +361,6 @@ public sealed class RemoteThreadExecutor
|
||||
$"Failed to write struct bytes to the remote process (wrote {written} of {size} bytes).");
|
||||
}
|
||||
|
||||
allocations.Add(remote);
|
||||
return (nuint)(nint)remote;
|
||||
}
|
||||
|
||||
|
||||
@@ -117,28 +117,39 @@ public sealed class Detour : IDisposable
|
||||
"Failed to write the detour trampoline into the target process.");
|
||||
}
|
||||
|
||||
// Make the target page writable if necessary, then write the detour jump.
|
||||
// Make the target page writable if necessary, then write the detour jump,
|
||||
// restoring the original protection regardless of success or failure.
|
||||
if (!NativeMethods.VirtualProtectEx(
|
||||
_memory.Handle,
|
||||
Target,
|
||||
preserveLength,
|
||||
MemoryProtectionType.ExecuteReadWrite,
|
||||
out _))
|
||||
out MemoryProtectionType oldProtect))
|
||||
{
|
||||
int error = Marshal.GetLastPInvokeError();
|
||||
throw new InvalidOperationException(
|
||||
$"Failed to change target memory protection: error {error}");
|
||||
}
|
||||
|
||||
written = _memory.WriteBytes(Target, hookJump);
|
||||
if (written != hookJump.Length)
|
||||
try
|
||||
{
|
||||
throw new InvalidOperationException("Failed to write detour jump to target.");
|
||||
}
|
||||
written = _memory.WriteBytes(Target, hookJump);
|
||||
if (written != hookJump.Length)
|
||||
throw new InvalidOperationException("Failed to write detour jump to target.");
|
||||
|
||||
Trampoline = trampoline;
|
||||
Original = Marshal.GetDelegateForFunctionPointer(Trampoline, Hook.GetType());
|
||||
IsApplied = true;
|
||||
Trampoline = trampoline;
|
||||
Original = Marshal.GetDelegateForFunctionPointer(Trampoline, Hook.GetType());
|
||||
IsApplied = true;
|
||||
}
|
||||
finally
|
||||
{
|
||||
NativeMethods.VirtualProtectEx(
|
||||
_memory.Handle,
|
||||
Target,
|
||||
preserveLength,
|
||||
oldProtect,
|
||||
out _);
|
||||
}
|
||||
}
|
||||
catch
|
||||
{
|
||||
@@ -164,9 +175,21 @@ public sealed class Detour : IDisposable
|
||||
Target,
|
||||
OverwrittenBytes.Length,
|
||||
MemoryProtectionType.ExecuteReadWrite,
|
||||
out _);
|
||||
out MemoryProtectionType oldProtect);
|
||||
|
||||
_memory.WriteBytes(Target, OverwrittenBytes);
|
||||
try
|
||||
{
|
||||
_memory.WriteBytes(Target, OverwrittenBytes);
|
||||
}
|
||||
finally
|
||||
{
|
||||
NativeMethods.VirtualProtectEx(
|
||||
_memory.Handle,
|
||||
Target,
|
||||
OverwrittenBytes.Length,
|
||||
oldProtect,
|
||||
out _);
|
||||
}
|
||||
}
|
||||
|
||||
if (Trampoline != IntPtr.Zero)
|
||||
|
||||
@@ -53,12 +53,12 @@ internal static class PrologueDecoder
|
||||
return i + 2;
|
||||
}
|
||||
|
||||
// sub r/m32/64, imm8.
|
||||
if (op == 0x83 && bytes.Length > i + 2)
|
||||
// sub esp/rsp, imm8 — register-direct ModRM 0xEC only.
|
||||
if (op == 0x83 && bytes.Length > i + 2 && bytes[i + 1] == 0xEC)
|
||||
return i + 3;
|
||||
|
||||
// sub r/m32/64, imm32.
|
||||
if (op == 0x81 && bytes.Length > i + 5)
|
||||
// sub esp/rsp, imm32 — register-direct ModRM 0xEC only.
|
||||
if (op == 0x81 && bytes.Length > i + 5 && bytes[i + 1] == 0xEC)
|
||||
return i + 6;
|
||||
|
||||
return -1;
|
||||
|
||||
Reference in New Issue
Block a user