Skip to content

memory: a guest holds only the pages it was given, and a closed span stays closed - #594

Closed
senseix21 wants to merge 2 commits into
linux/syncfrom
fix/593-memory-confinement
Closed

senseix21 wants to merge 2 commits into
linux/syncfrom
fix/593-memory-confinement

Conversation

@senseix21

Copy link
Copy Markdown
Collaborator

Two commits on top of linux/sync at 950541d8a, porting #586's memory confinement onto the branch that targets main.

Why this, and why now

#593 is the branch pointed at main, and it carries none of #586's memory work: no demand_refuse.rs, no PROT_NONE bit in peer_guard.rs, and a fork_copy that maps each piece with write and exec only. Landing it as written ships the guard-page hole to main and leaves #586 stranded on a base 127 commits behind, so whoever ports it afterwards resolves fork_copy.rs against a third variant.

Doing it here instead means the merge order stops mattering.

What was actually open on this branch

Scoping it turned up three holes rather than the one I expected, because Region has diverged between the two branches — linux/sync grew kept, #586 grew access.

1. A PROT_NONE mmap was readable and writable. map_anon::anonymous sends prot == 0 to Guest::reserve, which maps nothing, and the comment said what followed: "reserve it and let the first access fault a page in". The kernel demand-filled a zeroed frame on first touch, so a reservation read back as zeros and a musl pthread's guard page — the unopened bottom of a PROT_NONE stack reservation — took a real page and guarded nothing.

2. mprotect(PROT_NONE) on a backed span left it readable. protect_span built its bits from PROT_WRITE and PROT_EXEC only, so PROT_NONE arrived as prot 0, and the kernel's perms_of(0) returned READ | USER. The guest kept reading a span it had just given up. This one is independent of the fork path and was not in my review of #593; I found it while scoping the port.

3. Fork re-opened closed spans, and nothing recorded the protection anyway. prot_of dropped any notion of PROT_NONE, and the mprotect loop never wrote the new protection back to the region list — which is the list fork maps a child from.

The change

Kernel (4a50be4f3):

  • demand_refuse.rs refuses a not-present fault for a foreign guest. Every page a guest is meant to have is already mapped by its supervisor with MkPeerMap, so a fault elsewhere is a page nobody gave it. The fault path ends the thread and the supervisor is told, as before.
  • PROT_NONE as a peer protection bit. perms_of maps it to READ without USER: present for the kernel, which copies it at fork and frees it at teardown, and absent for every access the guest makes. perms_of moves from peer_map.rs to peer_protect.rs beside the bit it reads.

Userland (f5d969af8):

  • Region records access alongside write and exec, and peer_prot turns the three into the bits a peer call carries.
  • protect_span sends that and records it through a new set_prot, so the region list no longer holds the old protection.
  • fork_copy maps each piece with span.peer_prot().
  • A reservation still maps nothing, but is marked without access, so a touch before a commit faults as Linux faults.

demand_refuse.rs, region_prot.rs and the perms_of PROT_NONE arm are #586's, carried over unchanged. The rest is the same idea fitted to this branch's Region.

What is verified, and what is not

Compile-checked clean, exit 0 in all four:

  • nonos_kernel against x86_64-nonos.json
  • capsule_linux against x86_64-nonos-user.json, with RUSTFLAGS=-D warnings
  • capsule_linux_proofs
  • nonos_libc

I also traced the whole chain and checked the bits agree across the boundary, since these are hand-synced in two places and have drifted here before:

peer_guard.rs:   PROT_WRITE 1<<0   PROT_EXEC 1<<1   PROT_NONE 1<<2
libc/peer.rs:    PEER_PROT_WRITE   PEER_PROT_EXEC   PEER_PROT_NONE

It has not been booted. This was prepared in a worktree with no signing keys, so a release build stops at build.rs:349 and nothing can be run. That is the gap worth closing before this merges, and #586 already has the guests for it: guardpage (a musl pthread recursing into its guard page, which must end on SIGSEGV with status 139) and protfork (a 2 MiB mapping forked read-write and forked closed, where the child's read must fault). Neither is on this branch. Running those two against this is the proof I could not produce.

