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/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. 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 diff --git a/lib/tinykvm/linux/system_calls.cpp b/lib/tinykvm/linux/system_calls.cpp index 5937613e..8eee16a2 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()); @@ -351,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)) @@ -441,7 +464,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 +764,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 +792,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 +1467,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 +1531,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; @@ -1519,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); @@ -1578,7 +1607,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; @@ -1599,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); @@ -1659,12 +1693,23 @@ 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; 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; @@ -2193,7 +2238,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; } @@ -2263,10 +2312,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", @@ -2419,9 +2478,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)); @@ -2511,11 +2574,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}); @@ -2530,11 +2596,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}); @@ -2874,6 +2944,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 @@ -2907,7 +2988,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; @@ -3257,12 +3338,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/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/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..ca9de020 --- /dev/null +++ b/tests/unit/syscalls.cpp @@ -0,0 +1,531 @@ +#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); +} + +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("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 + 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); +} + +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); +} + +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); +} + +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("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 + 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); +}