Each forwarded raw-input event is written to g_raw_slots[head++ % kRawSlots] and
a WM_INPUT carrying that slot's ADDRESS is posted to the game, which reads it back
through hk_GetRawInputData. With only 64 slots, a burst that queues more than 64
WM_INPUTs before the game pumps could overwrite a slot before the game reads it,
so it would decode a newer event for a stale message. No memory unsafety (the
address stays in-bounds), but wrong event data under backlog.
Grow the ring to 512 (a few tens of KB) so realistic input rates can't lap it.
Deliberately not per-slot consume-tracking: that would permanently exhaust slots
and silently stop forwarding for a game that ignores WM_INPUT, whereas a large
ring always forwards and only risks a rare stale read under extreme backlog.
Also drops the "publish() synthetic-input timing" review item: verified it uses
GetTickCount64() (thread-safe), not ImGui state -- not a bug, no change needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The remove_* comments still described the superseded "disable -> drain -> destroy"
flow; the code keeps hooks alive (persistent) and re-enables on re-install. Updated
the comments to match, and corrected the XInput note (its detours return synthesized
state and never call the trampoline, so destroying its vector is safe -- unlike the
trampoline-calling present/MKB/focus-cursor hooks).
- hook_install_test: a fast, single-threaded contract test for hook_install.hpp --
install_inline creates the hook once and reuses the SAME trampoline across 50
install/remove cycles (never freed -> no stale-detour UAF), toggling enable/disable
cleanly. Fills the guard the removed (flaky, concurrency-bound) reproducer left, with
no threads so it can't flake on SafetyHook's enable/disable atomicity.
x64 23/23, x86 3/3.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The uncapped, input-polling mock_game_test storm (thousands of presents/s, now
also driving the input/focus/MKB hooks) drove out a family of install/remove races
the slow vsync'd mock had masked. Fixes (hook/src/hook_install.hpp + hook_guard.hpp):
- Persistent hooks. The old model created a hook on install and DESTROYED it on
remove (= {}), freeing the trampoline; a detour about to call it (.stdcall) then
hit freed memory -> 0xC0000005. drain() can't fully close that window (a thread
can be inside the detour but not past its Guard ctor). So hooks are now created
ONCE and only enable()/disable()d across install/remove cycles -- never destroyed
during the session -- so a stale detour always calls a live trampoline (disabled,
it just runs the original). Reused, so no churn and no leak. remove_* therefore
disable()s + drain()s but does not destroy; install guards check .enabled().
- Install race. create_inline() enables the hook before the result is move-assigned
into the global the detour reads; a call landing in the detour mid-assign reads a
torn hook -> AV. install_inline() creates StartDisabled, assigns, then enable()s.
- drain() Sleep(1)s BEFORE each zero-check, so a thread that entered the detour but
hasn't reached its Guard registers before we conclude zero.
- Focus: publish g_orig_proc before SetWindowLongPtr activates the subclass (and
subclass_proc falls back to DefWindowProc if null); and disable the focus-query
hooks in reverse install order, because GetForegroundWindow shares user32 code
with GetActiveWindow (keep GFW hooked until GAW is unhooked).
- disable()/enable() [[nodiscard]] results are handled (logged), not (void)-discarded.
Storm now survives on every backend across repeated runs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uncapping the mock game (next commit) turned mock_game_test's hook/unhook storm
into a real stress test (thousands of presents/s instead of tens), which reliably
crashed the game on remove (0xC0000005) for dx9/dx11/dx12. Two races the slow
vsync'd mock had masked:
1. Trampoline use-after-free. remove_*_hooks did `hook = {}` (destroy) BEFORE the
DetourGate drain. Destroying a SafetyHook InlineHook frees its trampoline
immediately, but an in-flight detour about to call the original via .stdcall()
(the trampoline) then used freed memory. Fix: disable() first (restores the
original bytes under thread suspension, but KEEPS the trampoline alive) -> drain
-> only then destroy. Applied to present/d3d9/opengl/vk/xinput/mkb/focus.
2. Entry-window race in DetourGate::drain(). It returned the instant the active
count read zero, but a thread can be inside the detour yet not have reached its
Guard constructor (the prologue is unguarded), so the count reads zero while a
detour is about to run -- and the freed state is then used. Fix: Sleep(1) BEFORE
each zero-check; with the hook disabled no new detour starts, so any
already-entered thread registers within that window. This alone fixed dx11 (the
highest present rate, ~11000/s, which hit the window every storm).
Audio is unaffected (it uses vtable swaps, which keep a real original pointer, not
a trampoline). Full suite 21/21, and the storm now survives on every backend.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The MKB subsystem covered message-loop (PostMessage) and polling
(GetAsyncKeyState/GetKeyboardState/GetCursorPos) games. Add the two remaining
read paths, both fed from the same synthesized state:
- DirectInput: vtable-swap IDirectInputDevice8::GetDeviceState (a COM method ->
vtable swap, not inline, per the x86 COM-prologue trap), reading the shared
vtable from a kept-alive probe device (the game made its devices before we
injected). Dispatch on cbData: 256 = keyboard BYTE[256] indexed by DIK
scan-code (map VK->DIK via MapVirtualKey VK_TO_VSC); DIMOUSESTATE = mouse
buttons + relative deltas from the forwarded cursor.
- Raw Input: games get no WM_INPUT while unfocused, so mkb_pump synthesizes it
(PostMessage WM_INPUT with lParam = one of our RAWINPUT slots) and the hooked
GetRawInputData serves that slot back (RID_HEADER + RID_INPUT). Covers keys +
buttons; relative raw-mouse movement isn't in the position-based MKB stream.
Both detours use the DetourGate safe-unhook guard, and the mock_game_test storm
exercises their install/remove. dinput_hook_test drives the real path end-to-end:
forward a key + mouse button through the ring, then a real DI keyboard/mouse
device's GetDeviceState returns them. The Raw Input round-trip is operator-
validated against a real game (too brittle to assert in-process). vtable_hook.hpp
factors the COM vtable-swap helper out for reuse.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Spamming a subsystem toggle (the "Mirror video" button) could crash the game:
remove_*_hooks freed a hook's shared D3D / Vulkan / IPC state immediately, while a
capture detour was still mid-flight on the game's render thread -> use-after-free.
Only the audio hooks had the safe-unhook drain; the video (Present/D3D9/D3D10/GL/
Vulkan) and XInput/focus/MKB hooks did not.
Test-first: mock_game_test now runs an aggressive hook/unhook storm -- a separate
thread thrashes every subsystem on/off while the game presents, across all backends.
It crashed gl + vk (0xC0000005) and failed dx9 capture-resume before the fix.
Fix (hook/src/hook_guard.hpp, DetourGate): each detour wraps its body in an RAII
active-count Guard; remove_* restores the hook first (so no new detour starts),
drains the in-flight detours to zero, and only then frees the shared state. Vulkan
is special-cased -- the game caches hk_vkQueuePresentKHR, so removal closes an
atomic capture gate (detours then pass through to the real present), drains, then
frees the read-back resources. Storm now passes on every backend.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Forward the host window's clicks and keystrokes into the injected game so guests
can drive menus / "Press Start" / text entry that a pad can't.
Protocol (v7->v8): new HookSubsys_Mkb and an SPSC MkbRing of MkbEvents in
SharedBlock (host produces, hook consumes); push/pop helpers.
Hook (hook/src/mkb_hook.cpp, new subsystem): a worker-loop pump drains the ring at
~5 ms and PostMessageW's the matching window messages (WM_KEY*/WM_CHAR, mouse
buttons, WM_MOUSEWHEEL) to the game's main window; it also inline-hooks user32
GetAsyncKeyState / GetKeyboardState / GetCursorPos (stdcall trampolines per the x86
rule) to report a synthesized state so polling games react too. Removing the
subsystem clears all synthesized keys (no stuck input).
Host: the Injection panel gets a "Mouse + keyboard forwarding" subsystem toggle
(opt-in, default off -- the toggle is the hook). host/src/inject/mkb_forward.cpp
reads ImGui IO each frame and forwards only when the host window is focused and
ImGui isn't capturing the event; keyboard always, mouse only while mirroring (clicks
+ wheel, not movement). Mouse coords are mapped through the letterbox to game-client
space (host/src/inject/mkb_map.hpp), accounting for WGC-of-decorated-window vs
hooked/borderless. RawInput/DirectInput games are out of scope for this version.
Verified: new mkb_ring_test + mkb_map_test pass; full build x64 + x86 clean; ctest
x64 9/9 and x86 3/3 green (no regression from the protocol bump). The subsystem is
opt-in, so it can't affect existing behavior unless enabled; the end-to-end
click-into-game path needs live Remote Play + a real game to confirm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
