Skip to content

Kernel: stop reserving the stack's image offset as a physical range - #584

Merged
Japabu merged 4 commits into
mainfrom
wt/toyos-stackres
Sep 29, 2026
Merged

Japabu merged 4 commits into
mainfrom
wt/toyos-stackres

Conversation

@Japabu

@Japabu Japabu commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

KernelArgs::kernel_stack_addr is an offset into the kernel image, not an address. kernel_main read it as a physical address and reserved [kernel_stack_addr, +8 MiB), so the PMM held back low physical frames that nothing uses. The deleted line is replaced by two boot-time checks.

Changes

  1. The line is deleted, not corrected. A corrected region would duplicate the image's reservation.
  2. New assert: the stack is inside the image. kernel_stack_addr + kernel_stack_size <= kernel_memory_size, which is exactly the loader fact the deletion depends on.
  3. New assert: every loader allocation lies inside one LoaderData descriptor of the firmware map. The image, the ELF, the black-box page and ROOT are named in a loader array and checked against it. The check calls toyos_rootimage::handoff::held — the function kernel/src/rootfs.rs already calls for ROOT's image — instead of an inline copy; block size 1, because the ELF region is not page-aligned.
  4. reserved destructures loader by name, not by index. let [image, elf, black_box, root] = loader; let reserved = [image, elf, black_box, root, arch::boot::reserved()]; — indexing (loader[0], …) let a region added to loader compile, pass the containment check, and still be silently dropped from what mm::init withholds. Destructuring by name turns that mutation into a compile error. Proof: appending a fifth element to loader (a checked patch, applied and reverted, not part of this diff) makes cargo run -- --build-only fail with error[E0527]: pattern requires 4 elements but array has 5 at src/main.rs:392:9, EXIT=101; the unmutated tree builds at EXIT=0.
  5. The boot record now prints stack image+<offset>+<size>. Before, it printed the offset in the slot where an address goes. No test parses that record.

The architecture's own reserved page is not a loader allocation and is appended to reserved after the containment check runs, rather than exempted by position.

No KernelArgs layout change. The misleading name kernel_stack_addr is filed as issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md, owned by the author of PR #583 (wt/toyos-loader1), which is already changing that struct.

Gates, at 65591b9

gate exit
cargo run -- --ci host 0 (54/54 host steps)
cargo run -- --build-only 0

High-risk checks (memory management)

  • Negative control: reinstate the deleted line in the reserved array, just ahead of the architecture's page, and revert the two asserts, the held call and the loader/reserved split with it — the whole change, back onto 762e4ba. Not yet re-run at this head; round 1's run at b0c5640 panicked before mm::init for the named reason.
  • Independent oracle, narrowed to x86-64 OVMF, where it is true: the firmware map OVMF reports at ExitBootServices is real and independent of the kernel's own computation — the loader copies entry.ty.0 verbatim (bootloader/src/main.rs:771), and the containment assert's refusal is OVMF's descriptors disagreeing with a reserved region. No such oracle exists for the T14 (no reading is on record at any head) or for AArch64 (no guest run reaches the check), and it says nothing about where the stack is — that premise rests on the stack-in-image assert alone, which has never been seen red on any run.

What I'm unsure of

Measured by the orchestrator: pmm_accounting's withheld total is 29360128 bytes at 762e4ba and 20971520 at 65591b9; the 8388608-byte difference is the stack's size.

  • The containment assert depends on each loader allocation lying inside one descriptor. A firmware that splits a single allocation across descriptors, or a map truncated past MAP_MARGIN (tracked in issues/panic-path/the-loaders-truncated-map-refusal-is-executed-by-nothing.md), would now panic at boot instead of booting; the image and ELF pool allocations lying in one descriptor each is unmeasured on the T14, and its first boot of this head is that measurement.
  • No AArch64 guest test reaches this code: the virt_* tests stop at test-early-panic, before the reservations.

🤖 Generated with Claude Code

`KernelArgs::kernel_stack_addr` is an offset into the kernel image, not an
address. The loader writes `StackedImage::place`'s `stack` there
(bootloader/src/main.rs, `stack_offset: placed.stack`), and both entries
read it that way: x86-64 `_start` adds it to `kernel_memory_addr`, and the
AArch64 `_start` loads it under the name `stack_offset`. `kernel_main`
alone read it as physical and reserved `[kernel_stack_addr, +8 MiB)`.

