From fc1877b3688c2882db6234f85a82520963055e3b Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:51:48 +0200 Subject: [PATCH 01/12] fuzz: coverage-guided fuzzer for the syscall emulation layer Drives Machine::system_call() directly on a CoW fork, so no VM entry is needed and the ~112 handlers in linux/system_calls.cpp run under ASan+UBSan at ~8k exec/s. Input is a record stream: POKE writes fuzzer-chosen bytes into guest memory, CALL sets the six argument registers and dispatches a syscall number. Both are needed -- most handlers read structs (iovec, sockaddr, msghdr, pollfd, timespec, paths) out of the guest, so the fuzzer has to control the pointer and the bytes it points at. Pointer arguments are drawn from a table of page boundaries, end-of-mapping addresses, unmapped holes and non-canonical values; uniformly random pointers just bounce off copy_from_guest() and coverage stalls. MachineException escaping a handler is expected and ignored -- that is how the layer rejects bad input. Sanitizer reports are findings. Any other std::exception is reported once per unique type+message, since an embedder catching MachineException would let it through. Sandboxed: guest paths are rewritten to two files in a private tmpdir, connect/bind/listen/accept are denied, guest-created fds are forced O_NONBLOCK, and clock_nanosleep's timespec is value-clamped so its marshalling is still covered without sleeping. blocked_syscall() refuses a handful that would block a worker or escape the sandbox; none of those has a handler today. Also makes -fuse-ld=lld conditional, and ignores .build-* dirs. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 1 + fuzz/CMakeLists.txt | 33 +- fuzz/gen_syscall_seeds.py | 265 ++++++++++++ fuzz/guest/syscall_fuzz_guest.c | 29 ++ fuzz/syscall_fuzz.cpp | 695 ++++++++++++++++++++++++++++++++ fuzz/syscall_fuzzer.sh | 37 ++ fuzz/triage_syscall_crashes.sh | 65 +++ 7 files changed, 1117 insertions(+), 8 deletions(-) create mode 100644 fuzz/gen_syscall_seeds.py create mode 100644 fuzz/guest/syscall_fuzz_guest.c create mode 100644 fuzz/syscall_fuzz.cpp create mode 100755 fuzz/syscall_fuzzer.sh create mode 100755 fuzz/triage_syscall_crashes.sh diff --git a/.gitignore b/.gitignore index 53b7add6..a0ab0cf8 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ **/build-* **/build **/.build +**/.build-* .vscode/ guest/interrupts guest/guest.elf diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index ba68086f..a96083e5 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -12,17 +12,34 @@ if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang") message(FATAL_ERROR "libfuzzer is part of the Clang compiler suite.") endif() -set(SOURCES - fuzz.cpp -) +find_program(LLD_LINKER NAMES lld ld.lld) -function(add_fuzzer NAME MODE) - add_executable(${NAME} ${SOURCES}) +function(add_fuzzer NAME SOURCE MODE) + add_executable(${NAME} ${SOURCE}) target_link_libraries(${NAME} PUBLIC tinykvm) - set_target_properties(${NAME} PROPERTIES CXX_STANDARD 17) + set_target_properties(${NAME} PROPERTIES CXX_STANDARD 20) target_link_libraries(${NAME} PUBLIC "-fsanitize=${FUZZER_MODE},fuzzer") - target_link_libraries(${NAME} PUBLIC "-fuse-ld=lld") + if (LLD_LINKER) + target_link_libraries(${NAME} PUBLIC "-fuse-ld=lld") + endif() target_compile_definitions(${NAME} PRIVATE ${MODE}=1) endfunction() -add_fuzzer(elffuzzer FUZZ_ELF) +add_fuzzer(elffuzzer fuzz.cpp FUZZ_ELF) +add_fuzzer(syscallfuzzer syscall_fuzz.cpp FUZZ_SYSCALLS) + +# The syscall fuzzer needs a real, fully-initialized master VM to fork from, +# so it loads a small static guest ELF built here. It is an ordinary Linux +# userspace binary -- not sanitized, not linked against the library. +find_program(GUEST_CC NAMES gcc cc REQUIRED) +set(GUEST_ELF "${CMAKE_CURRENT_BINARY_DIR}/syscall_fuzz_guest.elf") +add_custom_command( + OUTPUT ${GUEST_ELF} + COMMAND ${GUEST_CC} -O2 -static -std=c11 + -o ${GUEST_ELF} ${CMAKE_CURRENT_SOURCE_DIR}/guest/syscall_fuzz_guest.c + DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/guest/syscall_fuzz_guest.c + COMMENT "Building syscall fuzzer guest ELF" +) +add_custom_target(syscall_fuzz_guest DEPENDS ${GUEST_ELF}) +add_dependencies(syscallfuzzer syscall_fuzz_guest) +target_compile_definitions(syscallfuzzer PRIVATE FUZZ_GUEST_PATH="${GUEST_ELF}") diff --git a/fuzz/gen_syscall_seeds.py b/fuzz/gen_syscall_seeds.py new file mode 100644 index 00000000..cf4a71d8 --- /dev/null +++ b/fuzz/gen_syscall_seeds.py @@ -0,0 +1,265 @@ +#!/usr/bin/env python3 +"""Generate a seed corpus for fuzz/syscall_fuzz.cpp. + +libFuzzer can discover the input grammar on its own, but it would burn a long +time rediscovering the ~112 valid syscall numbers by chance. These seeds hand +it one plausible call per syscall number, plus a few multi-step sequences +(open-then-use) that single-record mutation cannot reach on its own. + +Grammar (must stay in sync with syscall_fuzz.cpp): + record := op:u8 + (op & 3) == 0 -> POKE sel:u8 off:i16 len:u8 bytes[len] + otherwise -> CALL sysno:u16 arg[6] + arg := kind:u8 payload + kind&3 == 0 -> u8 + kind&3 == 1 -> u64 + kind&3 == 2 -> sel:u8 off:i16 + kind&3 == 3 -> sel:u8 (index into INTERESTING) +""" +import os +import struct +import sys + +# Address-table indices, in the order build_address_table() appends them. +A_ZERO = 0 +A_SCRATCH = 8 +A_SCRATCH_8 = 9 +A_SCRATCH_64 = 10 +A_PAGE_M4 = 11 +A_PAGE_M1 = 12 +A_PAGE = 13 +A_SCRATCH_LAST = 16 +A_SMALL = 18 +A_MAX_M8 = 20 +A_TABLE_LEN = 27 + +# INTERESTING[] indices for the values we care about in seeds. +I_ZERO = 0 +I_ONE = 1 +I_NEG1 = 19 # uint64_t(-1) +I_VFD = 29 # FileDescriptors::VFD_START (0x1000) +I_VFD1 = 30 +I_VFD2 = 31 + +OP_POKE = 0x00 +OP_CALL = 0x01 + + +def arg_u8(v): + return bytes([0x00, v & 0xFF]) + + +def arg_u64(v): + return bytes([0x01]) + struct.pack(" 1 else "seeds" + os.makedirs(outdir, exist_ok=True) + seeds = generic_seeds() + sequence_seeds() + for name, data in seeds: + with open(os.path.join(outdir, name), "wb") as f: + f.write(data) + print("wrote %d seeds to %s" % (len(seeds), outdir)) + + +if __name__ == "__main__": + main() diff --git a/fuzz/guest/syscall_fuzz_guest.c b/fuzz/guest/syscall_fuzz_guest.c new file mode 100644 index 00000000..5b4af7c7 --- /dev/null +++ b/fuzz/guest/syscall_fuzz_guest.c @@ -0,0 +1,29 @@ +/* Guest program for the syscall fuzzer. + * + * Its only job is to produce a realistic, fully-initialized master VM: + * a loaded ELF with a stack, a heap, and a large writable data-segment + * region ("scratch") that the harness hands to the fuzzer as a pool of + * known-valid guest addresses. The fuzzer then drives syscalls directly + * against forks of this master, so nothing here needs to issue syscalls + * itself. + * + * scratch must be in .data (initialized) so the pages are actually + * mapped writable by the ELF loader, and must not be optimized away. + */ + +#define SCRATCH_SIZE (128 * 1024) + +volatile unsigned char scratch[SCRATCH_SIZE] = { 1 }; + +/* A second, smaller region so the harness has two disjoint mapped areas. */ +volatile unsigned char scratch_small[8192] = { 1 }; + +int main(void) +{ + /* Touch every page so the loader/CoW machinery has them present. */ + for (unsigned i = 0; i < SCRATCH_SIZE; i += 4096) + scratch[i] = (unsigned char)(i >> 12); + for (unsigned i = 0; i < sizeof(scratch_small); i += 4096) + scratch_small[i] = (unsigned char)(i >> 12); + return 0; +} diff --git a/fuzz/syscall_fuzz.cpp b/fuzz/syscall_fuzz.cpp new file mode 100644 index 00000000..adc8b3b0 --- /dev/null +++ b/fuzz/syscall_fuzz.cpp @@ -0,0 +1,695 @@ +/** + * Coverage-guided fuzzer for the host-side Linux syscall emulation layer + * (lib/tinykvm/linux/system_calls.cpp and friends). + * + * Threat model: the guest is untrusted. Its only channel into the host + * process is the syscall trampoline, which lands in Machine::system_call() + * with six fully guest-controlled 64-bit register arguments, several of + * which are pointers into guest memory whose *contents* the guest also + * controls. This harness reproduces exactly that: it drives + * Machine::system_call() directly on a forked VM, so no VM entry is + * needed and the interesting code -- the ~112 hand-written handlers -- + * runs under ASan/UBSan at native speed. + * + * Input grammar (little-endian, truncation-tolerant): + * + * record := op:u8 payload + * (op & 3) == 0 -> POKE sel:u8 off:i16 len:u8 bytes[len] + * otherwise -> CALL sysno:u16 arg[6] + * arg := kind:u8 payload + * kind & 3 == 0 -> u8 (small ints: fds, flags, counts) + * kind & 3 == 1 -> u64 (fully arbitrary) + * kind & 3 == 2 -> sel:u8 off:i16 (guest pointer from the address table) + * kind & 3 == 3 -> sel:u8 (interesting constant) + * + * POKE writes fuzzer-chosen bytes into guest memory, which is what makes + * the pointer arguments worth anything: most handlers read structs + * (iovec, sockaddr, pollfd, timespec, path strings) out of the guest. + * The address table is deliberately stocked with page boundaries, + * end-of-mapping addresses and unmapped holes, because uniformly random + * 64-bit pointers just bounce off copy_from_guest() and coverage stalls. + * + * Environment: + * TINYKVM_FUZZ_STRICT=1 treat any non-MachineException escaping a + * handler as a finding (abort). Off by default so + * a single easily-reached std::out_of_range does + * not wall off the rest of the search space. + * TINYKVM_FUZZ_UNSAFE=1 install the "unsafe" syscall set. + * TINYKVM_FUZZ_VERBOSE=1 trace every call (debugging the harness only). + */ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "helpers.cpp" + +using namespace tinykvm; + +static constexpr uint64_t MAX_MEMORY = 32ull << 20; /* 32MB */ +static constexpr uint64_t MAX_COW_MEM = 8ull << 20; /* 8MB per fork */ +static constexpr size_t MAX_RECORDS = 24; + +/* NB: deliberately no __asan_on_error()/abort() hook here (unlike fuzz.cpp). + Aborting from the callback runs *before* the sanitizer prints its report, + which loses the stack trace. Use ASAN_OPTIONS=abort_on_error=1 if a + coredump is wanted. */ + +static Machine* g_master = nullptr; +static Machine* g_fork = nullptr; +static bool g_strict = false; +static bool g_verbose = false; + +static std::string g_sandbox_dir; +static std::string g_sandbox_ro; +static std::string g_sandbox_rw; + +static std::vector g_addrs; + +/* ------------------------------------------------------------------ */ +/* Syscalls we refuse to issue. */ +/* */ +/* Only two reasons qualify: the handler can block the host for a */ +/* guest-controlled duration (which shows up as a fuzzer timeout and */ +/* drowns out real findings), or it has a side effect outside the */ +/* sandbox. Everything else stays enabled -- including the whole */ +/* mmap/mprotect/thread/fd surface, which is the interesting part. */ +/* ------------------------------------------------------------------ */ +static bool blocked_syscall(unsigned no) +{ + switch (no) { + /* Blocking waits. None of these currently has a handler -- they fall + through to the unhandled path and return -ENOSYS -- so refusing them + costs no coverage today and guards against a future handler that would + block a worker. accept4 and clock_nanosleep *do* have handlers and are + deliberately NOT here: both are made safe by wrappers instead, so their + bodies still get covered (see install_nonblocking_wrappers). */ + case 23: /* select */ + case 34: /* pause */ + case 61: /* wait4 */ + case 128: /* rt_sigtimedwait */ + case 247: /* waitid */ + case 270: /* pselect6 */ + case 43: /* accept */ + /* Actually allocates disk blocks. */ + case 285: /* fallocate */ + /* Process-level side effects. Not installed today; refuse anyway so a + future handler cannot surprise a running campaign. */ + case 57: /* fork */ + case 58: /* vfork */ + case 59: /* execve */ + case 101: /* ptrace */ + case 165: /* mount */ + case 166: /* umount2 */ + case 169: /* reboot */ + case 322: /* execveat */ + return true; + default: + return false; + } +} + +/* ------------------------------------------------------------------ */ +/* Make guest-created fds non-blocking. */ +/* */ +/* read()/recvfrom() on a guest-created pipe or datagram socket would */ +/* otherwise block forever. Rather than blocklisting the read family */ +/* (which is prime attack surface) we wrap the four handlers that mint */ +/* fds and set O_NONBLOCK on the real fd afterwards. The original */ +/* handler still runs, so its coverage is unaffected. */ +/* ------------------------------------------------------------------ */ +static Machine::syscall_t g_orig_socket; +static Machine::syscall_t g_orig_pipe2; +static Machine::syscall_t g_orig_socketpair; +static Machine::syscall_t g_orig_eventfd2; + +static void set_nonblocking_vfd(Machine& m, long vfd) +{ + if (vfd < FileDescriptors::VFD_START) + return; + try { + const int fd = m.fds().translate(int(vfd)); + if (fd > 2) + fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); + } catch (...) { + } +} + +/* Sweep the whole managed-vfd range rather than the socket-pair list: a + handler can register both pipe ends and only afterwards fail to write the + fds back to the guest, in which case the pair was never recorded but the + real fds exist and would still block a later read(). */ +static void nonblock_all_managed_fds(Machine& m) +{ + try { + auto& fds = m.fds(); + const int last = FileDescriptors::VFD_START + int(fds.get_max_files()) + 8; + for (int vfd = FileDescriptors::VFD_START; vfd < last; vfd++) { + const auto entry = fds.entry_for_vfd(vfd); + if (!entry.has_value() || (*entry) == nullptr) + continue; + const int fd = (*entry)->real_fd; + if (fd > 2) + fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); + } + } catch (...) { + } +} + +/* clock_nanosleep passes the guest's timespec straight to the real + clock_nanosleep, so a guest can pin the host thread for years (see + docs/syscall-fuzzing.md -- a real issue, not a harness one). Rather than + refuse the syscall and lose all coverage of it, clamp the *values* in guest + memory first and then run the original handler unmodified. The fuzzer still + chooses the pointer, so the copy_from_guest/copy_to_guest edges -- the part + worth fuzzing -- are all still reachable; only the duration is bounded. */ +static Machine::syscall_t g_orig_clock_nanosleep; + +static void install_clock_nanosleep_wrapper() +{ + g_orig_clock_nanosleep = Machine::get_syscall_handler(230); + if (g_orig_clock_nanosleep == nullptr) + return; + + Machine::install_syscall_handler(230, [](vCPU& cpu) { + const uint64_t g_req = cpu.registers().sysarg(2); + if (g_req != 0x0) { + try { + struct timespec ts {}; + cpu.machine().copy_from_guest(&ts, g_req, sizeof(ts)); + if (ts.tv_sec != 0 || ts.tv_nsec > 1000000L) { + ts.tv_sec = 0; + ts.tv_nsec = 1000L; /* 1us */ + cpu.machine().copy_to_guest(g_req, &ts, sizeof(ts)); + } + } catch (const std::exception&) { + /* Unreadable/unwritable pointer: leave it alone so the handler + under test gets to reject it itself. */ + } + } + g_orig_clock_nanosleep(cpu); + }); +} + +static void install_nonblocking_wrappers() +{ + g_orig_socket = Machine::get_syscall_handler(41); /* socket */ + g_orig_pipe2 = Machine::get_syscall_handler(293); /* pipe2 */ + g_orig_socketpair = Machine::get_syscall_handler(53); /* socketpair */ + g_orig_eventfd2 = Machine::get_syscall_handler(290); /* eventfd2 */ + + install_clock_nanosleep_wrapper(); + + /* NB: the post-step must run even when the original handler throws. A + handler can register both ends of a pipe and only then fail to write the + fds back to a bad guest address -- the fds exist, so a later read() on + one would block forever if it were left blocking. */ + if (g_orig_socket) { + Machine::install_syscall_handler(41, [](vCPU& cpu) { + try { + g_orig_socket(cpu); + } catch (...) { + nonblock_all_managed_fds(cpu.machine()); + throw; + } + set_nonblocking_vfd(cpu.machine(), long(cpu.registers().sysret())); + }); + } + if (g_orig_eventfd2) { + Machine::install_syscall_handler(290, [](vCPU& cpu) { + try { + g_orig_eventfd2(cpu); + } catch (...) { + nonblock_all_managed_fds(cpu.machine()); + throw; + } + set_nonblocking_vfd(cpu.machine(), long(cpu.registers().sysret())); + nonblock_all_managed_fds(cpu.machine()); + }); + } + if (g_orig_pipe2) { + Machine::install_syscall_handler(293, [](vCPU& cpu) { + try { + g_orig_pipe2(cpu); + } catch (...) { + nonblock_all_managed_fds(cpu.machine()); + throw; + } + nonblock_all_managed_fds(cpu.machine()); + }); + } + if (g_orig_socketpair) { + Machine::install_syscall_handler(53, [](vCPU& cpu) { + try { + g_orig_socketpair(cpu); + } catch (...) { + nonblock_all_managed_fds(cpu.machine()); + throw; + } + nonblock_all_managed_fds(cpu.machine()); + }); + } +} + +/* ------------------------------------------------------------------ */ +/* Sandbox policy. */ +/* */ +/* The path callbacks rewrite *every* requested path to one of two */ +/* files inside a private directory. That exercises the full open -> */ +/* manage -> translate -> read/write/stat/close path with no way to */ +/* reach anything else on the filesystem. Sockets are allowed to be */ +/* created (the fd bookkeeping is what we want to test) but never */ +/* connected or bound, so there is no egress. */ +/* ------------------------------------------------------------------ */ +static void install_sandbox_policy(Machine& m) +{ + auto& fds = m.fds(); + fds.set_open_readable_callback([](std::string& path) { + path = g_sandbox_ro; + return true; + }); + fds.set_open_writable_callback([](std::string& path) { + path = g_sandbox_rw; + return true; + }); + fds.set_resolve_symlink_callback([](std::string&) { return false; }); + fds.set_current_working_directory(g_sandbox_dir); + + /* No network egress. */ + fds.connect_socket_callback = [](int, struct sockaddr_storage&) { return false; }; + fds.bind_socket_callback = [](int, struct sockaddr_storage&) { return false; }; + fds.listening_socket_callback = [](int, int) { return false; }; + fds.accept_callback = [](int, int, int) { return false; }; + + /* poll()/ppoll() honour a guest-supplied timeout verbatim on a forked + VM, so let the guest reach the argument marshalling (the part that + parses the guest pollfd array) but never the blocking poll() itself. */ + fds.poll_callback = [](struct pollfd*, unsigned, int) { return false; }; + + fds.set_max_files(64); +} + +/* ------------------------------------------------------------------ */ + +static std::vector load_file(const std::string& path) +{ + FILE* f = fopen(path.c_str(), "rb"); + if (f == nullptr) { + fprintf(stderr, "syscall_fuzz: cannot open guest ELF '%s'\n", path.c_str()); + exit(1); + } + fseek(f, 0, SEEK_END); + const long size = ftell(f); + fseek(f, 0, SEEK_SET); + std::vector out; + out.resize(size_t(size)); + if (fread(out.data(), 1, out.size(), f) != out.size()) { + fclose(f); + fprintf(stderr, "syscall_fuzz: short read on guest ELF\n"); + exit(1); + } + fclose(f); + return out; +} + +static void remove_sandbox() +{ + if (g_sandbox_dir.empty()) + return; + unlink(g_sandbox_ro.c_str()); + unlink(g_sandbox_rw.c_str()); + rmdir(g_sandbox_dir.c_str()); +} + +/* atexit() is not enough: libFuzzer's fork-mode children are frequently killed + rather than allowed to exit, and a long campaign spawns thousands of them. + Sweep the sandbox directories whose owning pid is gone, so the set is + self-healing rather than a slow /tmp leak. */ +static void reap_stale_sandboxes(const std::string& base, const std::string& prefix) +{ + DIR* dir = opendir(base.c_str()); + if (dir == nullptr) + return; + while (const struct dirent* ent = readdir(dir)) { + const std::string name = ent->d_name; + if (name.compare(0, prefix.size(), prefix) != 0) + continue; + const std::string pid_part = name.substr(prefix.size()); + if (pid_part.empty() || pid_part.find_first_not_of("0123456789") != std::string::npos) + continue; + const pid_t pid = pid_t(strtol(pid_part.c_str(), nullptr, 10)); + if (pid <= 0 || pid == getpid()) + continue; + if (kill(pid, 0) == 0 || errno != ESRCH) + continue; /* still alive, or we cannot tell */ + const std::string path = base + "/" + name; + unlink((path + "/readable").c_str()); + unlink((path + "/writable").c_str()); + rmdir(path.c_str()); + } + closedir(dir); +} + +static void make_sandbox() +{ + static const std::string PREFIX = "tinykvm-syscall-fuzz-"; + const char* env_base = getenv("TMPDIR"); + const std::string base = env_base ? env_base : "/tmp"; + + reap_stale_sandboxes(base, PREFIX); + + g_sandbox_dir = base + "/" + PREFIX + std::to_string(getpid()); + mkdir(g_sandbox_dir.c_str(), 0700); + + g_sandbox_ro = g_sandbox_dir + "/readable"; + g_sandbox_rw = g_sandbox_dir + "/writable"; + + /* Give the readable file some content so read()/mmap()/sendfile() paths + have something to move around. */ + if (FILE* f = fopen(g_sandbox_ro.c_str(), "wb")) { + std::vector data(64 * 1024); + for (size_t i = 0; i < data.size(); i++) + data[i] = uint8_t(i * 31 + 7); + fwrite(data.data(), 1, data.size(), f); + fclose(f); + } + if (FILE* f = fopen(g_sandbox_rw.c_str(), "wb")) { + fputs("writable\n", f); + fclose(f); + } +} + +/* Guest addresses worth pointing a syscall argument at. Ordinary valid + pointers get us into the handler bodies; the boundary and hole entries + are what actually stress the page-walking helpers. */ +static void build_address_table(Machine& m, uint64_t scratch, uint64_t scratch_small) +{ + const uint64_t page = 4096; + auto add = [](uint64_t a) { g_addrs.push_back(a); }; + + add(0x0); + add(0x1000); + add(m.kernel_end_address()); + add(m.start_address()); + add(m.stack_address()); + add(m.stack_address() - page); + add(m.heap_address()); + add(m.mmap_current()); + + /* The bread-and-butter valid region. */ + add(scratch); + add(scratch + 8); + add(scratch + 64); + /* Straddling a page boundary: the struct/buffer spans two guest pages + which, after CoW, are very often not physically contiguous. */ + add(((scratch + page - 1) & ~(page - 1)) - 4); + add(((scratch + page - 1) & ~(page - 1)) - 1); + add(((scratch + page - 1) & ~(page - 1))); + add(scratch + page * 3 - 8); + add(scratch + 0x1F000); /* last page of the 128K region */ + add(scratch + 0x20000 - 8); /* last 8 bytes: reads past the end fall off */ + add(scratch + 0x20000); /* one past the end */ + + add(scratch_small); + add(scratch_small + 0x2000 - 4); + + /* Out of bounds / unmapped / non-canonical. */ + add(m.max_address() - 8); + add(m.max_address()); + add(m.max_address() + page); + add(0x7FFFFFFFF000ull); + add(0xFFFF800000000000ull); + add(~uint64_t(0) - 0xFFF); + add(~uint64_t(0)); +} + +static const uint64_t INTERESTING[] = { + 0, 1, 2, 3, 8, 0x10, 0x40, 0xFF, 0x100, 0x1000, 0x2000, 4095, 4096, 8192, + 0xFFFF, 0x10000, 0x100000, 0x1000000, 64ull << 20, + uint64_t(-1), uint64_t(-2), uint64_t(-4095), uint64_t(int64_t(INT32_MIN)), + 0x7FFFFFFF, 0x80000000, 0xFFFFFFFF, 0x100000000ull, 0x7FFFFFFFFFFFFFFFull, + 0x8000000000000000ull, + FileDescriptors::VFD_START, FileDescriptors::VFD_START + 1, + FileDescriptors::VFD_START + 2, 0x100, 0x1001, +}; +static constexpr size_t INTERESTING_COUNT = sizeof(INTERESTING) / sizeof(INTERESTING[0]); + +/* ------------------------------------------------------------------ */ + +struct Reader { + const uint8_t* p; + const uint8_t* end; + + bool empty() const { return p >= end; } + size_t left() const { return size_t(end - p); } + + uint8_t u8() { return p < end ? *p++ : 0; } + uint16_t u16() { const uint16_t a = u8(); return uint16_t(a | (uint16_t(u8()) << 8)); } + int16_t i16() { return int16_t(u16()); } + uint64_t u64() + { + uint64_t v = 0; + for (int i = 0; i < 8; i++) + v |= uint64_t(u8()) << (i * 8); + return v; + } +}; + +static uint64_t table_address(Reader& r) +{ + const uint64_t base = g_addrs[r.u8() % g_addrs.size()]; + return base + uint64_t(int64_t(r.i16())); +} + +static uint64_t decode_arg(Reader& r) +{ + const uint8_t kind = r.u8(); + switch (kind & 3) { + case 0: return r.u8(); + case 1: return r.u64(); + case 2: return table_address(r); + default: return INTERESTING[r.u8() % INTERESTING_COUNT]; + } +} + +/* Report a non-MachineException exactly once per (type, message) pair so a + non-strict campaign still tells us what it found without stopping. */ +static void note_foreign_exception(const char* type, const char* what) +{ + static std::set seen; + std::string key = std::string(type) + ": " + what; + if (seen.insert(key).second) + fprintf(stderr, "syscall_fuzz: foreign exception escaped a handler: %s\n", key.c_str()); +} + +static void do_call(Reader& r) +{ + const unsigned sysno = r.u16() & 0x3FF; + + uint64_t args[6]; + for (unsigned i = 0; i < 6; i++) + args[i] = decode_arg(r); + + if (blocked_syscall(sysno)) + return; + + /* Setting up the register frame is harness bookkeeping, not the code under + test; a failure here must not be reported as a finding. */ + tinykvm_regs regs; + try { + regs = g_fork->registers(); + for (unsigned i = 0; i < 6; i++) + regs.sysarg(i) = args[i]; + regs.sysret() = 0; + g_fork->set_registers(regs); + } catch (const std::exception& e) { + fprintf(stderr, "syscall_fuzz: harness could not set registers: %s\n", e.what()); + return; + } + + if (g_verbose) { + fprintf(stderr, "call %u(0x%llX, 0x%llX, 0x%llX, 0x%llX, 0x%llX, 0x%llX)\n", + sysno, (unsigned long long)regs.sysarg(0), (unsigned long long)regs.sysarg(1), + (unsigned long long)regs.sysarg(2), (unsigned long long)regs.sysarg(3), + (unsigned long long)regs.sysarg(4), (unsigned long long)regs.sysarg(5)); + } + + try { + g_fork->system_call(g_fork->cpu(), sysno); + } catch (const MachineException&) { + /* Expected and correct: this is how the emulation layer rejects a + bad pointer, a bad length or an unsupported request. */ + } catch (const std::exception& e) { + /* Not expected: an embedder catching MachineException would let this + escape into its own request loop. */ + if (g_strict) + throw; + note_foreign_exception(typeid(e).name(), e.what()); + } +} + +static void do_poke(Reader& r) +{ + const uint64_t addr = table_address(r); + const size_t len = r.u8(); + uint8_t buf[256]; + const size_t n = len < r.left() ? len : r.left(); + for (size_t i = 0; i < n; i++) + buf[i] = r.u8(); + if (n == 0) + return; + try { + g_fork->copy_to_guest(addr, buf, n); + } catch (const MachineException&) { + /* Expected: the fuzzer is meant to aim at unmapped addresses too. */ + } catch (const std::exception& e) { + /* Harness-side, not a syscall handler -- note it but never abort. */ + note_foreign_exception(typeid(e).name(), e.what()); + } +} + +static void reset_fork() +{ + try { + g_fork->reset_to(*g_master, { + .max_mem = MAX_MEMORY, + .max_cow_mem = MAX_COW_MEM, + }); + install_sandbox_policy(*g_fork); + g_fork->set_printer([](const char*, size_t) {}); + return; + } catch (const std::exception& e) { + fprintf(stderr, "syscall_fuzz: reset_to failed (%s), rebuilding fork\n", e.what()); + } + delete g_fork; + g_fork = new Machine { *g_master, { + .max_mem = MAX_MEMORY, + .max_cow_mem = MAX_COW_MEM, + } }; + install_sandbox_policy(*g_fork); + g_fork->set_printer([](const char*, size_t) {}); +} + +extern "C" int LLVMFuzzerInitialize(int*, char***) +{ + g_strict = getenv("TINYKVM_FUZZ_STRICT") != nullptr; + g_verbose = getenv("TINYKVM_FUZZ_VERBOSE") != nullptr; + const bool unsafe = getenv("TINYKVM_FUZZ_UNSAFE") != nullptr; + + /* The guest can get the host killed by SIGPIPE: create a pipe or + socketpair, close one end, write() to the other. The emulation layer + passes MSG_NOSIGNAL on the sendmsg/sendto paths but plain + write()/writev()/pwritev64() have no equivalent, so the default + disposition terminates the process. That is a real finding about the + library (see docs/syscall-fuzzing.md), not about this harness -- ignore it + here so one trivially-reachable signal does not end every campaign. */ + signal(SIGPIPE, SIG_IGN); + + /* read(0, ...) must not be able to block on the fuzzer's own stdin. */ + if (const int devnull = open("/dev/null", O_RDONLY); devnull >= 0) { + dup2(devnull, STDIN_FILENO); + if (devnull != STDIN_FILENO) + close(devnull); + } + + make_sandbox(); + atexit(remove_sandbox); + + Machine::init(); + Machine::setup_linux_system_calls(unsafe); + Machine::setup_multithreading(); + Machine::install_unhandled_syscall_handler([](vCPU&, unsigned) {}); + install_nonblocking_wrappers(); + + const char* guest_path = getenv("TINYKVM_FUZZ_GUEST"); +#ifdef FUZZ_GUEST_PATH + if (guest_path == nullptr) + guest_path = FUZZ_GUEST_PATH; +#endif + if (guest_path == nullptr) { + fprintf(stderr, "syscall_fuzz: set TINYKVM_FUZZ_GUEST to a static guest ELF\n"); + exit(1); + } + static const std::vector binary = load_file(guest_path); + + /* Everything below must succeed or be reported loudly. libFuzzer has not + installed its crash handlers yet at this point, so an exception escaping + here dies silently via std::terminate -- in fork mode that shows up as an + artifact-less "crash" in the parent's tally and looks like a finding. + Booting the master can genuinely fail transiently under load (the run + timeout is wall-clock), so retry a few times before giving up. */ + uint64_t scratch = 0, scratch_small = 0; + std::string last_error; + for (int attempt = 0; attempt < 4; attempt++) { + try { + delete g_master; + g_master = nullptr; + + g_master = new Machine { binary, { .max_mem = MAX_MEMORY } }; + install_sandbox_policy(*g_master); + g_master->set_printer([](const char*, size_t) {}); + g_master->setup_linux({ "syscall_fuzz" }, { "LC_ALL=C", "USER=root" }); + /* Generous: this is a trivial guest, but 8 sanitized workers can + starve each other badly enough to trip a tight timeout. */ + g_master->run(60.0f); + + scratch = g_master->address_of("scratch"); + scratch_small = g_master->address_of("scratch_small"); + if (scratch == 0x0 || scratch_small == 0x0) + throw std::runtime_error("guest ELF is missing the scratch symbols"); + + g_master->prepare_copy_on_write(MAX_COW_MEM); + + g_fork = new Machine { *g_master, { + .max_mem = MAX_MEMORY, + .max_cow_mem = MAX_COW_MEM, + } }; + install_sandbox_policy(*g_fork); + g_fork->set_printer([](const char*, size_t) {}); + last_error.clear(); + break; + } catch (const std::exception& e) { + last_error = e.what(); + fprintf(stderr, "syscall_fuzz: master setup attempt %d failed: %s\n", + attempt + 1, last_error.c_str()); + } + } + if (!last_error.empty()) { + fprintf(stderr, "syscall_fuzz: giving up on master setup: %s\n", last_error.c_str()); + exit(2); + } + + build_address_table(*g_master, scratch, scratch_small); + + fprintf(stderr, "syscall_fuzz: ready. scratch=0x%lX addresses=%zu strict=%d unsafe=%d\n", + scratch, g_addrs.size(), int(g_strict), int(unsafe)); + return 0; +} + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t len) +{ + Reader r { data, data + len }; + + for (size_t i = 0; i < MAX_RECORDS && !r.empty(); i++) { + const uint8_t op = r.u8(); + if ((op & 3) == 0) + do_poke(r); + else + do_call(r); + } + + reset_fork(); + return 0; +} diff --git a/fuzz/syscall_fuzzer.sh b/fuzz/syscall_fuzzer.sh new file mode 100755 index 00000000..8fd7b5c4 --- /dev/null +++ b/fuzz/syscall_fuzzer.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Build and run the syscall-emulation fuzzer. +# +# ./syscall_fuzzer.sh # run until interrupted +# ./syscall_fuzzer.sh -runs=100000 # extra libFuzzer flags are passed through +# +# Findings land in ./.build-syscall/crashes/. Fork mode is on so a crash does +# not end the campaign -- each one is saved and the run continues. +set -e + +cd "$(dirname "$0")" +BUILD=.build-syscall + +export ASAN_OPTIONS=detect_leaks=0:handle_segv=0:handle_sigfpe=0:allocator_may_return_null=1 +export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0 + +: "${CXX:=clang++}" +: "${CC:=clang}" +export CXX CC + +mkdir -p "$BUILD" +(cd "$BUILD" && cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo >/dev/null && make -j"$(nproc)" syscallfuzzer) + +python3 gen_syscall_seeds.py "$BUILD/seeds" +mkdir -p "$BUILD/corpus" "$BUILD/crashes" + +cd "$BUILD" +exec ./syscallfuzzer \ + -fork="$(( $(nproc) / 2 ))" \ + -ignore_crashes=1 \ + -ignore_timeouts=1 \ + -ignore_ooms=1 \ + -timeout=10 \ + -rss_limit_mb=4096 \ + -max_len=4096 \ + -artifact_prefix=crashes/ \ + corpus seeds "$@" diff --git a/fuzz/triage_syscall_crashes.sh b/fuzz/triage_syscall_crashes.sh new file mode 100755 index 00000000..c8c7e98c --- /dev/null +++ b/fuzz/triage_syscall_crashes.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Group syscall-fuzzer crash artifacts by root-cause signature. +# +# ./triage_syscall_crashes.sh [crash-dir] +# +# For each artifact this replays it under the sanitizers and reduces the report +# to (error kind, first frame inside lib/tinykvm). Identical signatures are the +# same bug reached by different inputs, so only the count and one representative +# input matter. +set -u + +BUILD="$(dirname "$0")/.build-syscall" +CRASHDIR="${1:-$BUILD/crashes}" +FUZZER="$BUILD/syscallfuzzer" +OUT="$BUILD/triage" + +mkdir -p "$OUT" +: > "$OUT/signatures.txt" + +export ASAN_OPTIONS=detect_leaks=0:handle_segv=0:handle_sigfpe=0:allocator_may_return_null=1 +export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0 + +classify() { + local f="$1" + local log + log="$(timeout 60 "$FUZZER" "$f" 2>&1)" + + # Error kind: prefer a sanitizer diagnosis over libFuzzer's generic signal. + local kind + kind="$(printf '%s\n' "$log" | grep -oE "AddressSanitizer: [a-z-]+" | head -1)" + if [ -z "$kind" ]; then + kind="$(printf '%s\n' "$log" | grep -oE "runtime error: [^']*('[^']*')?" | head -1 \ + | sed 's/ of type .*/ of type .../')" + fi + if [ -z "$kind" ]; then + kind="$(printf '%s\n' "$log" | grep -oE "libFuzzer: (deadly signal|out-of-memory|timeout)" | head -1)" + fi + [ -z "$kind" ] && kind="unknown" + + # First frame in the library or the fuzz harness, minus addresses. + local frame + frame="$(printf '%s\n' "$log" \ + | grep -oE "in [^ ]+ /home/[^ ]*/(lib/tinykvm|fuzz)/[^ ]+:[0-9]+" \ + | head -1 | sed 's|.*/\(lib/tinykvm\|fuzz\)/|\1/|')" + [ -z "$frame" ] && frame="$(printf '%s\n' "$log" | grep -oE "/home/[^ ]*/lib/tinykvm/[^ ]+:[0-9]+" \ + | head -1 | sed 's|.*/lib/tinykvm/|lib/tinykvm/|')" + [ -z "$frame" ] && frame="no-frame" + + printf '%s\t%s\t%s\n' "$kind" "$frame" "$f" +} +export -f classify +export FUZZER + +find "$CRASHDIR" -type f -name 'crash-*' -o -type f -name 'oom-*' -o -type f -name 'timeout-*' \ + | sort > "$OUT/artifacts.txt" + +echo "triaging $(wc -l < "$OUT/artifacts.txt") artifacts..." +xargs -a "$OUT/artifacts.txt" -P "$(nproc)" -I{} bash -c 'classify "$@"' _ {} \ + >> "$OUT/signatures.txt" 2>/dev/null + +echo +echo "=== distinct signatures (count, error, first library frame, example input) ===" +awk -F'\t' '{key=$1"\t"$2; cnt[key]++; if (!(key in ex)) ex[key]=$3} + END { for (k in cnt) printf "%6d\t%s\t%s\n", cnt[k], k, ex[k] }' \ + "$OUT/signatures.txt" | sort -rn From 53fa0a66a1f113d6c09fc4de22b2aea72caf2d72 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:52:44 +0200 Subject: [PATCH 02/12] openat: do not pass a mode to openat2 without O_CREAT openat2 rejects a non-zero open_how.mode unless the open can create a file. The write path passed S_IWUSR|S_IRUSR unconditionally, so O_RDWR or O_WRONLY on a path that already exists always failed with EINVAL -- guests could never open an existing file for writing. Found while writing the regression test for the zero-length I/O fix, not by the fuzzer; there is no oracle for "returns EINVAL when it shouldn't". Adds tests/unit/syscalls.cpp for this series. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 6 +- tests/unit/CMakeLists.txt | 5 ++ tests/unit/syscalls.cpp | 92 ++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tests/unit/syscalls.cpp diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 5937613e..ac66faf8 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -2419,9 +2419,13 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) } else { resolve |= RESOLVE_IN_ROOT | RESOLVE_NO_SYMLINKS | RESOLVE_NO_XDEV; } + /* openat2 rejects a non-zero mode unless the open is able to + create a file, so passing one unconditionally made every + write-open of an *existing* path fail with EINVAL. + (O_TMPFILE cannot appear here; flags is masked above.) */ struct open_how how { .flags = __u64(flags), - .mode = __u64(S_IWUSR | S_IRUSR), + .mode = __u64((flags & O_CREAT) ? (S_IWUSR | S_IRUSR) : 0), .resolve = resolve, }; int fd = syscall(SYS_openat2, pfd, real_path.c_str(), &how, sizeof(how)); diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index e28773ab..6a8138b1 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -29,6 +29,11 @@ if (TINYKVM_ARCH STREQUAL "AMD64") add_unit_test(reset reset.cpp) add_unit_test(timeout timeout.cpp) add_unit_test(tegridy tegridy.cpp) + add_unit_test(syscalls syscalls.cpp) + # One case here asserts that a guest-controlled length is not walked page + # by page. Before the fix it does not fail fast, it grinds -- cap it so a + # regression cannot wedge CI. + set_tests_properties(test_syscalls PROPERTIES TIMEOUT 120) elseif (TINYKVM_ARCH STREQUAL "ARM64") add_unit_test(arm64_minimal arm64_minimal.cpp) add_unit_test(arm64_elf arm64_elf.cpp) diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp new file mode 100644 index 00000000..ddbfb7c3 --- /dev/null +++ b/tests/unit/syscalls.cpp @@ -0,0 +1,92 @@ +#include + +#include +#include +#include +#include +#include + +/* Regression tests for host-side syscall emulation bugs found by + fuzz/syscall_fuzz.cpp. Each of these is reachable from an ordinary, + unprivileged guest using a completely legal syscall. */ + +extern std::vector build_and_load(const std::string& code); +extern std::pair> + build_and_load(const std::string& code, const std::string& args); +static const uint64_t MAX_MEMORY = 8ul << 20; /* 8MB */ +static const std::vector env { + "LC_TYPE=C", "LC_ALL=C", "USER=root" +}; + +namespace { + /* A real file on the host that the guest's open() calls are redirected to, + mirroring how an embedder's path policy rewrites guest paths. */ + struct ScratchFile { + std::string path; + + ScratchFile() + { + char tmpl[] = "/tmp/tinykvm-syscall-test-XXXXXX"; + const int fd = mkstemp(tmpl); + REQUIRE(fd >= 0); + /* Some content, so reads have something to return. */ + const std::string data(4096, 'A'); + REQUIRE(write(fd, data.data(), data.size()) == ssize_t(data.size())); + close(fd); + path = tmpl; + } + ~ScratchFile() { unlink(path.c_str()); } + }; + + void allow_scratch_file(tinykvm::Machine& machine, const ScratchFile& file) + { + machine.fds().set_open_readable_callback([&file](std::string& path) { + path = file.path; + return true; + }); + machine.fds().set_open_writable_callback([&file](std::string& path) { + path = file.path; + return true; + }); + /* The openat handler resolves against the cwd fd; without one, opens + for writing fail before the handler under test is reached. */ + machine.fds().set_current_working_directory("/tmp"); + } +} + +TEST_CASE("Initialize KVM", "[Initialize]") +{ + tinykvm::Machine::init(); +} + +TEST_CASE("Opening an existing file for writing succeeds", "[Syscalls]") +{ + /* The openat handler built an open_how with a non-zero .mode for every + write-open. openat2 only accepts a mode when the open can create a file, + so O_RDWR/O_WRONLY on a path that already exists failed with EINVAL -- + guests could never open an existing file for writing. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include + +int main() { + int fd = open("/scratch", O_RDWR); + if (fd < 0) return 100 + (errno & 0x7F); + close(fd); + + fd = open("/scratch", O_WRONLY); + if (fd < 0) return 200 + (errno & 0x7F); + close(fd); + return 0; +})M"); + + ScratchFile file; + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + allow_scratch_file(machine, file); + machine.setup_linux({"open-existing-writable"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} From 14857bfbe61e0073f57657f7c402e78d57bba715 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:53:12 +0200 Subject: [PATCH 03/12] linux: do not index an empty gather-buffer list on zero-length I/O read/write/pread64/pwrite64/mmap/sendmsg/recvmsg/sendto/recvfrom/sendmmsg gather the guest range into a vector of host buffers. A zero-length request gathers nothing, and the handlers then used buffers[0] or &buffers[0] on the empty vector. write() actually loaded through it, so write(fd, p, 0) -- a legal no-op -- null-dereferenced and killed the VMM. The rest bound a reference to a null pointer, which is UB even though the syscalls tolerate an iovec count of zero. Use buffers.data() throughout, and answer a zero-length write directly (the fd is still validated first, since write(badfd, _, 0) must fail). Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 32 +++++--- tests/unit/syscalls.cpp | 117 +++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 11 deletions(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index ac66faf8..b9f091ef 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -79,9 +79,11 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) ssize_t result = 0; if (bufcount == 1) { result = read(fd, buffers[0].ptr, buffers[0].len); - } else { - result = readv(fd, (struct iovec *)&buffers[0], bufcount); + } else if (bufcount > 1) { + result = readv(fd, (struct iovec *)buffers.data(), bufcount); } + /* A zero-length read gathers no buffers; buffers.data() is null and + must not be indexed. Result stays 0, matching read(fd, _, 0). */ if (UNLIKELY(result < 0)) { regs.sysret() = -errno; } else { @@ -125,8 +127,13 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) const int fd = cpu.machine().fds().translate_writable_vfd(regs.sysarg(0)); if (bufcount > 1) { regs.sysret() = writev(fd, (const struct iovec *)buffers.data(), bufcount); - } else { + } else if (bufcount == 1) { regs.sysret() = write(fd, buffers[0].ptr, buffers[0].len); + } else { + /* A zero-length write gathers no buffers. buffers[0] would + dereference null here, so answer it directly -- the fd was + still validated above, as write(badfd, _, 0) must fail. */ + regs.sysret() = 0; } SYSPRINT("write(fd=%d (%d), data=0x%llX, size=%zu) = %lld\n", vfd, fd, regs.sysarg(1), bytes, regs.sysret()); @@ -441,7 +448,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) const size_t cnt = cpu.machine().writable_buffers_from_range(buffers, dst, read_length); // Seek to the given offset in the file and read the contents into guest memory - if (preadv64(real_fd, (const iovec *)&buffers[0], cnt, voff) < 0) { + if (preadv64(real_fd, (const iovec *)buffers.data(), cnt, voff) < 0) { PRINTMMAP("preadv64 failed: %s for %zu buffers, vfd %d fd %d at offset %ld\n", strerror(errno), cnt, vfd, real_fd, voff); for (size_t i = 0; i < cnt; i++) @@ -741,8 +748,10 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) const auto bufcount = cpu.machine().writable_buffers_from_range(buffers, g_buf, bytes); + /* bufcount == 0 for a zero-length read: buffers.data() is null, + which preadv64 accepts with an iovec count of 0. */ ssize_t result = - preadv64(fd, (iovec *)&buffers[0], bufcount, offset); + preadv64(fd, (iovec *)buffers.data(), bufcount, offset); if (result < 0) { regs.sysret() = -errno; } @@ -767,7 +776,8 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) const auto bufcount = cpu.machine().gather_buffers_from_range(buffers, g_buf, bytes); - if (pwritev64(fd, (const iovec *)&buffers[0], bufcount, offset) < 0) { + /* See pread64: buffers.data() is null when bufcount == 0. */ + if (pwritev64(fd, (const iovec *)buffers.data(), bufcount, offset) < 0) { regs.sysret() = -errno; } else { @@ -1441,7 +1451,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) struct msghdr msg {}; msg.msg_name = nullptr; msg.msg_namelen = 0; - msg.msg_iov = (struct iovec *)&buffers[0]; + msg.msg_iov = (struct iovec *)buffers.data(); msg.msg_iovlen = bufcount; msg.msg_control = nullptr; msg.msg_controllen = 0; @@ -1505,7 +1515,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) struct msghdr msg {}; msg.msg_name = &addr; msg.msg_namelen = sizeof(addr); - msg.msg_iov = (struct iovec *)&buffers[0]; + msg.msg_iov = (struct iovec *)buffers.data(); msg.msg_iovlen = bufcount; msg.msg_control = nullptr; msg.msg_controllen = 0; @@ -1578,7 +1588,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) struct msghdr msg_recv {}; msg_recv.msg_name = &addr; msg_recv.msg_namelen = sizeof(addr); - msg_recv.msg_iov = (struct iovec *)&buffers[0]; + msg_recv.msg_iov = (struct iovec *)buffers.data(); msg_recv.msg_iovlen = bufcount; msg_recv.msg_control = nullptr; msg_recv.msg_controllen = 0; @@ -1659,7 +1669,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) struct msghdr msg_send {}; msg_send.msg_name = nullptr; msg_send.msg_namelen = 0; - msg_send.msg_iov = (struct iovec *)&buffers[0]; + msg_send.msg_iov = (struct iovec *)buffers.data(); msg_send.msg_iovlen = bufcount; msg_send.msg_control = nullptr; msg_send.msg_controllen = 0; @@ -2911,7 +2921,7 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) const size_t cnt = cpu.machine().gather_buffers_from_range( buffers, iov.iov_base, iov.iov_len); - ssize_t result = writev(fd, (const iovec *)&buffers[0], cnt); + ssize_t result = writev(fd, (const iovec *)buffers.data(), cnt); if (result < 0) { total = -errno; diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index ddbfb7c3..61278a36 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -90,3 +90,120 @@ int main() { REQUIRE(machine.return_value() == 0); } + +TEST_CASE("Zero-length I/O does not dereference an empty buffer list", "[Syscalls]") +{ + /* read/write/pread64/pwrite64 gather the guest range into a vector of + host buffers. A zero-length request gathers nothing, and the handlers + then indexed buffers[0] on the empty vector. For write() that was a + load from a null pointer -- i.e. any guest could kill the VMM process + with write(fd, p, 0), a legal no-op call. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include +#include + +int main() { + int fd = open("/scratch", O_RDWR); + if (fd < 0) return 1; + + char b[1] = {'x'}; + if (write(fd, b, 0) != 0) return 2; + if (read(fd, b, 0) != 0) return 3; + if (pwrite(fd, b, 0, 0) != 0) return 4; + if (pread(fd, b, 0, 0) != 0) return 5; + + /* Zero-length writev/readv too, for good measure. */ + struct iovec iov = { b, 0 }; + if (writev(fd, &iov, 1) != 0) return 6; + + return 0; +})M"); + + ScratchFile file; + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + allow_scratch_file(machine, file); + machine.setup_linux({"zero-length-io"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} + +TEST_CASE("Socket and pipe I/O still round-trips", "[Syscalls]") +{ + /* Positive counterpart to the bounds checks and empty-buffer-list fixes + below: those added new -EINVAL paths and changed how the gathered buffer + list is handed to the host, so verify the ordinary cases still work. + Covers write/read, writev/readv, sendmsg/recvmsg and sendto/recvfrom over + a socketpair and a pipe, including a multi-entry iovec. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include +#include +#include + +int main() { + int sv[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) < 0) return 1; + + /* write() / read() */ + if (write(sv[0], "hello", 5) != 5) return 2; + char buf[32] = {0}; + if (read(sv[1], buf, sizeof(buf)) != 5) return 3; + if (memcmp(buf, "hello", 5) != 0) return 4; + + /* writev() with two entries. NB: readv() has no handler in the emulation + layer at all (it returns -ENOSYS), so read it back with read(). */ + struct iovec wv[2] = { { "ab", 2 }, { "cde", 3 } }; + if (writev(sv[0], wv, 2) != 5) return 5; + memset(buf, 0, sizeof(buf)); + if (read(sv[1], buf, sizeof(buf)) != 5) return 6; + if (memcmp(buf, "abcde", 5) != 0) return 7; + + /* sendmsg() / recvmsg() with msg_name unused (namelen 0) */ + struct iovec siov = { "msghdr", 6 }; + struct msghdr smsg; + memset(&smsg, 0, sizeof(smsg)); + smsg.msg_iov = &siov; + smsg.msg_iovlen = 1; + if (sendmsg(sv[0], &smsg, 0) != 6) return 8; + + memset(buf, 0, sizeof(buf)); + struct iovec riov = { buf, sizeof(buf) }; + struct msghdr rmsg; + memset(&rmsg, 0, sizeof(rmsg)); + rmsg.msg_iov = &riov; + rmsg.msg_iovlen = 1; + if (recvmsg(sv[1], &rmsg, 0) != 6) return 9; + if (memcmp(buf, "msghdr", 6) != 0) return 10; + + /* sendto() / recvfrom() on a connected socket: no address */ + if (sendto(sv[0], "sendto", 6, 0, NULL, 0) != 6) return 11; + memset(buf, 0, sizeof(buf)); + if (recvfrom(sv[1], buf, sizeof(buf), 0, NULL, NULL) != 6) return 12; + if (memcmp(buf, "sendto", 6) != 0) return 13; + + close(sv[0]); + close(sv[1]); + + /* And the same for a pipe. */ + int pfd[2]; + if (pipe(pfd) < 0) return 14; + if (write(pfd[1], "pipe", 4) != 4) return 15; + memset(buf, 0, sizeof(buf)); + if (read(pfd[0], buf, sizeof(buf)) != 4) return 16; + if (memcmp(buf, "pipe", 4) != 0) return 17; + + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"socket-roundtrip"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} From dd68749e9a6c7e9e37fad7e2303c27730419301b Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:53:22 +0200 Subject: [PATCH 04/12] getdents64: keep the host result in a signed local The handler stored the host getdents64() return value straight into sysret(), which is __u64, and then tested it with `> 0`. On failure that turned -1 into ~2^64 and handed it to copy_to_guest() as a length, so the host stack was streamed into guest memory page after page until the copy walked off the end of the guest address space. getdents64 on any regular fd returns ENOTDIR, which is enough to trigger it: an unprivileged guest gets a host stack disclosure and then a crash. Also report -errno on failure, matching the rest of the file. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 16 +++++++++-- tests/unit/syscalls.cpp | 45 ++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index b9f091ef..1345f240 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -2273,10 +2273,20 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) int fd = cpu.machine().fds().translate(regs.sysarg(0)); char buffer[2048]; - regs.sysret() = syscall(SYS_getdents64, fd, buffer, sizeof(buffer)); - if (regs.sysret() > 0) + /* NB: keep the host result in a *signed* local. sysret() is __u64, + so testing it directly turns a -1 error return into a ~2^64 + length and copies the host stack into the guest. */ + const ssize_t result = syscall(SYS_getdents64, fd, buffer, sizeof(buffer)); + if (result > 0) { - cpu.machine().copy_to_guest(regs.sysarg(1), buffer, regs.sysret()); + cpu.machine().copy_to_guest(regs.sysarg(1), buffer, result); + regs.sysret() = result; + } + else if (result < 0) { + regs.sysret() = -errno; + } + else { + regs.sysret() = 0; } cpu.set_registers(regs); SYSPRINT("GETDENTS64 to vfd=%lld, fd=%d, data=0x%llX = %lld\n", diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index 61278a36..60ff25ed 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -131,6 +131,51 @@ int main() { REQUIRE(machine.return_value() == 0); } +TEST_CASE("getdents64 on a non-directory does not leak host memory", "[Syscalls]") +{ + /* The handler stored the host getdents64() return value in sysret(), + which is __u64, and then tested it with `> 0`. On failure (-1) that + became ~2^64 and was passed to copy_to_guest() as a length, streaming + the host stack into guest memory until the copy walked off the end of + the guest address space. getdents64 on a regular fd -- which returns + ENOTDIR -- is enough to trigger it. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include +#include + +/* In .bss, far away from anything the guest cares about. */ +char dirbuf[8192]; + +int main() { + int fd = open("/scratch", O_RDONLY); + if (fd < 0) return 1; + + memset(dirbuf, 0xAA, sizeof(dirbuf)); + + long rc = syscall(SYS_getdents64, fd, dirbuf, sizeof(dirbuf)); + /* A regular file is not a directory: this must fail, not succeed. */ + if (rc >= 0) return 2; + + /* Nothing may have been written into the guest buffer. */ + for (unsigned i = 0; i < sizeof(dirbuf); i++) { + if ((unsigned char)dirbuf[i] != 0xAA) + return 3; + } + return 0; +})M"); + + ScratchFile file; + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + allow_scratch_file(machine, file); + machine.setup_linux({"getdents64-enotdir"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} + TEST_CASE("Socket and pipe I/O still round-trips", "[Syscalls]") { /* Positive counterpart to the bounds checks and empty-buffer-list fixes From b9ce634b3e09576c55026c5358e68e09c89abb2f Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:53:33 +0200 Subject: [PATCH 05/12] sendmsg: bound msg_namelen before copying into sockaddr_storage msg_namelen comes from the guest's own msghdr and was used unchecked as the length of a copy into a 128-byte sockaddr_storage on the handler's stack. Both the length and the bytes are guest-controlled, so this was a straight stack smash of the VMM -- it segfaults a plain release build. Reject anything larger than the destination, as connect()/bind()/sendto() already do. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 11 ++++++++ tests/unit/syscalls.cpp | 42 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 1345f240..85c8e60d 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -1675,6 +1675,17 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) msg_send.msg_controllen = 0; msg_send.msg_flags = MSG_NOSIGNAL; // Ignore SIGPIPE + /* msg_namelen comes from the guest's own msghdr and addr is a + 128-byte stack object, so an unchecked length is a + guest-controlled stack overflow. */ + if (UNLIKELY(msg.msg_namelen > sizeof(addr))) + { + regs.sysret() = -EINVAL; + cpu.set_registers(regs); + SYSPRINT("sendmsg(fd=%d (%d), msg=0x%lX, flags=0x%X) = %lld (EINVAL, namelen too large)\n", + vfd, fd, g_msg, flags, regs.sysret()); + return; + } if (msg.msg_namelen > 0 && msg.msg_name != 0x0) { cpu.machine().copy_from_guest(&addr, reinterpret_cast(msg.msg_name), msg.msg_namelen); msg_send.msg_name = &addr; diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index 60ff25ed..e296d8b9 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -252,3 +252,45 @@ int main() { REQUIRE(machine.return_value() == 0); } + +TEST_CASE("sendmsg rejects an oversized msg_namelen", "[Syscalls]") +{ + /* The handler copied msg_namelen bytes out of the guest into a 128-byte + sockaddr_storage on its own stack, with the length taken straight from + the guest's msghdr and never bounded. Both the length and the bytes are + guest-controlled, so this was a straightforward stack smash of the VMM. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include + +static char payload[8192]; + +int main() { + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if (fd < 0) return 1; + + memset(payload, 0x41, sizeof(payload)); + + char body[8] = {0}; + struct iovec iov = { body, sizeof(body) }; + + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + msg.msg_name = payload; + msg.msg_namelen = sizeof(payload); /* >> sizeof(sockaddr_storage) */ + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + + /* Must be refused, not copied. Any error is acceptable. */ + if (sendmsg(fd, &msg, 0) >= 0) return 2; + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"sendmsg-namelen"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} From 35c225ba6ec1876a709dcc10341a68aeddb98b55 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:53:42 +0200 Subject: [PATCH 06/12] sendmmsg: bound vlen before copying the mmsghdr array vlen was only checked for being positive before being multiplied by sizeof(mmsghdr) and used as the length of a copy into a 1024-entry (64KB) stack array. A large vlen overflowed the VMM's stack with guest-supplied bytes; it segfaults a plain release build. iovlen was already bounded a few lines below; vlen was not. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 11 ++++++++++ tests/unit/syscalls.cpp | 34 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 85c8e60d..4ea93dfa 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -2909,6 +2909,17 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) std::array guest_msgs; std::array guest_iovecs; std::vector buffers; + /* vcnt is guest-controlled and guest_msgs is a 64KB stack + object: without this bound the copy below is a + guest-controlled stack overflow. (iovlen is bounded below.) */ + if (UNLIKELY(size_t(vcnt) > guest_msgs.size())) + { + regs.sysret() = -EINVAL; + cpu.set_registers(regs); + SYSPRINT("sendmmsg(fd=%d, vlen=%d) = %lld (EINVAL, vlen too large)\n", + fd, vcnt, regs.sysret()); + return; + } // Fetch the mmsghdrs from the guest cpu.machine().copy_from_guest(guest_msgs.data(), g_buf, vcnt * sizeof(struct mmsghdr)); // For each mmsghdr, fetch the iovec and sockaddr diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index e296d8b9..32b1e863 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -294,3 +294,37 @@ int main() { REQUIRE(machine.return_value() == 0); } + +TEST_CASE("sendmmsg rejects an oversized message count", "[Syscalls]") +{ + /* vlen was only checked for being positive before being multiplied by + sizeof(mmsghdr) and used as the length of a copy into a 1024-entry + (64KB) stack array. A large vlen overflowed the VMM's stack with + guest-supplied bytes. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include +#include + +static char payload[262144]; + +int main() { + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if (fd < 0) return 1; + + memset(payload, 0x42, sizeof(payload)); + + /* 4096 mmsghdrs is 4x the handler's fixed capacity. */ + long rc = syscall(SYS_sendmmsg, fd, payload, 4096, 0); + if (rc >= 0) return 2; + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"sendmmsg-vlen"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} From b26c174f78e0e45d6f3eb5dbb58eb575edfe8f8b Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:54:19 +0200 Subject: [PATCH 07/12] memory: clamp memzero() to the VM's own address space memzero() walks a range one page-table lookup per 4K and deliberately ignores pages that are not present, so an out-of-range length is not an error -- it is an unbounded walk. madvise(MADV_DONTNEED) hands the guest's length straight in, so a single syscall pins the host thread: measured ~9M pages/s, i.e. minutes for a 16TB length and years for 2^64. It runs inside the syscall handler, so the execution timeout never fires. Clamp to memory.remote_end (which already accounts for extra virtual remappings) plus a connected remote's range, so address-space merging still works. Pages past the end can never be dirty, so nothing is lost: mmap_allocate() hands out virtual addresses below max_address -- MMAP_PHYS_BASE is the physical base of the mmap arena, not a virtual one -- so mmap'd memory stays inside the clamp and is still zeroed. Tests cover a full range, a partial (middle-pages-only) discard, and the has_remote() branch. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/machine_utils.cpp | 21 +++++ tests/unit/syscalls.cpp | 143 ++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/lib/tinykvm/machine_utils.cpp b/lib/tinykvm/machine_utils.cpp index 1132e6e3..aef23b00 100644 --- a/lib/tinykvm/machine_utils.cpp +++ b/lib/tinykvm/machine_utils.cpp @@ -20,6 +20,27 @@ void Machine::memzero(address_t addr, size_t len) user page on arm64 (bit 6 is AP[1] there), so a large PROT_NONE mmap reservation walked off the end of guest RAM. */ const uint64_t dirty_bit = paging_dirty_bit(); + + /* Clamp the range to the addresses this VM's page tables can actually + cover before walking it. The loop below deliberately ignores missing + pages rather than faulting, so an out-of-range length is not an error + here -- it is an unbounded walk at one page-table lookup per 4K. + madvise(MADV_DONTNEED) hands a guest-chosen length straight in, so + without this a single guest syscall pins the host thread for years, and + because it runs inside the syscall handler the execution timeout does not + cover it. Pages past the end can never be dirty, so nothing is lost. + remote_end (not physbase+size) is the bound, because it already accounts + for extra virtual remappings; a connected remote VM's range is added on + top so address-space merging keeps working. */ + address_t limit = memory.remote_end; + if (this->has_remote()) { + limit = std::max(limit, m_remote->main_memory().remote_end); + } + if (addr >= limit) + return; + if (len > limit - addr) + len = limit - addr; + while (len != 0) { const size_t offset = addr & PageMask(); diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index 32b1e863..af2f57b3 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -328,3 +328,146 @@ int main() { REQUIRE(machine.return_value() == 0); } + +TEST_CASE("madvise(MADV_DONTNEED) still zeroes in-range memory", "[Syscalls]") +{ + /* Positive counterpart to the clamp added to Machine::memzero(): the range + is now bounded to the addresses this VM's page tables can cover, so verify + an ordinary in-range MADV_DONTNEED is still honoured and not clipped away. + Uses several pages, and checks the page straddling the end of the range is + handled too. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include + +#define LEN (16 * 4096) + +int main() { + unsigned char *p = mmap(NULL, LEN, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (p == MAP_FAILED) return 1; + + memset(p, 0xA5, LEN); + /* Confirm the write landed, so a zero result below means something. */ + for (unsigned i = 0; i < LEN; i += 4096) { + if (p[i] != 0xA5) return 2; + } + if (p[LEN - 1] != 0xA5) return 3; + + if (madvise(p, LEN, MADV_DONTNEED) != 0) return 4; + + /* Every byte must now read back as zero. */ + for (unsigned i = 0; i < LEN; i++) { + if (p[i] != 0x00) return 5; + } + + /* Partial range: dirty it again, discard only the middle two pages. */ + memset(p, 0x5A, LEN); + if (madvise(p + 4096, 2 * 4096, MADV_DONTNEED) != 0) return 6; + for (unsigned i = 0; i < 4096; i++) { + if (p[i] != 0x5A) return 7; /* before: untouched */ + } + for (unsigned i = 4096; i < 3 * 4096; i++) { + if (p[i] != 0x00) return 8; /* discarded */ + } + for (unsigned i = 3 * 4096; i < LEN; i++) { + if (p[i] != 0x5A) return 9; /* after: untouched */ + } + return 0; +})M"); + + /* Dirtying 64KB of fresh anonymous memory in a non-forked master needs more + headroom than the 8MB the other cases use. */ + tinykvm::Machine machine { binary, { .max_mem = 64ul << 20 } }; + machine.setup_linux({"madvise-dontneed"}, env); + machine.run(8.0f); + + REQUIRE(machine.return_value() == 0); +} + +TEST_CASE("madvise still zeroes when a remote VM is connected", "[Syscalls]") +{ + /* The clamp added to Machine::memzero() widens its bound to include a + connected remote's range, so that address-space merging keeps working. + That branch is only taken when has_remote() is true, which no other test + reaches -- so exercise it and confirm zeroing of the VM's *own* memory is + unaffected by the presence of a remote. */ + const auto storage_binary = build_and_load(R"M( +int main() { return 1234; } +)M", "-Wl,-Ttext-segment=0x40400000"); + + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include +#include + +#define LEN (8 * 4096) + +int main() { + unsigned char *p = mmap(NULL, LEN, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (p == MAP_FAILED) return 1; + + memset(p, 0x3C, LEN); + if (p[0] != 0x3C || p[LEN - 1] != 0x3C) return 2; + + if (madvise(p, LEN, MADV_DONTNEED) != 0) return 3; + for (unsigned i = 0; i < LEN; i++) { + if (p[i] != 0x00) return 4; + } + return 0; +})M"); + + tinykvm::Machine storage { storage_binary.second, { + .max_mem = 16ULL << 20, + .vmem_base_address = 1ULL << 30, /* 1GB, above the main VM */ + } }; + storage.setup_linux({"storage"}, env); + storage.run(4.0f); + REQUIRE(storage.return_value() == 1234); + + tinykvm::Machine machine { binary, { .max_mem = 64ul << 20 } }; + machine.setup_linux({"madvise-remote"}, env); + machine.remote_connect(storage); + machine.set_remote_allow_page_faults(true); + REQUIRE(machine.has_remote()); + + machine.run(8.0f); + REQUIRE(machine.return_value() == 0); +} + +TEST_CASE("madvise with an out-of-range length returns promptly", "[Syscalls]") +{ + /* madvise(MADV_DONTNEED) passes the guest's length straight to + Machine::memzero(), which walks the range one page-table lookup per 4K + and deliberately ignores pages that are not present -- so an + out-of-range length is not an error, it is an unbounded walk. At ~10M + pages/second a 16TB length is minutes and a 2^64 length is years, and + because this runs inside the syscall handler the execution timeout does + not apply: the host thread is simply gone. + + 16TB is chosen so the pre-fix behaviour is slow enough to fail this + assertion by a wide margin without hanging CI outright. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include + +int main() { + /* Return value is unimportant -- what matters is that we get here. */ + madvise((void *)0x1000000, 1UL << 44, MADV_DONTNEED); + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"madvise-huge"}, env); + + const auto t0 = std::chrono::steady_clock::now(); + machine.run(30.0f); + const auto elapsed = std::chrono::duration( + std::chrono::steady_clock::now() - t0).count(); + + REQUIRE(machine.return_value() == 0); + REQUIRE(elapsed < 5.0); +} From 44271c8e213658f8325765e0a8790a8fcc7097ec Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:54:53 +0200 Subject: [PATCH 08/12] prctl: do not read past the name literal in PR_GET_NAME buflen (up to 16) was used as the length of a copy straight out of the 8-byte literal "tinykvm", so the guest got up to 8 bytes of whatever .rodata followed it. Copy out of a zero-padded 16-byte buffer instead, which also matches Linux always writing 16 bytes. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 6 +++++- tests/unit/syscalls.cpp | 31 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 4ea93dfa..0763dc33 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -2214,7 +2214,11 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) regs.sysret() = -EINVAL; else { - const char *name = "tinykvm"; + /* Copy out of a padded 16-byte buffer: reading buflen bytes + straight from the literal ran off the end of it and handed + the guest whatever .rodata followed. */ + char name[16] {}; + __builtin_strncpy(name, "tinykvm", sizeof(name) - 1); cpu.machine().copy_to_guest(g_buf, name, buflen); regs.sysret() = 0; } diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index af2f57b3..998945a0 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -329,6 +329,37 @@ int main() { REQUIRE(machine.return_value() == 0); } +TEST_CASE("prctl(PR_GET_NAME) does not read past its name literal", "[Syscalls]") +{ + /* The handler copied buflen bytes (up to 16) directly out of the 8-byte + string literal "tinykvm", handing the guest whatever .rodata happened to + follow it. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include + +int main() { + char name[16]; + memset(name, 0xCC, sizeof(name)); + + if (prctl(PR_GET_NAME, name, sizeof(name), 0, 0) != 0) return 1; + if (strncmp(name, "tinykvm", 7) != 0) return 2; + + /* Everything after the name must be NUL padding, not host .rodata. */ + for (unsigned i = 7; i < sizeof(name); i++) { + if (name[i] != 0) return 3; + } + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"prctl-get-name"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} + TEST_CASE("madvise(MADV_DONTNEED) still zeroes in-range memory", "[Syscalls]") { /* Positive counterpart to the clamp added to Machine::memzero(): the range From 8e4683fae4e8c622b99d6f001a162bd85cd70a17 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:55:55 +0200 Subject: [PATCH 09/12] linux: check the host fd before handing it to manage() timerfd_create, eventfd2 and inotify_init1 passed a failed host fd straight to FileDescriptors::manage(), which throws on a negative fd. The `if (vfd < 0)` below each call was therefore dead code, and a std::runtime_error escaped the handler instead -- past every embedder that catches MachineException. timerfd_create's clockid and inotify_init1's flags are guest-controlled, so any guest can trigger it with a single call. epoll_create1 in the same file already had the right shape; follow it. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 22 +++++++++++++++------- tests/unit/syscalls.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 0763dc33..f7a509ff 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -2550,11 +2550,14 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) /* SYS eventfd2 */ auto& regs = cpu.registers(); const int real_fd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - const int vfd = cpu.machine().fds().manage(real_fd, false, true); - if (UNLIKELY(vfd < 0)) { + /* manage() throws on a negative fd, so the failure has to be caught + here -- testing its return value afterwards never runs. Same + shape as epoll_create1() below. */ + if (UNLIKELY(real_fd < 0)) { regs.sysret() = -errno; } else { + const int vfd = cpu.machine().fds().manage(real_fd, false, true); regs.sysret() = vfd; // Record the eventfd2 in the socket pairs cpu.machine().fds().add_socket_pair({vfd, -1, FileDescriptors::SocketType::EVENTFD}); @@ -2569,11 +2572,15 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) auto& regs = cpu.registers(); const int clockid = regs.sysarg(0); const int real_fd = timerfd_create(clockid, TFD_CLOEXEC | TFD_NONBLOCK); - const int vfd = cpu.machine().fds().manage(real_fd, false, true); - if (UNLIKELY(vfd < 0)) { + /* clockid is guest-controlled, so this call really does fail, and + manage() throws std::runtime_error on a negative fd -- which would + escape past every embedder that catches MachineException. Check + before managing, as epoll_create1() below does. */ + if (UNLIKELY(real_fd < 0)) { regs.sysret() = -errno; } else { + const int vfd = cpu.machine().fds().manage(real_fd, false, true); regs.sysret() = vfd; // TODO: Record the timerfd in the socket pairs //cpu.machine().fds().add_socket_pair({vfd, -1, FileDescriptors::SocketType::EVENTFD}); @@ -3307,12 +3314,13 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) auto& regs = cpu.registers(); const int flags = regs.sysarg(0); const int real_fd = inotify_init1(flags); - const int vfd = cpu.machine().fds().manage(real_fd, false, true); - if (UNLIKELY(vfd < 0)) { + /* flags is guest-controlled: an invalid value fails here, and + manage() throws on a negative fd rather than returning one. */ + if (UNLIKELY(real_fd < 0)) { regs.sysret() = -errno; } else { - regs.sysret() = vfd; + regs.sysret() = cpu.machine().fds().manage(real_fd, false, true); } cpu.set_registers(regs); SYSPRINT("inotify_init1(flags=0x%X) = %d (%lld)\n", diff --git a/tests/unit/syscalls.cpp b/tests/unit/syscalls.cpp index 998945a0..ca9de020 100644 --- a/tests/unit/syscalls.cpp +++ b/tests/unit/syscalls.cpp @@ -360,6 +360,33 @@ int main() { REQUIRE(machine.return_value() == 0); } +TEST_CASE("timerfd_create with a bad clockid returns an error", "[Syscalls]") +{ + /* clockid is guest-controlled and reaches timerfd_create() directly. On + failure the handler passed the -1 to FileDescriptors::manage(), which + throws std::runtime_error -- so the following `if (vfd < 0)` was dead + code and the exception escaped the handler entirely. An embedder that + catches MachineException (the documented contract) would not catch it. */ + const auto binary = build_and_load(R"M( +#define _GNU_SOURCE +#include +#include + +int main() { + /* Not a valid clockid. */ + int fd = timerfd_create(0x7FFFFFFF, 0); + if (fd >= 0) return 1; + if (errno <= 0) return 2; + return 0; +})M"); + + tinykvm::Machine machine { binary, { .max_mem = MAX_MEMORY } }; + machine.setup_linux({"timerfd-bad-clockid"}, env); + machine.run(4.0f); + + REQUIRE(machine.return_value() == 0); +} + TEST_CASE("madvise(MADV_DONTNEED) still zeroes in-range memory", "[Syscalls]") { /* Positive counterpart to the clamp added to Machine::memzero(): the range From 76a2268ad487b627d3fbb52a1fe015a93f737859 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:56:05 +0200 Subject: [PATCH 10/12] ppoll: validate the guest timespec instead of overflowing it ts.tv_sec * 1000 was computed on a raw guest value: signed overflow UB, and the int() truncation then produced an arbitrary timeout. Reject what Linux rejects (negative tv_sec/tv_nsec, tv_nsec >= 1e9) and compute in a range where the multiply cannot overflow, saturating at INT32_MAX ms. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index f7a509ff..88ffcaf3 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -358,7 +358,23 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) if (regs.sysarg(2) != 0) { struct timespec ts {}; cpu.machine().copy_from_guest(&ts, regs.sysarg(2), sizeof(ts)); - timeout = int(ts.tv_sec * 1000 + ts.tv_nsec / 1000000); + /* tv_sec/tv_nsec come from the guest. Reject what Linux rejects, + and compute in a range where tv_sec * 1000 cannot overflow -- + the multiply on a raw guest value was signed-overflow UB, and + the int() truncation then produced an arbitrary timeout. */ + if (UNLIKELY(ts.tv_sec < 0 || ts.tv_nsec < 0 || ts.tv_nsec >= 1000000000L)) + { + regs.sysret() = -EINVAL; + cpu.set_registers(regs); + SYSPRINT("ppoll(fds=0x%llX, count=%u, bad timespec) = %lld\n", + regs.sysarg(0), guest_count, regs.sysret()); + return; + } + static constexpr int64_t MAX_TIMEOUT_MS = INT32_MAX; + const int64_t secs_ms = (ts.tv_sec > MAX_TIMEOUT_MS / 1000) + ? MAX_TIMEOUT_MS : int64_t(ts.tv_sec) * 1000; + const int64_t total_ms = secs_ms + int64_t(ts.tv_nsec) / 1000000; + timeout = int(std::min(total_ms, MAX_TIMEOUT_MS)); } if (auto& callback = cpu.machine().fds().poll_callback; callback) { if (!callback(fds, guest_count, timeout)) From 2f5e6f3eaf6839a57bbd61788fe0073b13cfc70e Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:56:36 +0200 Subject: [PATCH 11/12] recvmsg, recvfrom: do not bind socklen_t& to a guest pointer The guest chooses the address, so it has arbitrary alignment; binding a socklen_t reference to it via writable_memarray() is UB and the accesses are unaligned. Benign on x86-64, but it breaks under stricter codegen and on strict-alignment targets. Copy in and out instead. poll/ppoll still do the same thing with writable_memarray; fixing that changes guest-visible semantics, so it is left alone for now. Co-Authored-By: Claude Opus 5 (1M context) --- lib/tinykvm/linux/system_calls.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 88ffcaf3..8eee16a2 100644 --- a/lib/tinykvm/linux/system_calls.cpp +++ b/lib/tinykvm/linux/system_calls.cpp @@ -1545,10 +1545,13 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) { if (g_addrlen != 0x0) { const socklen_t addrlen = msg.msg_namelen; - // Get a writable reference to the guest addrlen - socklen_t& guest_addrlen = - *cpu.machine().writable_memarray(g_addrlen); + /* g_addrlen is a guest pointer with arbitrary + alignment, so binding a socklen_t& to it is UB. + Copy in and out instead. */ + socklen_t guest_addrlen = 0; + cpu.machine().copy_from_guest(&guest_addrlen, g_addrlen, sizeof(guest_addrlen)); guest_addrlen = std::min(guest_addrlen, addrlen); + cpu.machine().copy_to_guest(g_addrlen, &guest_addrlen, sizeof(guest_addrlen)); // Write back the address if there is space if (g_addr != 0x0 && guest_addrlen > 0) { cpu.machine().copy_to_guest(g_addr, &addr, guest_addrlen); @@ -1625,12 +1628,17 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls) regs.sysret() = -errno; } else { if (msg.msg_name != nullptr && msg.msg_namelen > 0) { - // Write back the address if there is space - socklen_t& guest_addrlen = *cpu.machine().writable_memarray( - g_msg + offsetof(struct msghdr, msg_namelen)); + /* The guest chooses g_msg, so msg_namelen inside it has + arbitrary alignment; binding a socklen_t& to it is UB. + Copy in and out instead. */ + const address_t g_namelen = + g_msg + offsetof(struct msghdr, msg_namelen); + socklen_t guest_addrlen = 0; + cpu.machine().copy_from_guest(&guest_addrlen, g_namelen, sizeof(guest_addrlen)); const address_t g_addr = (uintptr_t)msg.msg_name; // Set/truncate the address length guest_addrlen = std::min(guest_addrlen, msg_recv.msg_namelen); + cpu.machine().copy_to_guest(g_namelen, &guest_addrlen, sizeof(guest_addrlen)); if (g_addr != 0x0 && guest_addrlen > 0) { // Write back the address cpu.machine().copy_to_guest(g_addr, &addr, guest_addrlen); From 5940ea6bfeff6e3d2bc66298cb0dd78bc4257542 Mon Sep 17 00:00:00 2001 From: Per Buer Date: Wed, 29 Jul 2026 21:56:55 +0200 Subject: [PATCH 12/12] docs: syscall fuzzing -- how to run it, what it found, what is open Records the ten fixes in this series, the five things deliberately left alone (SIGPIPE, the std::runtime_error taxonomy, clock_nanosleep, remote madvise isolation, callbacks returning without setting sysret), and the notes worth knowing: readv has no handler, poll/ppoll pointer alignment, and the unsoaked corners. Co-Authored-By: Claude Opus 5 (1M context) --- docs/syscall-fuzzing.md | 209 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 docs/syscall-fuzzing.md diff --git a/docs/syscall-fuzzing.md b/docs/syscall-fuzzing.md new file mode 100644 index 00000000..7eefe678 --- /dev/null +++ b/docs/syscall-fuzzing.md @@ -0,0 +1,209 @@ +# Fuzzing the syscall emulation layer + +`fuzz/syscall_fuzz.cpp` is a coverage-guided fuzzer for the host-side Linux +emulation in `lib/tinykvm/linux/`. It exists because that layer is the only +channel an untrusted guest has into the host process, and it is ~3300 lines of +hand-written pointer and length arithmetic over ~112 syscall handlers, every one +of which reads six fully guest-controlled 64-bit register arguments. + +```bash +fuzz/syscall_fuzzer.sh # build + run until interrupted +fuzz/syscall_fuzzer.sh -runs=1000000 # extra libFuzzer flags pass through +fuzz/triage_syscall_crashes.sh # group artifacts by root cause +``` + +Requires clang (libFuzzer), gcc (for the guest ELF), and `/dev/kvm`. + +## How it works + +The harness builds a master VM from a small static guest +(`fuzz/guest/syscall_fuzz_guest.c`), prepares it copy-on-write, and drives +`Machine::system_call()` **directly** on a fork — no VM entry, no trampoline. +The code under test is host code either way, so this runs at ~10k execs/second +under ASan+UBSan instead of the ~100/second a guest-driven harness would manage. +Each input is a sequence of records: + +- `POKE` writes fuzzer-chosen bytes into guest memory. +- `CALL` sets the six syscall argument registers and dispatches a syscall number. + +`POKE` is what makes the pointer arguments worth anything: most handlers read +structs out of the guest (`iovec`, `sockaddr`, `msghdr`, `pollfd`, `timespec`, +path strings), so the fuzzer has to control both the pointer *and* the bytes it +points at. Pointer arguments are drawn from a table of *interesting* guest +addresses — page boundaries, the last bytes of a mapping, one page past the end, +unmapped holes, non-canonical addresses — because uniformly random 64-bit +pointers just bounce off `copy_from_guest()` and coverage flatlines. + +The fork is reset from the master after every input, which also closes any file +descriptors the guest opened, so inputs stay reproducible. + +### Oracle + +- `MachineException` (and subclasses) escaping a handler is **correct** — that is + how the layer rejects a bad pointer or length. Caught and ignored. +- ASan/UBSan reports are findings. +- Any *other* `std::exception` escaping a handler is reported once per unique + type+message. An embedder that catches `MachineException` would let these + through into its own request loop. `TINYKVM_FUZZ_STRICT=1` turns them into + hard findings. + +### Sandboxing + +The handlers really do call `open`, `write`, `connect`, `mmap`. The harness +therefore: + +- rewrites every guest path to one of two files in a private temp directory, so + the whole open→manage→translate→read/write→close path is exercised with no way + to reach the rest of the filesystem; +- denies `connect`/`bind`/`listen`/`accept`, so sockets are created and tracked + but there is no egress; +- sets `O_NONBLOCK` on every fd the guest creates, so `read()` on an empty pipe + cannot wedge a worker; +- refuses a short list of syscalls that either block for a guest-controlled + duration or have side effects outside the sandbox (see `blocked_syscall()`). + +Note that `blocked_syscall()` is a *harness* concession, not a statement that +those syscalls are safe. As it stands, **none of the syscalls on that list has a +handler** — they all fall through to the unhandled path and return `-ENOSYS`, so +refusing them costs no coverage today and only guards against a future handler +that would block a worker or escape the sandbox. + +The two blocking handlers that *do* exist are covered rather than refused: + +- `accept4` — its `accept_callback` is consulted before the blocking `accept4()`, + and the sandbox installs one that declines, so the handler's prefix is still + exercised. +- `clock_nanosleep` — wrapped to clamp the *values* of the guest `timespec` in + guest memory to 1µs before running the original handler unmodified. The fuzzer + still picks the pointer, so the `copy_from_guest`/`copy_to_guest` edges (the + part worth fuzzing) stay reachable and only the duration is bounded. + +That clamp exists because the real handler passes a guest-controlled `timespec` +straight to `clock_nanosleep()`: a guest can pin a host thread for years, and +like finding #5 this sits inside the syscall handler where the execution timeout +does not reach. Unlike #5, bounding it changes guest-visible semantics (a guest +may legitimately want to sleep), so it is left as a decision — see below. + +## What it found + +First campaign, ~20M executions. All of these are reachable from an ordinary +unprivileged guest using legal syscalls, and all are fixed in the same series of +commits as this document except where noted. + +| # | Site | Bug | Impact | +|---|------|-----|--------| +| 1 | `getdents64` | host return value stored in `__u64 sysret()` then tested `> 0`, so `-1` became a ~2^64 `copy_to_guest` length | host stack streamed into guest memory, then a wild copy — **info leak + crash**, reachable via `getdents64` on any regular fd (ENOTDIR) | +| 2 | `sendmsg` | `msg_namelen` copied from the guest into a 128-byte `sockaddr_storage` with no bound | **guest-controlled stack buffer overflow** | +| 3 | `sendmmsg` | `vlen` checked only for `> 0` before being used as a copy length into a 64KB stack array | **guest-controlled stack buffer overflow** | +| 4 | `write`, `read`, `pread64`, `pwrite64`, `mmap`, `recvmsg`, `sendmsg`, `sendto`, `recvfrom`, `sendmmsg` | zero-length request gathers no buffers, then `buffers[0]` / `&buffers[0]` on the empty vector | `write(fd, p, 0)` — a legal no-op — **null-dereferences and kills the VMM**; UB elsewhere | +| 5 | `madvise(MADV_DONTNEED)` → `Machine::memzero` | guest length walked one page-table lookup per 4K, missing pages silently skipped, no bound | **unkillable host hang**: ~9M pages/s, so a 2^64 length pins the thread for years, and it is inside the syscall handler so the execution timeout does not apply | + +The clamp added for #5 bounds the walk to `memory.remote_end` (plus a connected +remote's range). That is the right bound for *virtual* addresses, which is what +`memzero` receives: `mmap_allocate()` hands out virtual addresses starting at +`heap_address` and well below `max_address` — `vMemory::MMAP_PHYS_BASE` +(0x4000000000) is the **physical** base of the mmap arena, not a virtual one, so +mmap'd guest memory is inside the clamped range and is still zeroed. Verified +both by measurement and by the positive +`madvise(MADV_DONTNEED) still zeroes in-range memory` test, which checks a full +range and a partial (middle-pages-only) discard. + +A second test connects a storage VM so `has_remote()` is true, which is the only +way to reach the remote-widening branch of that clamp. Be aware of what it does +and does not show: it proves the branch runs and does not break zeroing of the +VM's *own* memory. It cannot show the widening is *necessary*, since a narrower +bound would still cover own memory. Whether a guest should be able to +`madvise(MADV_DONTNEED)` a connected storage VM's pages at all is a separate +isolation question, and arguably the answer is no — worth settling before anyone +relies on the current behaviour. +| 6 | `prctl(PR_GET_NAME)` | `buflen` (up to 16) read straight out of the 8-byte literal `"tinykvm"` | OOB read; adjacent `.rodata` handed to the guest | +| 7 | `ppoll` | `ts.tv_sec * 1000` on a raw guest value | signed-overflow UB, then truncation to an arbitrary timeout | +| 8 | `recvmsg`, `recvfrom` | `socklen_t&` bound to a guest pointer of arbitrary alignment | UB; benign on x86-64, matters under stricter codegen | +| 9 | `openat` | non-zero `open_how.mode` passed for every write-open; `openat2` requires `mode == 0` without `O_CREAT` | functional: guests could **never** open an existing file for writing (EINVAL). Found while writing the regression test for #4, not by the fuzzer | +| 10 | `timerfd_create`, `eventfd2`, `inotify_init1` | failed host fd passed to `FileDescriptors::manage()`, which *throws* on a negative fd — so the `if (vfd < 0)` below each call was dead code | `std::runtime_error` escapes the handler; `timerfd_create`'s `clockid` and `inotify_init1`'s `flags` are guest-controlled, so any guest can trigger it. `epoll_create1` in the same file already had the correct shape | + +Regression tests for these are in `tests/unit/syscalls.cpp`. + +### Not fixed: guest-triggerable `std::runtime_error` + +Finding #10 is one instance of a wider issue. The layer's contract is that guest +misbehaviour surfaces as `MachineException` (`MemoryException` and +`RetryException` derive from it), which is what embedders catch. But several +paths throw plain `std::runtime_error` instead, and a guest can reach them at +will: + +- `futex()` with any unimplemented operation — + `throw std::runtime_error("Unimplemented futex op: N")` in + `linux/threads.cpp`. `FUTEX_REQUEUE` is enough. +- `FileDescriptors::manage()` on hitting `max_files`, and on a negative fd + (finding #10 was the reachable route to the latter). +- `FileDescriptors::translate_writable_vfd()` — "File descriptor is not + writable", reachable by writing to any read-only fd. + +Retyping these to `MachineException` is the obvious fix and is what the rest of +the layer does, but it is left alone here because it changes the library's +exception taxonomy — an embedder could plausibly be catching `std::runtime_error` +specifically today. Worth deciding deliberately rather than as a fuzzing +by-product. Until then, embedders should catch `std::exception`, not just +`MachineException`. + +The harness reports these once per unique type+message rather than treating them +as crashes; `TINYKVM_FUZZ_STRICT=1` promotes them to hard findings. + +### Not fixed: SIGPIPE kills the VMM + +```c +int fds[2]; pipe2(fds, 0); close(fds[0]); write(fds[1], buf, 16); +``` + +Three legal syscalls, and the host process dies with SIGPIPE. The emulation +layer passes `MSG_NOSIGNAL` on the `sendmsg`/`sendto` paths, so the hazard is +clearly known, but plain `write()`/`writev()`/`pwritev64()` have no equivalent +and pipes cannot be protected that way at all. + +This is left as a deliberate decision rather than a patch, because every fix +changes something outside the library's own scope: + +- `signal(SIGPIPE, SIG_IGN)` in `Machine::init()` is what most servers want, but + silently changing a host process's signal disposition is a side effect a + library arguably should not impose (the `src/` example CLIs would stop dying on + `EPIPE` when piped into `head`). +- Using `send(..., MSG_NOSIGNAL)` for socket fds would need `FileDescriptors` to + actually retain the `is_socket` flag it is currently passed and discards, and + still leaves pipes exposed. +- Blocking SIGPIPE around each write is correct but costs a syscall pair per I/O. + +Until it is decided, **embedders must ignore SIGPIPE**. The fuzz harness does so +itself, with a comment pointing here. + +## Notes for future work + +- `writable_memarray(guest_ptr, guest_count)` in `poll`/`ppoll` binds a `T*` + to a guest-chosen address with a guest-chosen count. The count is bounded by + the mapping check inside `writable_memview`, but the *alignment* is not, which + is the same UB as finding #8. Fixing it means either copying in/out or + rejecting misaligned arrays with `-EFAULT`; the latter changes guest-visible + semantics, so it needs a decision. +- `poll`/`ppoll` honour a guest timeout verbatim on a forked VM, so they can + block a worker indefinitely. The harness neutralises this with a + `poll_callback` returning false; production embedders should bound it. +- `poll`, `ppoll`, `epoll_wait` and `accept4` all `return` early when their + callback declines, *without* setting `sysret()`. The guest therefore reads + whatever was already in the return register — at syscall entry on AMD64 that is + the syscall number, so e.g. a declined `accept4` looks like it returned fd 288. + This is presumably fine when the embedder's callback also pauses or reschedules + the guest, but it is an implicit part of the callback contract that is not + documented anywhere, and is a trap for a new embedder. +- `readv()` (syscall 19) has no handler at all, so it returns `-ENOSYS`, while + `writev()` is fully implemented. Noticed while writing the positive + round-trip test in `tests/unit/syscalls.cpp`. Not a safety issue — a guest + just sees ENOSYS — but the asymmetry is surprising, and glibc does use `readv` + in some configurations. +- `TINYKVM_FUZZ_UNSAFE=1` installs the "unsafe" set — `symlink`, `fchdir`, + `io_uring_setup`, `inotify_init1`, `inotify_add_watch`. It has only been + smoke-tested (20k execs), which is what surfaced finding #10 via + `inotify_init1`. The path-taking handlers there are properly gated through + `is_writable_path`/`is_readable_path`, and `io_uring_setup` is an `-ENOSYS` + stub, so the surface is small — but it deserves a real campaign. +- ARM64 is untested here; the harness itself is arch-neutral but has only been + run on AMD64.