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.
`SetupPathForFileName` is the "find a file beside my binary" helper: it is how
the driver path, the hwdbg test/script files, the exported
`hyperdbg_u_setup_path_for_filename` and (now) the PCI ID database are all
resolved. On Linux it was a stub that printed an error and returned FALSE, so
every one of those lookups failed. Implementing it also removes the blocker that
kept two neighbouring helpers stubbed, so all three are done here.
install-linux.cpp — SetupPathForFileName:
A call-for-call port of the Windows body. `readlink("/proc/self/exe")` is the
`GetModuleFileName(GetModuleHandle(NULL), ...)` counterpart, `strrchr(..., '/')`
replaces `strrchr(..., '\\')` to strip the program name, and `access(path, F_OK)`
replaces the `CreateFile(..., OPEN_EXISTING)` probe behind `CheckFileExists`.
Two things it does that the Windows version does not:
- BufferLength is honoured on every write. `readlink` truncates silently, so a
result that fills the buffer is rejected instead of being handed back as a
valid-looking path to the wrong file, and the directory + '/' + name + NUL
total is checked before appending. install.cpp gets the second half of this
from StringCbCat, but its GetModuleFileName truncation is unchecked.
- Windows-style separators in FileName are normalized to '/'. The names come
from shared headers and are spelled the Windows way —
HWDBG_TEST_READ_INSTANCE_INFO_PATH ("..\\..\\..\\..\\hwdbg\\...") and
PCI_ID_DATABASE_PATH ("constants\\pci.ids"). Normalizing here keeps those
constants shared and keeps the #ifdef out of every call site. Only the
appended part is rewritten, so a backslash inside a real Linux directory name
survives.
common.cpp — ListDirectory:
opendir/readdir with fnmatch for the wildcard, mirroring how FindFirstFileA
applies the pattern to the file name. No fnmatch flag is passed, so a leading
'.' is unremarkable and "." / ".." match a bare "*", as they do in the Win32
walk; a missing directory throws the same std::runtime_error as the
INVALID_HANDLE_VALUE branch. One deliberate difference: the result is sorted,
since readdir returns entries in file-system order and the caller (eval.cpp's
script-engine test harness) is easier to compare across runs when the order is
stable.
pci-id.cpp — GetVendorById:
The Linux branch now builds the database path with SetupPathForFileName and
calls the same GetVendorByIdStr as Windows. It had been left Windows-only
because wrapping just the two Win32 calls would have left strrchr looking for a
'\\' and silently resolving against the cwd; the normalization above removes
that objection. !pcitree and !pcicam resolve vendor and device names on Linux
whenever constants/pci.ids is deployed next to the binary.
Verified by extracting SetupPathForFileName and the Linux ListDirectory branch
into a standalone harness built with -fsanitize=address,undefined: 20 probes
covering the separator normalization, both CheckFileExists outcomes, the
exact-fit and one-byte-short buffer boundaries, a buffer too small for the
executable path itself, the NULL/zero-length arguments, and the four
ListDirectory cases (filtering, sorting, empty match, missing directory). All
pass with no sanitizer diagnostics. The three modified files were also compiled
with the project's own Linux CMake build. The Windows build is untouched: every
change is inside a non-Windows branch or in a file that is not in the Windows
project.
The ledger in hyperdbg/linux/PORTING_STATUS.md is updated accordingly;
GetConfigFilePath stays open, as it is the same walk behind a PWCHAR
out-parameter and remains blocked on the wide-char work.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
hyperdbg/linux/README.md tells contributors to build with `cmake .`, which
is an in-source build: it drops CMakeCache.txt, CMakeFiles/,
cmake_install.cmake and a generated Makefile next to the sources, in
hyperdbg/ and in every subproject directory. None of that was ignored, so
building on Linux left thirteen untracked entries in `git status`.
The Makefile rules are scoped to the directories that actually contain a
CMakeLists.txt rather than matching the name globally, so the hand-written
Makefiles under hyperdbg/linux/mock/ and hwdbg/sim/ stay tracked. Verified
with git check-ignore over both sets, and by re-running `cmake .` and
confirming the working tree comes back clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
The remaining std::stoi() call sites in the user-mode helpers throw on
malformed and out-of-range input. None of the callers catch, and there is
no top-level handler, so a bad command argument terminates the process
instead of being rejected with an error message.
common/common.cpp:
- IsNumber() tested against the character set "[0123456789]", so the two
square brackets were accepted as digits. It is used as the guard in
front of stoi() by several callers, which turns the mistake into a
crash: "[1]" passes validation and then throws std::invalid_argument.
Removed the brackets from the set.
- ValidateIP() called stoi() on every octet. Together with the above,
".connect [1].2.3.4 50000" aborted the process; so did an octet long
enough to overflow int, e.g. ".connect 99999999999.1.1.1". Replaced by
a strtoul() range check, which saturates instead of throwing.
- CheckAddressCanonicality() compared with a strict "<" against the start
of the higher-half canonical range, so the boundary address itself
(0xFFFF800000000000 at a 48-bit VA width) was reported as user-mode.
CheckAccessValidityAndSafety() would then probe a kernel address from
user mode rather than refusing it.
- ListDirectory() called FindNextFileA() before consuming the match that
FindFirstFileA() had already stored in FindData, so the first file of
every listing was silently dropped. Turned into a do/while.
- isxdigit()/isdigit()/isspace() were handed a plain char; passing a
negative value to the <ctype.h> functions is undefined behavior and
trips the MSVC debug CRT on any input with a byte above 0x7f. Cast to
UCHAR at the four call sites.
The same crash class at the call sites that use stoi() directly on user
input, all replaced with strtoul() so the existing range checks reject
the value instead of an exception escaping:
- connect.cpp (x2) and listen.cpp: TCP port validation.
- debug.cpp: serial baud rate.
Verified by extracting the affected functions from common.cpp and running
them under ASan/UBSan over a 316-probe corpus, before and after. The
baseline aborts with SIGABRT on three inputs; afterwards every probe
returns a defined result and exactly the seven intended lines differ.
The four modified files were also compiled with the project's own Linux
CMake build to confirm they build clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3C1DuhHtqK64eEkHjKCHM
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
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! :)