The kernel is a static PIE at base 0, so the offset is the image's `vaddr`
extent rounded up to a page: 0x42c000 for main's x86-64 kernel, measured with
llvm-readobj -l on target/kernel-x86_64-4be7ccf4859a9aa6 (the last PT_LOAD
ends at 0x200240 + 0x22b940 = 0x42bb80). The bogus region
[0x42c000, 0xc2c000) touches the 2 MiB frames at 0x400000, 0x600000,
0x800000, 0xa00000 and 0xc00000, so the PMM withheld each of those five
(up to 10 MiB) that lies wholly inside a usable firmware entry, and nothing used
them. On QEMU's AArch64 `virt` machine RAM starts at 0x4000_0000, so the range
reached no RAM there.

The real stack was never at risk. The loader allocates the image and the stack
as one block: `mem_size = placed.size = stack + stack_size`, and it passes that
block as `kernel_memory_size`. So the stack is the image's tail,
[kernel_memory_addr + stack, kernel_memory_addr + kernel_memory_size), and the
image's own reservation already keeps it. The line is deleted rather than
corrected, because a corrected version would duplicate the image reservation.

Two checks replace it:
- an assert that `kernel_stack_addr + kernel_stack_size <= kernel_memory_size`,
  the loader fact the deletion relies on;
- every reservation except the architecture's own page must lie inside one
  `LoaderData` descriptor of the firmware map. Each region in that list is an
  allocation made by the loader: the image, the ELF, the black-box page and ROOT.
  A region not held that way withholds memory nothing uses. This is the check
  that refuses the deleted line if it is ever reinstated. The architecture's
  page moves to the end of the array so the check can exclude it by position.

The boot record printed the offset in an address's slot. It now reads
`stack image+<offset>+<size>`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6rME2DoqwjcYFStYHHY4j
@Japabu
Japabu marked this pull request as ready for review September 28, 2026 18:35
Japabu added a commit that referenced this pull request Sep 28, 2026
…s controls can fail

CLAUDE.md's Firmware paragraph now reads as the orchestrator ruled: the
kernel calls no UEFI service, and every UEFI call is the loader's, before
ExitBootServices. The loader's GetVariable, SetVariable, GetTime and
ResetSystem come before the handover, so the old wording was false of it.

The track:
- Stage 1 matches #583 at 8565cc5: KernelArgs::layout (0x5459_0001),
  kernel_args_layout_refused with the loader-writes-no-layout actuator,
  the probe's realtime=, and the kernel's IA32_TSC_ADJUST line. No test
  fails without that line, and the stage says so.
- Stage 3 drops "why nothing is weaker". A security version admits an
  older build the same key signed at that version. The floor issue records
  that as the owner's accepted cost. Raising the version is a reviewed PR
  that edits one constant and names the security fix. The loader deletes
  the build-time ToyOSImageFloor- variables instead of leaving them behind.
- Stage 4's panic handler writes loader.log and powers off, never resets.
  Its test panics on a floor planted in 9 bytes, a failure the machine
  causes. KernelArgs' layout word rises to 0x5459_0002, and
  kernel_args_last_layout_refused fails if it does not.
- Stage 5 refuses a signed kernel the loader cannot load inside verify,
  so the other slot boots instead of the pass bricking the machine. An
  install's priority rises above the kept slot's. update --good, run by
  init at the health gate under the slots claim, writes the good flag, and
  an image without that claim is never good. Each rule gets a named guest
  control: update_floor_waits_for_good, update_readonly_stick_boots_nothing
  (red under `let persisted = true;`) and
  update_unloadable_kernel_boots_the_other_slot. The slot-table oracle is
  decoded without production code.
