PCI CAM configuration space is 256 bytes, offsets 0x00 to 0xff, but
CAM_CONFIG_SPACE_LENGTH is defined as 255, next to BUS_MAX_NUM and
DEV_MAX_NUM which legitimately are 255.
The buffer that holds a captured config space is sized from that constant:
BYTE ConfigSpaceAdditional[CAM_CONFIG_SPACE_LENGTH - sizeof(PORTABLE_PCI_CONFIG_SPACE_HEADER)];
giving 64 + 191 = 255 contiguous bytes. Every loop over it, however, is
written in terms of the real 256-byte size and steps in units that overshoot
the constant. ExtensionCommandPcidevinfo() fills the buffer with
for (UINT16 i = 0; i < CAM_CONFIG_SPACE_LENGTH; i += 4)
which runs i = 0, 4, ... 252 and writes a DWORD each time, so it writes 256
bytes into those 255 - one byte past the end of the config space, into the
first byte of MmioBarInfo[0].Is64Bit that follows it in PCI_DEV. The write
stays inside the enclosing struct, so this is a correctness bug rather than
memory corruption, but the last byte of config space is stored outside the
buffer that is meant to hold it. The two raw-dump loops (i += 16) likewise
cover 256 bytes and read the last one from beyond the buffer.
Define the constant as the length it is named for. The buffer becomes 64 +
192 = 256 bytes, the fill loop lands exactly on the end, and both dump loops
cover exactly 16 rows of 16 bytes. The existing
static_assert(sizeof(DEBUGGEE_PCIDEVINFO_REQUEST_RESPONSE_PACKET) < PacketChunkSize)
still holds with the one byte the packet grows by.
PciReadCam()/PciWriteCam() compare a byte offset against the same constant,
so the bound is switched to >= to keep meaning a valid offset now that the
constant is a length rather than the highest offset. Both comparisons are
still statically false because Offset is a BYTE; this only stops the check
from being wrong if that parameter is ever widened.
EptHookCreateHookPage() records BreakpointAddresses[0] and sets
CountOfBreakpoints to 1, but never fills in the matching
PreviousBytesOnBreakpointAddresses[0]. The entry is allocated with
PlatformMemAllocateZeroedNonPagedPool(), so that slot deterministically
holds 0x00.
Every later breakpoint on the same page does save its original byte in
EptHookUpdateHookPage(), and EptHookUnHookSingleAddressHiddenBreakpoint()
restores all slots the same way. Removing the first breakpoint while a
second one is still on the page therefore writes 0x00 into the fake page
instead of the instruction byte that was there, corrupting the fake-page
instruction stream. The 0x00 can also swallow the remaining 0xcc as a
ModRM byte, so the surviving breakpoint stops triggering.
Read the byte after the page contents are copied and before the 0xcc is
written over it. This also makes the existing comment in the unhooking
path correct, which already assumed the first entry holds a valid
previous byte.
Fixes#637
The kernel module now links and compiles, there are still a lot of empty
TODOs left, there are empty stubs for hypertrace, assembly files and
kdserial etc. these can be introduced one by one. My first step will be
loading the kernel module from the userspace cli.
Big step forward! :)
Created a lot of new platform wrappers for windows only functions
also removed hyperevade from the linux module build and added a empty
stub for the transparent functions they provide which can be filled in
later.
Logging.c and UnloadDll.c compile and link into HyperDbg.ko, along with
the two components that were waiting on the logging layer (BinarySearch,
OptimizationsExamples).
- new PlatformStr.{h,c}: PlatformVsnprintf/Sprintf/Strnlen replace
vsprintf_s/sprintf_s/strnlen_s; PlatformSprintf moved here out of
PlatformMem so both spellings return -1 on truncation
- BasicTypes.h: IRP/IO_STACK_LOCATION/IO_STATUS_BLOCK/UNICODE_STRING
grow the members shared code touches, plus the NT status and access
constants the notify path needs
- Environment.h: RTL_NUMBER_OF, _Analysis_assume_, ASSERT -> WARN_ON
- PlatformEvent: ExEventObjectType placeholder token
- HyperLogCallback.c stays out of the module: it defines the same
LogCallback* entry points as Logging.c (it is the per-DLL forwarding
shim on Windows), so in one link unit it is a duplicate symbol
Route Spinlock.c's three MSVC intrinsics through the platform
intrinsics layer instead of the Windows builtins:
_mm_pause -> CpuPause (already existed)
_interlockedbittestandset -> CpuInterlockedBitTestAndSet (new)
InterlockedCompareExchange-> CpuInterlockedCompareExchange (new, 32-bit)
Add the two new Cpu* wrappers to the kernel PlatformIntrinsics
{.h,.c} with win + linux arms (Linux uses __atomic_* builtins,
mirroring the existing 64-bit CAS), keeping the Cpu* wrappers the
single intrinsic boundary.
