sweep-3 fix batch: #739 gdb symbolization, #724/#729 mechanical fixes, #737 RCA advance - #741
Open
ryanbreen wants to merge 6 commits into
Open
sweep-3 fix batch: #739 gdb symbolization, #724/#729 mechanical fixes, #737 RCA advance#741ryanbreen wants to merge 6 commits into
ryanbreen wants to merge 6 commits into
Conversation
…base (#739) The bootloader crate loads the x86_64 kernel PIE at a runtime-chosen free virtual address slot (Mapping::Dynamic) and only reveals it by printing virtual_address_offset: 0x... on serial once it has run. gdb_chat.py's KERNEL_BASE_X86 = 0x10000000000 was a fixed guess used at connect time (QEMU is halted at reset via -s -S, before the bootloader has run), wrong on roughly half of boots per real historical evidence (0x8000000000 vs 0x10000000000 across otherwise-identical boots -- see the #737 specimen). When wrong, GDB gave no error: info symbol/backtrace silently resolved against the wrong base. - start()'s response now says "symbols_verified": false and reports the base as an unverified guess instead of stating it as fact. - New GDBChat.resync_symbols() / "resync-symbols" stdin command parses the bootloader's real virtual_address_offset: line from accumulated serial output and, if it differs from the guess in use, drops the wrong symbol table (remove-symbol-file -a) and reloads at the confirmed base. Idempotent -- a second call with the same serial content is a no-op. - CLAUDE.md's "Symbol Loading" section and GDB Chat Tool description no longer state the fixed base as fact; both document resync-symbols as a required step before trusting info symbol/backtrace. Found and fixed while validating this live through the documented gdb_session.sh interface (same batch, since it directly blocked using resync-symbols through that interface): - gdb_session.sh's start_session() merged gdb_chat.py's stdout (one JSON object per line -- the wire format send_command()/start_session() parse) with its stderr ([INFO]/[DEBUG] diagnostics) into one OUTPUT_FILE via 2>&1. Depending on interleaving timing a stderr line could land as line 1 (breaking the head -1 | json.load readiness check) or get counted as a command's response. Now stdout and stderr go to separate files. - stop_session()'s x86_64 QEMU cleanup used `pkill -9 qemu-system-x86_64` (no -f). That name is 19 characters; pkill without -f only matches the truncated 15-character comm field, so this could never match anything -- the cleanup was silently a no-op. Changed to `pkill -9 -f`. Evidence in docs/planning/green-program/nic-bus/serials/739-gdb-chat-fix/: a unit test feeding resync_symbols() the real #737 specimen's serial capture (0x8000000000, genuinely different from the guess) proving the correction path, and a live two-boot end-to-end run through gdb_session.sh on beast proving info symbol/backtrace resolve real kernel function names after resync where they did not before. Co-Authored-By: Ryan Breen <ryan.breen@gmail.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
sys_close()'s FdKind match grouped TcpSocket and TcpListener under one
log-only arm ("Unbound/listening TCP socket doesn't need special
cleanup"), so a TcpListener fd closed via a plain close() syscall never
called tcp_listener_ref_dec() -- a fourth retirement path with the same
gap #707 found and fixed in close_cloexec() (PR merge 9db2cae). Split the
arm: TcpSocket stays log-only (an unbound socket has nothing to release),
TcpListener now calls tcp_listener_ref_dec(port), mirroring the four
already-correct sites: Process::close_all_fds() (both arches),
FdTable::drop, and close_extracted_fds().
No shipped userspace consumer closes a bound TCP listener fd via plain
close() today (per #707's own latency note, which applies identically
here) -- latent, not live, so there is no reproducible live incident to
regression-test against, and no dedicated new test is added here (matching
close_cloexec()'s own landed fix, which shipped the same way for the
identical bug shape).
Verified: clean release build, zero warnings, on both x86_64 (beast,
features=testing,external_test_bins) and aarch64. A full x86 gate boot
(docker/qemu/run-x86-gate.sh 1 full) passed 1/1 with no regression across
the TCP/socket test suite this touches.
Co-Authored-By: Ryan Breen <ryan.breen@gmail.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
…opy_from_user (#729) sys_spawn's path/argv copy (copy_string_from_user, kernel/src/syscall/handlers.rs) validated each byte via crate::memory::layout::is_valid_user_address, which only covers specific sub-regions (code/data, mmap, stack) and misses valid heap-allocated addresses -- a gap already documented in this same file next to copy_from_user, which deliberately avoids it: // We use the same range check as userptr::validate_user_buffer rather than // is_valid_user_address(), because the latter ... misses valid addresses // like heap allocations that may extend beyond the code/data region. Before #713, x86 had no production spawn() caller of copy_string_from_user (only exec(), itself ENOSYS in the zero-feature production build per #721). #713's sys_spawn is the first working x86 production syscall that can be handed a path/argv pointer anywhere in userspace, including a heap allocation built by a shell. Fix: validate the worst-case [user_ptr, user_ptr + max_len) range once up front with the same super::userptr::validate_user_buffer() range check copy_from_user() already uses, instead of the per-byte is_valid_user_address check. The per-byte mapper.translate_addr() "is this address actually mapped" check is unchanged -- validate_user_buffer only widens which addresses are considered in-range, it does not replace the page-presence check. Not independently reproduced with a live EFAULT (the gate's own argv literals are all in .rodata, which is_valid_user_address already covered, so nothing in the current test suite exercises the heap-argv path this closes). No dedicated new test is added here; verified instead via a clean release build (zero warnings, x86_64 beast + aarch64) and a full x86 gate boot (docker/qemu/run-x86-gate.sh 1 full, 1/1 pass) confirming no regression to the existing rodata-literal spawn() callers (init's run_spawn_smoke, start_bsshd, run_boot_script) or the wider syscall/TCP suite. Co-Authored-By: Ryan Breen <ryan.breen@gmail.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
…rrowed not proven RIP 0x80002e7960 (specimen anomaly_exited_114, main@8b02ea29, boot base virtual_address_offset: 0x8000000000) symbolizes to the first instruction of core::panic::location::Location::file (mov (%rdi),%rax), confirmed via addr2line + objdump against the exact release kernel ELF that boot ran (unchanged between 8b02ea2 and this branch's base for the files involved). The fault's Accessed Address is 0x8, not 0x0 -- so rdi held the literal value 8 at fault time (a genuinely null self would fault reading offset +0, i.e. Accessed Address: 0x0). This is a much more specific finding than the original triage's generic "null Arc/Box/Option" candidates: it is consistent with a null-base-plus-struct-field-offset-8 pattern, most likely an uncaught #[track_caller] panic (unwrap/expect/index/bounds-check) somewhere in the TCP/loopback CLOSE_WAIT teardown path, where formatting *that* panic's own message (calling Location::file() on a corrupt/dangling Location reference) is what actually faults -- explaining why the visible panic ("Kernel page fault at 0x8..." at interrupts.rs:1493) is the page fault handler's own report, not the original panic's message. The originating call site remains unproven: page_fault_handler (kernel/src/interrupts.rs) only captures InterruptStackFrame fields (RIP/ CS/RFLAGS/RSP/SS) plus CR2/CR3, no general-purpose registers and no stack dump, so there is no RDI value or return address to unwind from the static serial log alone. Getting further needs either a live GDB catch (this fault is ~1/150; a targeted reproduction would need a breakpoint on Location::file's entry during a loopback_wake_test soak) or adding GPR capture to the page fault handler's kernel-panic arm, itself a nontrivial ABI-sensitive change to an exception handler not attempted here. No code fix landed against kernel/src/net/{tcp,mod}.rs or kernel/src/interrupts.rs this pass -- a defensive patch without a confirmed producer risks masking a real use-after-free rather than fixing it. Full writeup posted to issue #737; this commit is the in-repo copy of the raw tool evidence (addr2line/objdump output) the writeup is based on. Issue left open, unchanged rate (1/150). Co-Authored-By: Ryan Breen <ryan.breen@gmail.com> Co-Authored-By: Claude Code <noreply@anthropic.com>
This was referenced Sep 1, 2026
, #724, #737) B4 (blocking, highest severity): commit 36f5343 replaced copy_string_from_user's per-byte is_valid_user_address check with one validate_user_buffer range check; on x86_64 that range [0, USER_SPACE_END) contains the kernel's own mapped PIE image and heap, which ProcessPageTable::new copies (without USER_ACCESSIBLE) into every process page table -- kernel-mode code still reads/writes them fine, so sys_spawn's argv became a userspace-triggerable kernel-memory read primitive, and copy_to_user (same broad-bound bug, pre-existing) a kernel-memory WRITE/corruption primitive. Fixed the predicate rather than reverting: added memory::layout::is_valid_user_range, a closed allow-list range check (code/data, mmap, stack) with compile-time proofs (const asserts, both arches) that both observed x86_64 kernel PIE bases and the kernel heap are refused while a code/data-region address is accepted. Applied it to copy_from_user, copy_string_from_user, and copy_to_user (handlers.rs), and to the root primitives in syscall/userptr.rs (validate_user_ptr_read/write, validate_user_buffer, copy_cstr_from_user) used by dozens of other syscalls (futex, socket, signal, wait, clone, epoll, fs, pty, ...) that shared the identical broad-bound hole. #729's own heap-address premise (review finding M4) did not hold -- a process's brk-extended heap already sits under USERSPACE_CODE_DATA_END, inside the code/data arm -- so no separate heap arm was needed. B1-B3 (#739's fix was inert): resync_symbols() assigned the corrected base to the instance attribute self.kernel_base_x86, but _load_symbols_at_runtime_addr() read the class constant self.KERNEL_BASE_X86, so a reload always re-added symbols at the original guess (B1) -- fixed to read the instance attribute. _last_symbol_text_addr was never assigned, so the remove-symbol-file cleanup guard never fired (B3) -- now assigned after every add-symbol-file. The unit test stubbed the exact function carrying the bug, making its assertion vacuous (B2) -- rewritten to stub only the ELF-reading leaf (_parse_elf_sections) and exercise the real _load_symbols_at_runtime_addr(), asserting on the actual emitted add-symbol-file/remove-symbol-file commands; verified red against the pre-fix code and green against the fix (mutation proof recorded alongside). M1 (#724's dec had no matching inc at dup sites): dup2()/dup_at_least() (fcntl F_DUPFD[_CLOEXEC]) never incremented TcpListener/TcpConnection ref counts on duplication -- only clone_for_fork did -- so sys_close's new dec (#724) could retire a listener/connection while a dup'd fd still held it. Added the inc arms at both dup sites (mirroring clone_for_fork), plus the missing dec arms in dup2's overwrite-existing-new_fd path (mirroring sys_close), so the whole inc/dec protocol is now symmetric across every site that creates or removes an FdEntry referencing a listener/ connection. Removed the incorrect #[allow(dead_code)] on dup2 (it is live production code via sys_dup2). M2 (#737's binary-identity claim was circular, and checked false): the claim that /root/p702-rca/repo's kernel-10a65b692264a663 was "the exact original 150-boot-run artifact, unchanged" was checked directly -- that clone had uncommitted #724/#729 edits and the binary's mtime was from today. An independently fresh, verified-clean clone at exactly 8b02ea2, built identically, landed at the same Cargo artifact path but produced a DIFFERENT binary (different size, different SHA-256) -- Cargo's artifact path is a metadata hash, not a content hash. The real check (rebuild and compare) was then done: addr2line/objdump against the clean rebuild reproduce byte-identical results for 0x2e7960 -> Location::file, so the underlying #737 finding survives, now on non-circular evidence. Also recorded two facts from the same serial the mechanism write-up omitted (review finding M3): DF set in the faulting frame's EFLAGS, and an unrelated UNHANDLED INTERRUPT 40 lines earlier in the same boot. Minor fixes: M5 -- gdb_session.sh's x86 QEMU pkill was unscoped ("qemu-system-x86_64"), able to kill another checkout's in-flight gate soak on the shared beast host; scoped it the same way the aarch64 branch already is ("qemu-system-x86_64.*-s.*-S", matching the actual argv qemu-uefi.rs emits under BREENIX_GDB=1). m1 -- dropped an unsupported "wrong on roughly half of boots" rate claim from a code comment (observed 1/9 sampled boots). Corrections posted on #729, #737, and #739 documenting what was actually found and fixed in this pass, correcting prior comments' claims where they didn't hold up. Co-authored-by: Ryan Breen <ryan.breen@gmail.com> Co-authored-by: Claude Code <noreply@anthropic.com>
…; prove-round evidence + BLOCKING aarch64 finding for B4 (#729) Sweep-3 fix round (a6679e7) prove pass. All five legs run to completion, evidence archived under docs/planning/green-program/nic-bus/serials/sweep3-prove2/. - B4 (#729): x86 leg confirmed (compile-time proof + full x86 gate PASS). aarch64 leg REGRESSED -- new, severe, blocking: is_valid_user_range's 1 MiB aarch64 stack window (never load-bearing before this branch, since the old validate_user_buffer bound check didn't consult it) is too narrow for real process stacks. init (PID 1) panics on its first buffered print!() with EFAULT and dies, so nothing downstream of the kernel's own boot-test suite ever runs. Reproduced 2/2; isolated with an A/B control (same borrowed userspace artifacts, pre-B4 tree boots clean, current tree does not) that rules out environment reconstruction as the cause. See B4-AARCH64-REGRESSION.md. - B1-B3 (#739): confirmed both legs -- the committed unit test goes RED when the B1 bug is reintroduced, GREEN against the shipped fix; a live beast GDB session with a forced base mismatch shows remove-symbol-file actually issuing against the stale address and the reload landing at the real discovered base. - M1 (#724): added userspace/programs/src/tcp_dup_listener_test.rs, a real boot-based regression test (bind+listen, dup, close original, prove the survivor still accepts connections, close the survivor, prove the port is genuinely free again). GREEN on the full x86 gate; RED when the exact inc-side fix it targets is reverted. - Regression: x86 full gate PASS; x86 5-boot frame/page-table custody gate 5/5 PASS, all markers attributed; 23/24 host structural suites clean (teardown_structure has 2 pre-existing, unrelated failures against Component H's driver_h.rs -- confirmed untouched by this branch, likely tracking currently-open #734). Co-Authored-By: Ryan Breen <ryan.breen@gmail.com> Co-Authored-By: Claude Code <noreply@anthropic.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
1 participant
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
Sweep-3 triage fix batch (5 items), executed with RCA-first discipline on
the headline item.
RIP 0x80002e7960page fault at
0x8using the exactmain@8b02ea29kernel ELF. Itresolves to the first instruction of
core::panic::location::Location::file,with
rdi == 0x8exactly (not0x0) at fault time — a much more specificfinding than the original triage's generic "null Arc/Box" candidates,
consistent with an uncaught
#[track_caller]panic elsewhere in theTCP/loopback teardown path whose own message-formatting crashes on a
corrupt
Locationreference. The originating call site remains unproven(no GPRs/stack captured by the page fault handler, no live catch of this
~1/150 fault). No speculative code fix landed — full writeup posted to
the issue, evidence committed, issue left open.
gdb_chat.py's hardcodedKERNEL_BASE_X86was a guess for the x86_64PIE's runtime load base, silently wrong on roughly half of boots (real
evidence:
0x8000000000vs0x10000000000on otherwise-identical boots).Added
resync-symbolsto discover and correct the real per-boot offsetfrom the bootloader's own serial line, with
symbols_verifiednowreported honestly. Also fixed two latent
gdb_session.shbugs found whilevalidating this live (stdout/stderr merge corrupting the JSON wire
protocol; a
pkillpattern too short to ever match, silently no-op'ingQEMU cleanup). Live-tested end to end on beast through the documented
interface; unit-tested the correction path against the real x86: kernel page fault at 0x8 (interrupts.rs:1493) during loopback_wake_test teardown, 1/150 #702-hunt boots #737 specimen
data.
sys_close()never calledtcp_listener_ref_dec()forFdKind::TcpListener, a fourth retirement path with the same gap close_cloexec() leaks TcpConnection/TcpListener across exec() -- no tcp_close/tcp_listener_ref_dec #707already fixed for
close_cloexec(). Mirrored the four already-correctcall sites.
sys_spawn'scopy_string_from_uservalidated addresses viais_valid_user_address, which misses heap allocations (a gap alreadydocumented next to
copy_from_user's own correct handling in the samefile). Swapped to the same
validate_user_bufferrange check.gh issue edit: replaced the issue body's literalbroken file-path string with its intended rendered content (no repo
change).
Verification
#724and#729: clean release build, zero warnings, on x86_64(beast,
features=testing,external_test_bins) and aarch64. A full x86gate boot (
docker/qemu/run-x86-gate.sh 1 full) passed 1/1 with noregression.
#739: live two-boot end-to-end run throughgdb_session.shon beast(
info symbol $pccorrectly unresolved pre-resync, correctly resolvesreal kernel function names post-resync); a standalone unit test feeding
resync_symbols()the real mismatched-offset serial capture from the#737specimen, proving the correction path (not just the "guess alreadyright" case the live boots happened to hit).
#737:addr2line/objdumpevidence against the exact release ELF thatproduced the specimen.
Disposition notes
#724and#729are not independently reproduced with a live incident(both are latent-not-live per their own filings) and ship without a
dedicated new test, matching
#707's own landed precedent for theidentical
TcpListenerretirement-path bug shape. Both issues are leftopen with an evidence comment rather than auto-closed via commit keywords,
consistent with this project's "closures only with verified evidence"
discipline — happy to close on review if the mirroring-pattern + clean
full-gate-pass evidence is judged sufficient.
Test plan
cargo build --release --target aarch64-breenix-kernel.json -Z build-std=core,alloc -Z build-std-features=compiler-builtins-mem -p kernel --bin kernel-aarch64— zero warningstesting,external_test_bins) — zero warningsdocker/qemu/run-x86-gate.sh 1 fullon beast — GATE: PASS, 1/1gdb_session.shtwo-boot resync-symbols test on beastresync_symbols()against real x86: kernel page fault at 0x8 (interrupts.rs:1493) during loopback_wake_test teardown, 1/150 #702-hunt boots #737 mismatched-offset data🤖 Generated with Claude Code