fix(security): validate syscall buffers against the caller VAS - #890
Merged
Conversation
added 5 commits
August 20, 2026 22:33
`memguard::validate_user_buffer` checked only that a numeric range lay between `KERNEL_END` and `RAM_END`. Syscalls run at PL1 over an identity-mapped DRAM, so a numerically valid pointer could reference another process's pages, allocator and kernel structures, a PROT_NONE page, or a read-only mapping that PL0 could not touch. Every fd, socket, pipe, futex and time syscall treated that bounds check as authority before dereferencing at PL1. Add `validate_user_range(ptr, len, Access)`, which keeps the numeric check as a cheap first gate and then walks the range page by page against the calling process's own L2 descriptors, requiring PL0 permission in the requested direction. Direction is the second half of the fix: a read-only user mapping is a legitimate source and an illegitimate destination, and the previous guard accepted both. Permission is decided by two questions, not one. `l2_entry_is_user` answers ownership via the software NG tag; the AP/APX field answers reachability. A PROT_NONE user page is user-owned and carries AP_KERNEL_ONLY, so it is indistinguishable from a kernel fill by AP alone and from a readable user page by ownership alone. Migrated every syscall caller with an explicit direction: fd read/write, stat, getcwd and the path arguments; socket bind/connect/sendto/recvfrom and both sockaddr writebacks; pipe read and write; futex wait; the IPC payload, UART write, execve path and argv; and both time syscalls. `sys_execve` also validated only the first byte of each argv string while scanning up to MAX_ARG_LEN - 1 bytes, so a string starting near the end of a mapping was scanned past it. It now validates the full scan window, matching the conservative policy `path_ptr` already used. `elf.rs` deliberately keeps the numeric guard. It places an image for a process that has no page table yet, through the kernel identity map, so a caller-VAS check there would test the loading process's mappings against the loaded process's addresses and reject every valid segment. The mapping gate is conditional on a user address space, determined positively by a new `process::current_user_page_table` — a non-zero PID holding a table that is not PID 0's kernel global L1. A PL0 caller always has one, so the numeric-only path is unreachable from userspace. A host build has no MMU and no user address space, which is why the permission logic and the range walk are factored as pure functions and tested directly rather than through the live walk. Fifteen tests, built around the cases that would otherwise pass: a PROT_NONE page accepted by an ownership-only check, a kernel page with permissive AP accepted by an AP-only check, AP_FULL with APX set read as writable, a hole after a valid first page, a read-only page late in a write span, and both unaligned span ends. Refs #871
Four fd fixtures failed once syscall buffers were validated against the caller's page tables: fd_isolation_cross_process_ops_return_ebadf, fork_shares_ofd_offset_advances_together, fork_then_open_is_independent_ofd, and getcwd_reports_the_current_process_cwd. Each installs a real process with `set_current_for_test` and then hands a syscall a host static. That worked while the guard was numeric only. It stops working the moment the guard asks the caller's page tables, because the spawned or forked process owns an address space in which none of those statics is mapped, so the calls return EFAULT before reaching the fd-layer behaviour the test is about. The fixtures were modelling a process without modelling its memory. On the device those same calls would also fault, so the tests were asserting an outcome the hardware would not produce. `map_user_buffer_for_test` maps the buffer a fixture is about to pass, identity-mapped and user read/write, and panics rather than returning failure so a fixture that cannot map its own buffer does not surface as an EFAULT blamed on the code under test. Only switches to a child or spawned pid need it. Switching back to pid 0 returns to the kernel global L1, where `current_user_page_table` reports no user address space and the numeric gate is the whole check. Refs #871
…ocess `socket_isolation_cross_process_ops_return_ebadf` is the same fixture gap the fd tests had, and it only surfaced once those were fixed — the earlier failure stopped the job before this test ran. It spawns a process, makes it current, and then hands sendto, recvfrom and bind three host pointers the spawned process does not map. Each now returns EFAULT before the fd-isolation assertion it exists to make. Swept every `set_current_for_test` call site rather than fixing this one instance: thirteen sites, of which the switches back to pid 0 stay on the numeric path, two pass no pointers at all (`with_current_cwd`, `sys_dup`), and the rest are the five already mapped. That is the whole class. Refs #871
The armv7a build fails `-D warnings` on an unused import: after every syscall path moved to `validate_user_range`, `validate_user_buffer` has no production caller in this module. Only its own tests still call it. Invisible to the i686 test profile, where the import is used, so the two profiles disagreed and the error surfaced only once the host-test failures ahead of it cleared. Gated `#[cfg(test)]`, matching the `board` import three lines above. Refs #871
The kernel zero-warning clippy gate fails all nine feature configurations
on `unused_unsafe` at fd.rs:2980, fd.rs:3043 and socket.rs:1911 — the
three `addr_of!` calls added while mapping fixture buffers.
Taking the address of a static is safe; only dereferencing it is not. The
surrounding code's `unsafe { &*core::ptr::addr_of!(X) }` needs the block
for the `&*`, and copying that shape to a bare `addr_of!` carried the
block along with it.
Refs #871
This was referenced Aug 21, 2026
forkwright
pushed a commit
that referenced
this pull request
Aug 21, 2026
🤖 I have created a release *beep* *boop* --- ## [0.8.5](v0.8.4...v0.8.5) (2026-08-21) ### Bug Fixes * **security:** validate syscall buffers against the caller VAS ([#890](#890)) ([9fe76e8](9fe76e8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
forkwright
added a commit
that referenced
this pull request
Aug 21, 2026
…te (#892) ## Summary Closes the validation half of #868. `sys_pipe` checked `fds_ptr` for null and nothing else, then wrote two `u32` file descriptors through it. The already-corrected `sys_pipe_read`/`sys_pipe_write` paths in the same module validated their caller buffers; the constructor did not — so a caller could aim both writes at kernel, MMIO, unmapped, read-only or boundary-crossing memory. It now validates the full eight-byte writable range through `memguard::validate_user_range(.., Access::Write)`, the mechanism #890 landed for #871: the numeric gate plus a walk of the caller's own page tables requiring PL0 write permission. ## The ordering is the property, not the check Validation runs **before** the pipe slot, both open-file descriptions and both fd-table entries are created. Validating late would return exactly the same `EFAULT` while leaving five objects to unwind — which turns a bad pointer into a cheap way to churn the pool, and depends on five rollback paths staying correct forever. Rejecting first means a refused call costs nothing and leaves nothing behind. Because both orderings produce the same errno, the test that carries this fix asserts **the pipe-pool occupancy after a refused call**, not the return value: ```rust assert_eq!(sys_pipe(crate::board::KERNEL_LOAD as u32), EFAULT); assert_eq!(occupied_slots(), 0, "a refused pipe() must leave no pipe slot behind"); ``` A suite that only checked the errno would pass against the defect. ## Adversarial range coverage Null, device MMIO, kernel image, a range **straddling `RAM_END`** whose first word is in range, and a wrapping range. The straddling case is the one a first-address-only or single-`u32` check accepts and then writes past the end of RAM. The separate null check is removed rather than retained: the numeric gate inside `validate_user_range` already rejects null, and two answers to one question is how they drift apart. ## Fixture change, and why it was predictable Two tests moved from stack arrays to function-local `static mut`. This binary's PIE image — hence any `static` — loads inside `[KERNEL_END, RAM_END)` on the host toolchain, while the per-test-thread stack sits above `RAM_END`, so a stack buffer is refused before `pipe()` runs. This is the same fixture class #890 hit, and it is called out here because it was anticipated from `fd.rs`'s existing comment rather than discovered in CI. The fd, socket and futex tests already use this convention. ## What #868 still owns The rollback paths were already complete and are unchanged; this PR makes them unreachable for the bad-pointer case rather than rewriting them. Still outstanding on the issue: the permission-transition race between validation and the write (owned by #871's fault-contained copyout), and a target witness — host tests have no MMU and no user address space, so they exercise the numeric gate only. `docs/target-test-ledger.toml` now says that in the `pipe` fidelity note instead of the previous text, which described the defect this PR fixes. No hardware, firmware, or raw-device operation is involved. Refs #868 --------- Co-authored-by: forkwright <cody@forkwright.com>
This was referenced Aug 21, 2026
forkwright
added a commit
that referenced
this pull request
Aug 26, 2026
Closes #871 ## Summary - Centralize syscall user-memory access in direction-aware `copy_from_user` / `copy_to_user` primitives that validate the complete overflow-checked range against the live caller VAS and PL0 permissions. - Use exact ARM unprivileged-transfer fixup sites so a post-validation `LDRBT` or `STRBT` abort becomes `EFAULT`; unrelated PL1 faults still halt. - Migrate fd, pipe, socket, futex, time, IPC, UART, exec/argv, stat/getcwd, and signal-frame pointer families. Untrusted counts use bounded transport-sized chunks, and stateful reads use peek/copy/commit or rollback. - Make private futex wait queues VAS-scoped, harden exec argv and signal-stack arithmetic against wraparound, and return `EFAULT` for failed futex copyin. - Add a real QEMU syscall permission/fault matrix, raw fixup probes, and the existing kernel-fault negative control to CI and the target ledger. ## Acceptance evidence The QEMU uaccess witness exercises a live caller page table rather than a host predicate: - mapped anonymous RW copyin and copyout succeed; - a read-only page succeeds as a source and fails as a destination; - `PROT_NONE`, unmapped, stale-after-`munmap`, and cross-page partial ranges return `EFAULT`; - ARMv7 execute-only mappings are refused (`EINVAL`) because this MMU cannot express execute-without-data-read, while the pure descriptor tests reject execute-only data access; - raw `LDRBT` and `STRBT` faults bypass prevalidation and recover only at the registered transfer PCs; - the caller continues into the service loop after contained syscall failures; - the separate `kfault` witness still halts on an unrelated PL1 undefined instruction (runner rc 4). Cross-process and transition coverage includes distinct-L1 address-space isolation, same-VA/different-VAS futex ownership, caller-owned fd/socket fixtures, forked-child copyout, independent exec replacement, fork-then-exec copyin in both old and replacement VASes, and real target `mprotect` / `munmap` transitions. Pipe publication and read consumption occur only after successful copyout, satisfying #868's no-side-effect failure boundary. ## Verification - `env -u CARGO_TARGET_DIR scripts/kernel-clippy.sh` — all nine ARM feature configurations passed. - `env -u CARGO_TARGET_DIR scripts/kernel-host-tests.sh` — 2,571/2,571 default and 2,575/2,575 debug-console passed; one intentional skip in each. - `scripts/check-target-test-ledger.sh` — 129 rows, 29 both-mechanism, 9 target-only, 2,576 runnable host tests, no drift. - `scripts/check-witness-extraction.sh`, `cargo fmt --all --manifest-path crates/thumos/Cargo.toml -- --check`, `git diff --check`, and changed-shell syntax — passed. - `env -u CARGO_TARGET_DIR scripts/witness/uaccess.sh` — passed. - `env -u CARGO_TARGET_DIR scripts/witness/kfault.sh` — passed with expected runner rc 4. - `env -u CARGO_TARGET_DIR scripts/witness-run-all.sh` — `ALL KERNEL QEMU WITNESSES: PASS`, including fork, exec, and forkexec. - `kanon gate --full --git-diff origin/main --stamp .` — workspace check, dependency audit, clippy, and 997/997 nextest passed; only Kanon lint failed. ### Kanon #756 baseline receipt A fresh linked worktree at exact base `a6f855bcc54b2083d05b274f4aa2a3654dadb4a6` reproduced exactly the branch result: 163 violations, 28 suppressed, strict-equivalent 191, with identical rule counts. This is the known repository lint debt tracked by #756. Per the documented exception, the branch was pushed with `--no-verify`; no `Gate-Passed` trailer was fabricated. ## Truthful residuals - UDP `recvfrom` prevalidates both outputs and preserves the datagram on a payload transfer fault, but a late payload fault can leave the already-copied source-address bytes in user memory; the pair is not byte-atomic. - `/dev/urandom` preserves file data/offset behavior, but its volatile PRNG state may advance if a post-validation copyout fault occurs. - The current four-register dispatch ABI supplies zero address arguments to `sendto` / `recvfrom`; direct helpers support address buffers. The existing `addr_len` output and `execve` `envp` remain unimplemented/ignored rather than being newly claimed here. - #890 supplied the earlier caller-VAS validation and its host-fixture corrections; this PR closes the remaining fault-containment, race, migration, and target-witness scope. - No #929 KDF, secrets, encryption, or kinit files are touched. --------- Co-authored-by: admin <admin@ardentleatherworks.com>
forkwright
added a commit
that referenced
this pull request
Aug 26, 2026
## Summary - run the canonical source-only wiring inventory check inside the branch-protection-required kernel context before the docs-only exemption, with checkout available on every path - remove the duplicate check from unrequired rustfmt while preserving the guarded full post-boot checker - add one deterministic topology regression that invokes the canonical checker against a malformed isolated inventory and proves later build/QEMU steps remain docs-only guarded - refresh the kernel-core inventory row to describe the uaccess work landed by #890/#952 and #868, including the remaining non-atomic/state/ABI residuals rather than treating #871 as a live owner ## Rationale The admission property belongs to the already-required kernel context: a malformed docs-only inventory must make that context red without paying for kernel build or QEMU. The regression checks workflow composition and reuses `scripts/check-wiring-inventory.sh --no-log`; it does not create a second inventory checker. The general closed-owner freshness mechanism is already tracked by [Kanon #3709](forkwright/kanon#3709), so this repair corrects the stale row without adding a competing Thumos mechanism. ## Verification Exact remote head: `739d67173f040cd5fc7ab173d9b8cb1bf27e08c2` At the authored repair commit, the ordinary pre-push hook passed fmt, workspace check, fitness, cargo-deny, workspace clippy, and nextest (997/997). It stopped only on the known repository-wide Kanon lint baseline. After the additive merge of current main, an apples-to-apples linked-worktree comparison was exact: branch and current main each report 163 violations, 28 suppressed findings, strict equivalent 191, with identical rule distributions. The one non-force `--no-verify` feature-branch push was explicitly authorized from that immutable receipt. Exact-head scoped receipts: - `bash -n` and `shellcheck` on `scripts/check-docs-only-kernel-gate.sh` - topology/malformed negative fixture passes and invokes the canonical inventory checker - `scripts/check-wiring-inventory.sh --no-log`: 123 modules / 30 capabilities / 23 witness markers / 9 compiled-only owners - doc inventory, witness extraction, and kernel build-entrypoint checks pass - Actionlint and TOML parsing pass - `kanon gate --tier=nobuild --force --paths ...`: 0 findings - `git diff --check`: clean No `Gate-Passed` trailer is asserted; hosted full admission remains authoritative. Closes #944. Co-authored-by: admin <admin@ardentleatherworks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the syscall-isolation hole in #871:
memguard::validate_user_bufferchecked only that a numeric range lay betweenKERNEL_ENDandRAM_END. Because syscalls execute at PL1 over an identity-mapped DRAM, a numerically valid pointer could reference another process's pages, allocator and kernel structures, aPROT_NONEpage, or a read-only mapping PL0 could not touch — and every fd, socket, pipe, futex and time syscall treated that bounds check as authority before dereferencing.validate_user_range(ptr, len, Access)keeps the numeric check as a cheap first gate, then walks the range page by page against the calling process's own L2 descriptors, requiring PL0 permission in the requested direction.Two questions, not one
Permission needs both halves, and each alone admits the other's failures:
l2_entry_is_useranswers ownership (the softwareNGtag).AP/APXanswer reachability.A
PROT_NONEuser page is user-owned and carriesAP_KERNEL_ONLY— so it is indistinguishable from a kernel fill by AP alone, and indistinguishable from a readable user page by ownership alone. Direction is the other half: a read-only mapping is a legitimate source and an illegitimate destination, and the old guard accepted both.Why the mapping gate is conditional, and why that is not a bypass
The gate asks what the caller's page tables say, which only has an answer while a user process is current. A new
process::current_user_page_tabledecides that positively — a non-zero PID holding a table that is not PID 0's kernel global L1. A PL0 caller always has one by construction, so the numeric-only path is not reachable from userspace.A bare
current_page_table() != 0would have been wrong here: PID 0 is a real entry whose table is the kernel L1, so that test would validate user pointers against kernel mappings — accepting exactly the addresses the check exists to reject.elf.rsdeliberately keeps the numeric guardIt places an image for a process that has no page table yet, through the kernel identity map. A caller-VAS check there would test the loading process's mappings against the loaded process's addresses and reject every legitimate segment. The reason is recorded at the call site so it does not read as an oversight.
sys_execveargvValidated only the first byte of each argv string while scanning up to
MAX_ARG_LEN - 1bytes, so a string starting near the end of a mapping was scanned past it — the instance recorded in #871's own follow-up comment. It now validates the full scan window, the same conservative policypath_ptralready used.Testing
Host builds have no MMU and no user address space, so the permission logic and the range walk are factored as pure functions and tested directly rather than through the live walk. Fifteen tests, built around the cases that would otherwise pass silently:
PROT_NONEpage accepted by an ownership-only checkAP_FULLwithAPXset, which reads as writable but is notDescriptors are minted by the real
prot_to_l2_flags, the sole producer of user L2 entries, rather than hand-assembled.docs/target-test-ledger.tomlis updated: memguard 13 → 28 tests, and it now carries a fidelity note, because addingl2_entry/page_tablereferences makes the module target-sensitive by the ledger's own pattern. The note states plainly what host tests cannot reach — the live walk — and that a target witness proving aPROT_NONEbuffer is refused on device is still owed.What this does not do
#871 stays open. Its remaining acceptance is fault containment (an invalid address must return
EFAULTrather than taking a privileged fault), centralized copyin/copyout, the permission-transition race cases, and the target witness above. This closes the validation clause; it does not close the issue.No hardware, firmware, or raw-device operation is involved. Builds and tests run on GitHub CI.
Refs #871