Worth a second opinion

  • Guest::reserve previously marked the span write: true; it is now write: false, access: false. I could find nothing that reads write on an unbacked region — fork_copy skips unbacked, and mprotect uses the prot it was passed — but that is an argument from absence.
  • is_foreign now runs on every not-present user fault, not only for guests. The kernel-half check returns before it, so a kernel-heap fault cannot re-enter the lock while registry::insert holds the write side across a Vec::push. Worth a look from someone who knows that path better.
  • This leaves linux: a guest holds only the memory it asked for #586 still needing to land for its other twelve commits — mremap provenance, brk in whole pages, mlock/msync/mincore, MAP_FIXED, the escape guest. Only the confinement part is here.

The kernel demand-filled any user page a thread touched first, foreign
guests included. A Linux guest's PROT_NONE reservation read as zeros, and
a pthread's guard page, which musl leaves as the unopened bottom of a
PROT_NONE stack reservation, took a fresh page and guarded nothing.

Every page a guest is meant to have is already mapped by its supervisor
with MkPeerMap, so a not-present fault in a foreign guest is now refused:
the fault path ends the thread and the supervisor is told, as before.

A peer protection of PROT_NONE now maps the page present without the user
bit, so the frame keeps its bytes for a later protection that allows
access while every guest access faults. perms_of moves to peer_protect
beside the bit it reads, and peer_map takes it from there.

Ported from #586 onto this branch; without it this branch lands the lane
with the hole still open.
PROT_NONE had no way to reach the kernel. protect_span sent only write
and exec bits, so mprotect(PROT_NONE) on a backed span asked for prot 0
and the kernel mapped it READ|USER: the guest kept reading what it had
just given up. A PROT_NONE mmap was worse, reserving the span and
leaving the kernel to demand-fill it on first touch, which is what let a
musl guard page take a real frame.

A region now records access alongside write and exec, and peer_prot
turns the three into the bits a peer call carries, PEER_PROT_NONE
included. protect_span sends that and records it with set_prot, so the
region list fork maps a child from no longer holds the old protection.
fork_copy maps each piece with the protection its span has now, so a
span the parent closed comes back closed in the child.

A reservation keeps nothing mapped, as before, but is marked without
access: a touch before a commit faults, which is what Linux does.

Ported from #586 onto this branch, alongside the kernel half.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

PROT_NONE mappings corrupt paging accounting when they are remapped or removed.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Ports memory-confinement protections to the Linux integration branch.

Changes:

  • Refuses demand paging for foreign guests.
  • Adds end-to-end PROT_NONE support.
  • Persists protections across mprotect and fork.
File Description
userland/​libc/​src/​peer.rs Adds the peer PROT_NONE ABI bit.
userland/​capsule_linux/​src/​linux/​guest/​region.rs Records and translates access permissions.
userland/​capsule_linux/​src/​linux/​guest/​region_prot.rs Updates stored region protections.
userland/​capsule_linux/​src/​linux/​guest/​mod.rs Registers and exports protection helpers.
userland/​capsule_linux/​src/​linux/​guest/​mem_reserve.rs Marks reservations inaccessible.
userland/​capsule_linux/​src/​linux/​call/​spawn/​fork_copy.rs Preserves protection during fork.
userland/​capsule_linux/​src/​linux/​call/​mem/​prot.rs Shares protection-mask handling.
userland/​capsule_linux/​src/​linux/​call/​mem/​prot_span.rs Applies and records protection changes.
src/​process/​foreign/​peer_protect.rs Implements kernel-side PROT_NONE.
src/​process/​foreign/​peer_map.rs Reuses centralized permission conversion.
src/​process/​foreign/​peer_guard.rs Defines the kernel ABI bit.
src/​memory/​paging/​manager/​faults/​mod.rs Registers demand-refusal logic.
src/​memory/​paging/​manager/​faults/​demand.rs Applies refusal before demand allocation.
src/​memory/​paging/​manager/​faults/​demand_refuse.rs Refuses unauthorized guest faults.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

* at teardown, and absent for every access the guest makes.
*/
if prot & PROT_NONE != 0 {
return PagePermissions::READ;
@senseix21

Copy link
Copy Markdown
Collaborator Author

Reviewed at f5d969af8 (2 commits on linux/sync at 950541d8a, 14 files, +197/−72).

A note on what this review is worth: I wrote this branch, so the usual value of a second reader is missing. What follows is an adversarial pass over my own work, and it did turn up something real — but "the author re-read it" is not the same as review, and the two items below marked for a second opinion are exactly the ones I am least able to judge.

Verdict: Request changes — on my own branch. The title claims a closed span stays closed. It does, through mprotect and fork. It does not through mremap, and I did not notice that when I wrote it.

Critical

1. mremap re-opens a span closed with mprotect(PROT_NONE), and this branch does not fix it.

Reachable in three calls on this branch as it stands:

  1. mmap(RW) → backed, access: true.
  2. mprotect(PROT_NONE) → set_prot records access: false, the kernel maps the pages READ without USER. Closed, correctly, by this PR.
  3. mremap(..., MREMAP_MAYMOVE) → re-opened.

Step 3 in detail. remap.rs:30 gates on guest.mapped_from(old) < old_len, and mapped_from (region_find.rs:23) walks the region list without consulting access or backed, so a closed span counts as mapped. r.exec is false, so the EPERM guard does not fire. Then remap_move::moved(guest, old, old_len, new_len, r.write):

// Writable while the bytes go in; the old protection after.
if guest.map(at, span, true, false) < 0 { ... }   // destination mapped writable, accessible
...
if guest.write(at, &bytes) < bytes.len() as i64 { ... }
if !write {
    let _ = super::prot::mprotect(guest, at, span, PROT_READ);
}

The destination is mapped writable and accessible, the bytes are copied in — guest.read on the source succeeds because sys_peer_copy resolves the frame through translate_in_asid and never consults the USER bit, which is the same property that makes PROT_NONE pages fillable at fork — and then, because write is false, the span is set to PROT_READ. The guest reads back what it closed. Worse, my own set_prot then faithfully records access: true, so the re-opening is inherited by any later fork.

This is #586's f60b8190b, which I did not port and did not mention. Its commit message describes the whole family:

mremap gave the part it grew read-write whatever the mapping was, so a PROT_NONE or read-only mapping grew a writable tail. A move read the old bytes and failed with EFAULT on a reservation, which has none; restored only read-only or read-write on the copy, so a PROT_NONE mapping came back readable; left the new span mapped when the copy failed; and dropped the mark that the bytes came from a file nothing proved, so the moved copy could then be made executable with mprotect, past the check that refuses it where the file was mapped.

#586 fixes it by passing the whole Region rather than a bool — guest.map_like(tail, grow, &r) for the grow and moved(guest, old, old_len, new_len, &r) for the move. That is the right shape and it is what this branch needs.

So the scope of this PR is wrong rather than its content. Either port f60b8190b here too, or retitle so it does not claim more than it closes — the first is better, because a half-closed PROT_NONE is the kind of thing a reader stops checking once the title says it is handled.

Important

2. I changed Guest::reserve's write flag and did not trace every reader.

reserve went from Region::new(start, span, true, false, false) to write: false, access: false. In the PR body I defended this as "I could find nothing that reads write on an unbacked region", and called it an argument from absence. It was, and it was incomplete: remap.rs:52 and :63 both read r.write and neither checks backed.

The effect on this branch is that an mremap of a PROT_NONE span now grows or restores a read-only tail where before it produced a writable one. Both are wrong against Linux, which grows PROT_NONE as PROT_NONE, so this is not a regression in severity — but it is an undocumented behaviour change that I introduced while claiming the field was unread. Item 1's fix subsumes it; until then it should at least be stated.

The general lesson for the next port: grep for a field name is not a reader analysis when the type is Copy and gets destructured into bools at call sites.

Minor

  1. protect_span calls guest.set_prot(...) before the mk_peer_protect loop, so a peer call that fails partway leaves the region list claiming the new protection while the kernel holds a mix. The old code recorded nothing, so this window is new. It is narrow and fails in the safe direction for PROT_NONE (the list says closed, some pages are still open — the next fork would under-grant rather than over-grant), but the reverse case, opening a span, records access the kernel has not applied. linux: a guest holds only the memory it asked for #586 has the same ordering, so this is inherited rather than introduced; worth recording the protection only after the loop succeeds.
  2. The PR body says Region "grew kept" on this branch and "access" on linux: a guest holds only the memory it asked for #586 — accurate, but it does not say that region_cut.rs and advise_drop.rs propagate the new field for free through ..*r. That is the reason the port touched six construction sites instead of twenty, and it is the kind of thing a reviewer would otherwise have to re-derive.

For a second opinion — the two I cannot judge

  1. is_foreign on every not-present user fault. I argued the ordering in demand_refuse::refused makes it deadlock-free: the kernel-half test returns before is_foreign, so a kernel-heap fault cannot re-enter FOREIGN.read() while registry::insert holds the write side across a Vec::push. I believe that is right, and I checked clear does not nest its read inside its write. But I am the person who wrote the argument, and a lock-ordering claim is exactly the kind that reads as sound to its author. Someone who knows the fault path should confirm it.
  2. The unbacked-reservation comment in fork_copy. I carried over linux: a guest holds only the memory it asked for #586's "the child holds the same reservation, and a touch there faults in the child as here" without verifying how the child's region list is populated — copy_spans skips !span.backed, so if the list is not cloned elsewhere, the child does not hold the reservation at all and a touch there is a plain unmapped fault rather than a reservation fault. The observable behaviour may be identical; the comment's claim is not something I checked.

Verified correct

  • The protection bits agree across the boundary. peer_guard.rs PROT_WRITE 1<<0, PROT_EXEC 1<<1, PROT_NONE 1<<2; libc/peer.rs the same three. Hand-synced constants in two components have drifted in this repo before, so this was checked rather than assumed.
  • The mprotect chain closes end to end. prot == 0 → prot & PROT_ANY == 0 → access false → peer_prot returns PEER_PROT_NONE → mk_peer_protect → sys_peer_protect → perms_of(4) → READ without USER. Each hop read at this head.
  • The two bit spaces are not confused. prot_span.rs imports Linux's PROT_WRITE = 2 / PROT_EXEC = 4 from prot.rs and converts through peer_prot into the peer bits; nothing compares a Linux prot against a PEER_PROT_*.
  • set_prot's backed filter is right. An mprotect(PROT_NONE) on an unbacked reservation continues before protect_span, so the region keeps the access: false it got from reserve; an mprotect that asks for access commits first, which makes it backed with access: true, and set_prot then records on a backed span.
  • Four crates compile clean, exit 0: nonos_kernel on x86_64-nonos.json; capsule_linux on x86_64-nonos-user.json under RUSTFLAGS=-D warnings; capsule_linux_proofs; nonos_libc. None of the files this touches are #[path]-mirrored into the proofs crate — only exec_shebang.rs is, from that directory.
  • demand_refuse.rs, region_prot.rs and the perms_of PROT_NONE arm are byte-identical to linux: a guest holds only the memory it asked for #586's. Diffed, not eyeballed.

What is still unproven

Nothing here has been booted — no signing keys in the worktree, so a release build stops at build.rs:349. #586's guardpage and protfork guests are the proof and neither is on this branch. Item 1 adds a third thing worth a guest: close a span with mprotect(PROT_NONE), mremap it, and read it back.

@eKisNonos

Copy link
Copy Markdown
Contributor

Superseded. This work is integrated into the 0.9.2 release and ships in the current tree. Closing as part of the 0.9.2 consolidation.

@eKisNonos eKisNonos closed this Oct 2, 2026
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.

3 participants