- Every #539 piece the review listed is placed or deleted. The #539-only
  issue names and the stack-offset closure (#584's) are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator Author

Review, round 1, at b0c5640

Ready: CI host success at b0c5640 (run 36466332047; the skipped run 36466314650 is the same head's duplicate). The orchestrator's guest runs at this head exited 0 (Fast; --nightly pmm_accounting, root_from_memory, blackbox_unclaimed_page). git merge-tree --write-tree origin/main HEAD (origin/main e3a1cdc, merge-base 762e4ba): clean. Net: +20 -3, all production (kernel/src/main.rs), 0 test lines.

Negative control: red for the named reason. The assert runs after panic_console::arm, blackbox::arm, serial::init and seven log! records, so a panic there reaches the 16550 and the panel. The kept 16550 log target/red-run-serial/toyos-tmp-36352-0/lane-0/uart-0.log ends EARLY PANIC: panicked at src/main.rs:374:9: boot: reserving 0x42e000..0xc2e000, which no LoaderData descriptor in the firmware map holds. The "nothing at all" is the harness: its timeout verdict quotes stdio only, which is already tracked in issues/build/a-boot-timeout-verdict-quotes-stdio-alone-and-never-the-16550-log.md. Neither the assert nor its placement has to change.

BLOCKER

  • kernel/src/main.rs:371-379 — the containment predicate e.uefi_type == EFI_LOADER_DATA && e.start <= region.start && region.end <= e.end duplicates toyos_rootimage::handoff::held (toyos-rootimage/src/handoff.rs:19). The kernel already depends on that crate, and toyos-rootimage/tests/handoff.rs tests every edge of it, including another type, straddling descriptors and overflow. No test covers the inline copy, and it drops held's checked_add. For example, deleting e.uefi_type == toyos_bootmap::EFI_LOADER_DATA && leaves every committed test green. Fix: call held(maps.iter().map(|e| Descriptor { ty: e.uefi_type, start: e.start, end: e.end }), EFI_LOADER_DATA, region.start, region.end - region.start, 1).is_some() (block 1, because the ELF region 0x7d093018+0x34c2a0 is not page-aligned). Or move held into toyos-bootmap next to is_usable_type and have both callers use it. After that, the type-clause mutation applied to held must turn an_extent_inside_a_descriptor_of_another_type_is_refused red.

NOTE

  • kernel/src/main.rs:367,371 — the architecture's page is exempted by position (reserved[..reserved.len() - 1], "Last:"), not by name. A region appended later is silently exempt. On AArch64, where the arch region is empty, the check would silently skip a real region. Name the exemption instead: check a loader array, then build reserved from its elements plus arch::boot::reserved().
  • T14 — I expect no split. The same one-descriptor containment already holds there for ROOT's AllocatePages (rootfs::init -> held, T14 run 143, The loader puts ROOT in memory and the kernel mounts it from there #506). The image and the ELF are single pool allocations (bootloader/src/main.rs:78-81, :155). A mismatch would be named on the panel, which is armed before the assert. The residual risk is the truncated map, which is already tracked (issues/panic-path/the-loaders-truncated-map-refusal-is-executed-by-nothing.md). The first T14 boot of this head is the measurement for the pool allocations.
  • AArch64 — no guest run executes either new assert: the virt_* tests stop at test-early-panic (kernel/src/main.rs:293), which comes before them. The deleted region reached no RAM there, so the claim of no change on AArch64 rests on reading and on clippy alone.
  • pmm_accounting — the drop of about 8 MiB is inferred from other branches' runs: 20971520 B withheld at b0c5640 against 29360128 B on the Every guest boots the UEFI firmware the host's QEMU declares; ovmf/ and aavmf/ go #572 runs, with the same 4089 MiB total and the same 29077504 B lost to alignment. No run at the merge-base 762e4ba is on record. Reverting the whole change stays green by construction, because the check is part of the change; the partial control above is the one that discriminates.
  • toyos-abi/src/boot.rs:8 — kernel_stack_addr, the misleading name that caused this bug, stays. The rename is deferred to "after Loader slimming stage 1: the RTC is UTC, KernelArgs refuses another layout by name, IA32_TSC_ADJUST moves to the kernel #583" with no issue filed. Record it in issues/ with an owner and an exit condition, or rename it once Loader slimming stage 1: the RTC is UTC, KernelArgs refuses another layout by name, IA32_TSC_ADJUST moves to the kernel #583 lands.

REMOVE

  • issues/kernel/the-kernel-reserves-its-stack-offset-as-a-physical-region.md — this branch meets its exit condition (the region is gone and a boot assertion checks the reserved list), so the file is now false. Delete it. git grep on HEAD finds no citations.
  • PR body "Guest runs are the orchestrator's and have not been run." — false at this head.
  • PR body "That run has not happened yet; it is queued below." — false at this head.
  • PR body "The exact number depends on QEMU's firmware map, which I did not measure because agents do not run QEMU." — narration of the author's process.
  • PR body "Second oracle: pmm_accounting's withheld= ... It should drop by ..." — a prediction, and the kernel's own count is not independent of the author.
  • PR body "(worktree, head below)" — no head follows it.

SEND BACK

…not a copy

The inline predicate in kernel_main's reservation loop duplicated
toyos_rootimage::handoff::held without its checked_add, and no test covered
the copy. Both crates were already kernel dependencies, so the loop now
builds a Descriptor iterator and calls held(..., block=1) — block 1 because
the ELF region is not page-aligned — the same call kernel/src/rootfs.rs
already makes for ROOT's image.

The architecture's own page is now named (a `loader` array of the four
loader allocations, checked, with arch::boot::reserved() appended after) so
a later region can't land inside the exempted slot by position.

Files the misleading kernel_stack_addr name (toyos-abi/src/boot.rs:8) as an
issue for after #583, and removes the tracker entry this branch's exit
condition already closes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Review, round 2, at 4126414

Ready: CI host success at 4126414 (run 36477581727). The orchestrator's guest runs at 4126414 exited 0 (Fast; --nightly pmm_accounting withheld=20971520, root_from_memory, blackbox_unclaimed_page). git merge-tree --write-tree origin/main HEAD (origin/main 8a8fe27): clean. Net: +47 -30; production kernel/src/main.rs +29 -4, issues/ +18 -26, tests 0.

Round-1 blockers

  • kernel/src/main.rs inline containment predicate: CLOSED. main.rs:372-385 calls toyos_rootimage::handoff::held. The body's host mutation (type clause deleted from held) reports left: Some(8192..24576), right: None for an_extent_inside_a_descriptor_of_another_type_is_refused. That is exactly what map()'s CONVENTIONAL descriptor yields for 2..6 pages.

Negative control and oracle

  • mutation-r2.patch is not a negative control under CLAUDE.md. It puts one line back onto HEAD, and into loader, not reserved. Both asserts, the held call, the loader/reserved split and the record format all stay. The whole change reverted onto 762e4ba stays green by construction: the check is part of the change, and pmm_accounting's judge only checks managed + withheld + lost == firmware. It is a valid mutation, which is CLAUDE.md's other arm. Its red comes from the check the PR relies on: target/red-run-serial/toyos-tmp-99869-0/lane-0/uart-0.log ends EARLY PANIC: panicked at src/main.rs:374:9: boot: reserving 0x42e000..0xc2e000, which no LoaderData descriptor in the firmware map holds. Line 374 is HEAD's containment assert! at 373, shifted by the added line. The harness's "nothing at all" is the tracked stdio-only verdict.
  • Oracle: the firmware map is real and independent on x86-64 OVMF. The loader copies entry.ty.0 verbatim (bootloader/src/main.rs:771), and the control's refusal is OVMF's descriptors disagreeing with the reinstated region. The oracle does not exist for T14, where no reading is on record at any head, or for AArch64, where no guest run reaches the check. It says nothing about where the stack is: that premise rests on the stack-in-image assert alone, and nothing has ever seen that assert red.

BLOCKER

  • kernel/src/main.rs:389 — let reserved = [loader[0], loader[1], loader[2], loader[3], arch::boot::reserved()]; copies by position, so a region added to loader compiles and is checked but silently dropped from what mm::init withholds. mutation-r2.patch is exactly that tree: reserved loses root_image, so the allocator may hand out ROOT's live image. Round 1's positional exemption became a positional drop. Fix: let [image, elf, black_box, root] = loader; let reserved = [image, elf, black_box, root, arch::boot::reserved()];. Its check: adding a fifth element to loader must fail to compile.

NOTE

REMOVE

  • PR body "(orchestrator's, at b0c5640 — this round's fix is host-only, no main.rs behavior it touches is untested by them)" — false: 4126414 changes the predicate and reserved's construction in main.rs.
  • PR body "Negative control:" label — it is a mutation on HEAD, not the whole change on 762e4ba.
  • PR body "or the T14's firmware" — no T14 reading exists.
  • kernel/src/main.rs:387 "(the AP trampoline on x86-64, empty on AArch64)" — restates arch::boot::reserved's own docs and rots with them.
  • issues/kernel/kernel_stack_addr-names-an-offset-not-an-address.md "(toyos-abi/src/boot.rs:8)" — a line number that rots.

SEND BACK

Japabu and others added 2 commits September 29, 2026 02:29
`reserved`'s construction copied `loader` by position, so a fifth region
added to `loader` compiled and passed the containment check but was silently
dropped from what `mm::init` withholds — exactly what mutation-r2.patch had
done to `root_image`. Destructuring `loader` by name (`image`, `elf`,
`black_box`, `root`) makes a fifth element a compile error instead.

The issue's owner line named a role ("whoever lands #583"), not a concrete
owner; it now names PR #583 (wt/toyos-loader1) itself, and drops the line
number citation that rots with the next edit to toyos-abi/src/boot.rs. The
same rotting citation is dropped from main.rs's comment on the
architecture's own reserved page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu

Japabu commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Review, round 3, at 65591b9

Ready: CI host success on 65591b9 (run 36504027919, headSha matches). gh pr view shows MERGEABLE. git merge-tree --write-tree origin/main HEAD (origin/main 3d90247) exits 0. Orchestrator's guest runs at 65591b9: pmm_accounting, root_from_memory, blackbox_unclaimed_page, boot_from_power_on and Fast all EXIT=0. Net: +52 -30. Production kernel/src/main.rs is +33 -4, issues/ +19 -26, tests 0.

Round-2 blockers

  • kernel/src/main.rs reserved built by index: CLOSED. main.rs:391-392 now reads let [image, elf, black_box, root] = loader; let reserved = [image, elf, black_box, root, arch::boot::reserved()];. The body records its proof: a fifth loader element fails with error[E0527]: pattern requires 4 elements but array has 5, EXIT=101. A dropped region is now a compile error.

Round-2 NOTEs, now measured

  • The stack-in-image assert has its red arm. boot_from_power_on under stack-outside-image.patch gives EXIT=1. The harness says "nothing at all", which is the tracked stdio-only verdict. The kept serial log target/red-run-serial/toyos-tmp-10759-0/lane-0/uart-0.log ends with EARLY PANIC: panicked at src/main.rs:353:5: boot: the loader put the stack at image+0x42e000+0x800000, past the 0xc2d000-byte image. That is the assert at main.rs:353. The arithmetic matches the mutation: 0x42e000+0x800000 = 0xc2e000, which is greater than 0xc2d000. It is not an unrelated boot failure.
  • The drop is measured. pmm_accounting at 762e4ba reads 29360128 B withheld. At 65591b9 it reads 20971520 B withheld, with lost at 29077504 on both. The difference is 8388608 = 0x800000, exactly kernel_stack_size (serial: Kernel stack size: 8388608). A green base is expected, because the judge only checks the sum. The base-vs-head withheld= difference is the measurement behind the claim, and it holds.

BLOCKER

None.

NOTE

  • kernel/src/main.rs:373 — ROOT's region is checked here a second time with block 1, after rootfs::init has already refused it with page blocks. The check is redundant and cannot fail. It stays because it iterates over loader uniformly, and splitting it out would cost more lines than it saves.
  • The whole-change negative control has not been re-run at this head. Round 1's run at b0c5640 is red for the named reason. Since then, the base-vs-head withheld readings show the change's effect directly. T14 and AArch64 remain unmeasured, as the body says.

REMOVE

  • PR body, "What I'm unsure of", first bullet ("pmm_accounting at 762e4ba has no run on record … that run is owed and I have not made it"). It is false now, and the "10 MiB" figure it cites appears nowhere in the body.
  • PR body, "## The round-2 NOTE's mutation …" section. It is a prediction ("should end in … first red arm on any run") that has since been measured, and it names a scratchpad path that main's record will not carry.
  • PR body, "mutation-r2.patch is a mutation, not a negative control — round 1's classification was wrong, and the correction stands …" bullet. This is review-round chronology about a scratchpad patch, and it goes into the merge commit.
  • PR body, "(round 2, BLOCKER)" in Changes item 4, and the sentence "That is exactly what mutation-r2.patch had done to root_image." Both are review chronology.
  • PR body, "git merge origin/main (8a8fe27) is already in this branch's history — no conflicts." This is a snapshot that rots.

LAND AFTER NAMED CHANGES

@Japabu
Japabu added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 08ea426 Sep 29, 2026
1 check passed
@Japabu
Japabu deleted the wt/toyos-stackres branch September 29, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant