T-025 — Mmu::translate + per-task user-access translation (B6 gate #1)#39
Merged
Conversation
…ranslation (B6 gate #1) Adds the read-only Mmu::translate walk query (the deferred ADR-0009 "translation walk query", now with its first caller) + the per-task user-access policy closing B6 gate #1: the syscall copy path resolves each user VA through the task's own TTBR0 and enforces USER, defeating the in-window kernel-VA confused-deputy read. Opens T-025 (gate #1 mechanism) + T-026 (gate #3 plumbing) per ADR-0025 Rule 1; ADR-0009 Revision rider records the additive trait method. Refs: ADR-0038 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…kipped) Two relayed reviews of the Propose set, verified against the live tree (workflow). Applied 8 valid fixes: T-026 — persist the per-task window in a scheduler array (H1; the LoadedImage span is reachable nowhere today) + mandate an early short-circuit for task_yield/task_exit which bypass the table/window (H2) + de-conditionalize the mandatory UNSAFE-2026-0014 amendment (M4). T-025 — two-pass probe-then-copy for true all-or-nothing (2.1.1) + round-trip over VALID flag combos only since DEVICE|EXECUTE is rejected + a lock-shut property test (M2) + FakeMmu page-base-align/never-BlockMapped + a module-doc-correction note (L1). ADR-0038 — reframe sim row 1 (a high-half VA fails the tight per-task window, not the USER check) + add a copy_to_user WRITE row + footnote (M1/2.3.1). ADR-0009 rider — lookup->translate naming rationale + clarify BlockMapped is REUSED not added (2.2.2/2.3.4). Skipped 3 (verified invalid): M3 (rider past-tense mirrors the MapperFlush precedent), 2.2.1 (MmuError::BlockMapped already exists — T-018), 2.4.2 (ADRs deliberately omit downstream-consumer back-links). Refs: ADR-0038 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…anslation (B6 gate #1) Careful re-read per write-adr §10: forward-refs ground in T-025/T-026 (real), dependency chain complete (ADR-0009/0033/0030 Accepted), negatives are real costs. The re-read tightened §Decision-outcome line 62 to the two-pass probe-then-copy model (consistency with §Simulation row 4). Flips ADR-0038 Proposed→Accepted; T-025 Draft→In Progress. Refs: ADR-0038 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…coder (gate #1 HAL surface) Adds the deferred ADR-0009 'translation walk query', realised per ADR-0038 as fn translate(&self, &AddressSpace, VirtAddr) -> Result<(PhysFrame, MappingFlags), MmuError>: a read-only L0->L3 walk returning the page frame + leaf flags (NotMapped on absent leaf, BlockMapped on block hit). vmsav8 gains the inverse decoder descriptor_bits_to_flags (lock-shut, round-trip + property tested). QemuVirtMmu::translate reuses the audited walker (UNSAFE-2026-0025, read-only); FakeMmu (page-base lookup, never BlockMapped) + OutOfFramesMmu/BlockMappedMmu/FailingMapMmu impls. No caller yet (gate #1 copy-user rewrite is step 3). Refs: ADR-0038, T-025 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…SyscallContext<M> (gate #1) Closes the B6 gate #1 mechanism (ADR-0038): copy_from_user/copy_to_user become generic over <M: Mmu>, two-pass (probe-all-then-copy) — pass 1 translates every spanned page through the task's own AS and requires USER (+WRITE for to_user), pass 2 rebases each frame via phys_frame_kernel_ptr and copies; any miss/non-USER/block-mapped page -> SyscallError::FaultAddress, never a panic. The per-page USER check is the confused-deputy boundary (a kernel/in-window VA is rejected); the per-task [entry_va,stack_top_va) window is the cheap first gate. SyscallContext gains mmu/task_as; sys_console_write probes the whole range up front (all-or-nothing emit). BSP: syscall_entry sources &MMU + the bootstrap AS (new BOOTSTRAP_AS static); the +0x200 stub console_write of a kernel VA is now correctly rejected (smoke status=0x3) — a positive gate-#1 demonstration. Shared host fixture tyrne_test_hal::FakeUserMem (page-aligned host pages mapped USER) drives the confused-deputy / unmapped / read-only / multi-page all-or-nothing tests. Gates: fmt; host+kernel clippy -D warnings; host tests 252 kernel / 46 hal / 58 test-hal / 3 doc; kernel build; QEMU smoke PASS (2 SVC exceptions, clean ERET, zero new fault class); Miri 0 UB. UNSAFE-2026-0030 + 0025 Amendments (no new entries). Remaining for the EL0 wire-up: gate #3 (T-026) sources the running task's AS + cap table. Refs: ADR-0038, T-025 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideImplements the B6 gate #1 user-access translation mechanism by adding a read-only Mmu::translate walk, wiring syscall copy-from/to-user to do per-page USER/WRITE checks via the task’s own address space, and updating syscall dispatch/BSP/test-hal docs and tests accordingly while preserving panic-free behaviour and tightening the privilege boundary. Sequence diagram for console_write with per-page Mmu::translate gatesequenceDiagram
actor El0Task
participant SyscallEntry
participant SyscallDispatch
participant SyscallContext
participant Mmu
participant Console
El0Task->>SyscallEntry: SVC console_write(ptr,len,cap)
SyscallEntry->>SyscallContext: build SyscallContext(mmu, task_as, user_window)
SyscallEntry->>SyscallDispatch: dispatch(args)
SyscallDispatch->>SyscallContext: sys_console_write(args)
SyscallContext->>SyscallContext: validate_debug_console_cap
SyscallContext->>SyscallContext: user_window.validate(ptr,len)
alt len > 0
SyscallContext->>Mmu: probe_user_pages(mmu, task_as, ptr, len, require_write=false)
Mmu-->>SyscallContext: translate(task_as, page_va) for each page
alt any translate error or !USER
SyscallContext-->>SyscallDispatch: SyscallError::FaultAddress
SyscallDispatch-->>SyscallEntry: SyscallReturn::error
SyscallEntry-->>El0Task: status=FaultAddress, bytes=0
else all pages OK
loop chunks
SyscallContext->>SyscallContext: copy_from_user(mmu, task_as, user_window, chunk_ptr)
SyscallContext->>Mmu: translate(task_as, page_base)
Mmu-->>SyscallContext: (PhysFrame, MappingFlags::USER)
SyscallContext->>Console: write_bytes(chunk)
end
SyscallDispatch-->>SyscallEntry: SyscallReturn::ok(bytes)
SyscallEntry-->>El0Task: status=0, bytes
end
else len == 0
SyscallDispatch-->>SyscallEntry: SyscallReturn::ok(0)
SyscallEntry-->>El0Task: status=0, bytes=0
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR implements B6 gate ChangesB6 Gate
Sequence DiagramsequenceDiagram
participant App as EL0 App
participant SyscallEntry as syscall_entry
participant Dispatcher as dispatch
participant ConsoleWrite as console_write
participant Probe as probe_user_pages
participant Copy as copy_from_user
participant Mmu as Mmu::translate
App->>SyscallEntry: syscall (console_write, ptr, len)
SyscallEntry->>Dispatcher: SyscallContext{mmu, task_as, ...}
Dispatcher->>ConsoleWrite: ctx, args
ConsoleWrite->>Probe: mmu, task_as, ptr, len
loop for each page in [ptr, ptr+len)
Probe->>Mmu: translate(va)
alt translates
Mmu-->>Probe: PhysFrame, MappingFlags
Probe->>Probe: check MappingFlags::USER
else not mapped or no USER
Mmu-->>Probe: NotMapped or missing USER
Probe-->>ConsoleWrite: FaultAddress
ConsoleWrite-->>App: FaultAddress (0 bytes written)
end
end
alt all pages pass
Probe-->>ConsoleWrite: Ok
loop for each page in [ptr, ptr+len)
ConsoleWrite->>Copy: chunk
Copy->>Mmu: translate(page_va)
Mmu-->>Copy: PhysFrame
Copy->>Copy: rebase via phys_frame_kernel_ptr
Copy->>Copy: memcpy into console buffer
end
ConsoleWrite-->>App: Ok (bytes written)
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the security-critical B6 gate #1 (ADR-0038 / T-025), introducing the read-only Mmu::translate walk query to the Mmu trait and updating the syscall copy-user path (copy_from_user and copy_to_user) to use a two-pass, per-page translation check that enforces USER permissions. Feedback on the changes highlights a high-severity bug in probe_user_pages where incrementing the page address by PAGE_SIZE can overflow if the range ends at the very end of the address space, causing valid accesses to incorrectly fail.
Comment on lines
+150
to
+164
| let mut page = user_ptr & !PAGE_MASK; | ||
| while page < end { | ||
| let (_frame, flags) = mmu | ||
| .translate(task_as, VirtAddr(page)) | ||
| .map_err(|_| SyscallError::FaultAddress)?; | ||
| if !flags.contains(MappingFlags::USER) { | ||
| return Err(SyscallError::FaultAddress); | ||
| } | ||
| if require_write && !flags.contains(MappingFlags::WRITE) { | ||
| return Err(SyscallError::FaultAddress); | ||
| } | ||
| page = page | ||
| .checked_add(PAGE_SIZE) | ||
| .ok_or(SyscallError::FaultAddress)?; | ||
| } |
There was a problem hiding this comment.
In probe_user_pages, the loop increments page by PAGE_SIZE and checks page < end. If the range [user_ptr, user_ptr + len) ends in the very last page of the address space (i.e., end > usize::MAX - PAGE_SIZE), the final increment page.checked_add(PAGE_SIZE) will overflow and return None. This causes the function to incorrectly return Err(SyscallError::FaultAddress) even though the range itself is completely valid and does not wrap.
We can fix this by using a loop and checking if the next page starts after or at end before performing the increment, avoiding the overflow error.
let mut page = user_ptr & !PAGE_MASK;
loop {
let (_frame, flags) = mmu
.translate(task_as, VirtAddr(page))
.map_err(|_| SyscallError::FaultAddress)?;
if !flags.contains(MappingFlags::USER) {
return Err(SyscallError::FaultAddress);
}
if require_write && !flags.contains(MappingFlags::WRITE) {
return Err(SyscallError::FaultAddress);
}
let next_page = page.checked_add(PAGE_SIZE);
match next_page {
Some(next) if next < end => {
page = next;
}
_ => break,
}
}![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The multi-page copy logic in
copy_from_userandcopy_to_user(page alignment,runcomputation, offset arithmetic, and repeatedtranslate+ flag checks) is almost identical; consider extracting this into a shared internal helper to reduce duplication and the chance of subtle divergence between the read and write paths. - Range arithmetic (
end,checked_add, wrap handling) now appears inUserAccessWindow::validate,probe_user_pages, and the copy loops; factoring the range computation into a single helper reused across these sites would make it easier to reason about overflow/wrap behavior and avoid future inconsistencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The multi-page copy logic in `copy_from_user` and `copy_to_user` (page alignment, `run` computation, offset arithmetic, and repeated `translate` + flag checks) is almost identical; consider extracting this into a shared internal helper to reduce duplication and the chance of subtle divergence between the read and write paths.
- Range arithmetic (`end`, `checked_add`, wrap handling) now appears in `UserAccessWindow::validate`, `probe_user_pages`, and the copy loops; factoring the range computation into a single helper reused across these sites would make it easier to reason about overflow/wrap behavior and avoid future inconsistencies.
## Individual Comments
### Comment 1
<location path="docs/analysis/tasks/phase-b/T-025-user-access-translation.md" line_range="8" />
<code_context>
+- **Dependencies:** [ADR-0038](../../../decisions/0038-mmu-translate-and-user-access.md) (drives this task — the `Mmu::translate` surface + the per-task user-access policy); [ADR-0009](../../../decisions/0009-mmu-trait.md) (the `Mmu` trait this extends; §Open-questions "Translation walk queries" is the deferred capability this realises); [ADR-0033](../../../decisions/0033-kernel-high-half-migration.md) (the high-half direct map [`phys_frame_kernel_ptr`](../../../../kernel/src/mm/mod.rs) rebases a translated frame through); [ADR-0030](../../../decisions/0030-syscall-abi.md) (the [`UserAccessWindow`](../../../../kernel/src/syscall/user_access.rs) + `SyscallError::FaultAddress` copy-user contract this plugs into); [T-016](T-016-mmu-activation.md) (the `vmsav8` encoders `descriptor_bits_to_flags` inverts); [T-019](T-019-task-loader.md) (`LoadedImage{entry_va, stack_top_va}` — the contiguous span the per-task window is derived from).
</code_context>
<issue_to_address>
**issue (typo):** Clarify grammar in the phrase about `vmsav8` encoders and `descriptor_bits_to_flags`.
This parenthetical is grammatically awkward and may confuse readers. Consider rephrasing to "the `vmsav8` encoders that `descriptor_bits_to_flags` inverts" or "`descriptor_bits_to_flags` inverts the `vmsav8` encoders" to clarify the subject–verb relationship.
```suggestion
- **Dependencies:** [ADR-0038](../../../decisions/0038-mmu-translate-and-user-access.md) (drives this task — the `Mmu::translate` surface + the per-task user-access policy); [ADR-0009](../../../decisions/0009-mmu-trait.md) (the `Mmu` trait this extends; §Open-questions "Translation walk queries" is the deferred capability this realises); [ADR-0033](../../../decisions/0033-kernel-high-half-migration.md) (the high-half direct map [`phys_frame_kernel_ptr`](../../../../kernel/src/mm/mod.rs) rebases a translated frame through); [ADR-0030](../../../decisions/0030-syscall-abi.md) (the [`UserAccessWindow`](../../../../kernel/src/syscall/user_access.rs) + `SyscallError::FaultAddress` copy-user contract this plugs into); [T-016](T-016-mmu-activation.md) (the `vmsav8` encoders that `descriptor_bits_to_flags` inverts); [T-019](T-019-task-loader.md) (`LoadedImage{entry_va, stack_top_va}` — the contiguous span the per-task window is derived from).
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bsp-qemu-virt/src/mmu.rs`:
- Around line 325-377: translate currently accepts any L3 descriptor with V=1
and page bit=1 even if the access flag (AF) is clear; update translate (inside
the L3 leaf handling after reading `leaf` in function translate) to check the
AF/access-flag bit on the L3 descriptor and return Err(MmuError::NotMapped) if
AF==0 before constructing the PhysFrame and calling descriptor_bits_to_flags;
ensure you reference the same bit mask used elsewhere for the AF/access flag
(the constant used to test the AF bit) so the check mirrors how other non-usable
descriptors are validated.
In `@kernel/src/syscall/dispatch.rs`:
- Around line 346-348: The unsafe block around calling
FakeMmu::create_address_space(...) must be expanded to explain (a) why unsafe is
necessary here (e.g., create_address_space needs a raw physical root frame and
cannot be expressed safely because it performs low-level address-space setup),
(b) the exact invariants the call upholds (the provided
PhysFrame::from_aligned(PhysAddr(0x1000)) is valid/aligned, the FakeMmu
implementation will store but never dereference the root, mmu remains valid for
the lifetime used, and no aliasing/UB is possible), and (c) why a safe
alternative was rejected (no safe constructor exists that can create an address
space with a fake/root frame in tests or it would require changing API
semantics). Reference the symbols FakeMmu::create_address_space,
PhysFrame::from_aligned, PhysAddr, mmu and the local as_ binding in the comment,
and add a corresponding audit entry to docs/audits/unsafe-log.md describing this
justification.
In `@kernel/src/syscall/user_access.rs`:
- Around line 298-305: The unsafe write must be justified: update the
unsafe-block comment around the kernel->user copy (the block that calls
core::ptr::copy and complements copy_from_user) to explicitly state (a) why
unsafe is required here — e.g., copying into user memory cannot be expressed
with safe APIs because we must operate on raw user-page pointers obtained from
page tables and do an in-place memmove across kernel/user boundary, (b) the
precise invariants being upheld (range validity via window.validate and run/cur
arithmetic, ownership/permission re-check of USER writable leaves from the
probe, disjointness of kernel src vs user dst, and DAIF-masked SVC single-core
protection), and (c) why safer alternatives were rejected (no existing safe
wrapper can perform a zero-copy, checked, in-place copy into a mapped user page
with the required permission re-checks and SVC interrupt masking). Also add an
audit entry referencing UNSAFE-2026-0030 into docs/audits/unsafe-log.md with the
same rationale and the function/context names (mentioning copy_from_user and the
core::ptr::copy usage) so the audit can be traced.
- Around line 147-163: The loop can overflow when advancing the page cursor:
replace the checked_add that returns Err with a non-failing advance (e.g., use
page = page.saturating_add(PAGE_SIZE)) so the page cursor clamps to usize::MAX
instead of triggering a FaultAddress; update the code around user_ptr, len, end,
page and PAGE_SIZE (where page =
page.checked_add(PAGE_SIZE).ok_or(SyscallError::FaultAddress)? is used) to use
saturating_add (or otherwise handle the overflow by setting next = usize::MAX)
so the while page < end termination works correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43f4e15b-253b-46a6-bf5b-58fe88bcc15c
📒 Files selected for processing (18)
bsp-qemu-virt/src/main.rsbsp-qemu-virt/src/mmu.rsbsp-qemu-virt/src/syscall.rsdocs/analysis/tasks/phase-b/T-025-user-access-translation.mddocs/analysis/tasks/phase-b/T-026-current-task-cap-table.mddocs/audits/unsafe-log.mddocs/decisions/0009-mmu-trait.mddocs/decisions/0038-mmu-translate-and-user-access.mddocs/decisions/README.mddocs/roadmap/current.mddocs/roadmap/phases/phase-b.mdhal/src/mmu/mod.rshal/src/mmu/vmsav8.rskernel/src/obj/task_loader.rskernel/src/syscall/dispatch.rskernel/src/syscall/user_access.rstest-hal/src/lib.rstest-hal/src/mmu.rs
Comment on lines
+346
to
+348
| // SAFETY: FakeMmu::create_address_space stores the root, never derefs it. | ||
| let as_ = | ||
| unsafe { mmu.create_address_space(PhysFrame::from_aligned(PhysAddr(0x1000)).unwrap()) }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Expand this unsafe justification.
This note only captures one invariant. It still needs to say why the unsafe call is necessary in this helper and why a safe alternative is not viable, otherwise it doesn't meet the repo's required unsafe-block format.
As per coding guidelines, **/*.rs: All kernel and userspace code must be written in Rust, with every unsafe block documented with comments explaining (a) why it is needed, (b) what invariants it upholds, and (c) why safer alternatives were rejected; audit entries must be logged in docs/audits/unsafe-log.md
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kernel/src/syscall/dispatch.rs` around lines 346 - 348, The unsafe block
around calling FakeMmu::create_address_space(...) must be expanded to explain
(a) why unsafe is necessary here (e.g., create_address_space needs a raw
physical root frame and cannot be expressed safely because it performs low-level
address-space setup), (b) the exact invariants the call upholds (the provided
PhysFrame::from_aligned(PhysAddr(0x1000)) is valid/aligned, the FakeMmu
implementation will store but never dereference the root, mmu remains valid for
the lifetime used, and no aliasing/UB is possible), and (c) why a safe
alternative was rejected (no safe constructor exists that can create an address
space with a fake/root frame in tests or it would require changing API
semantics). Reference the symbols FakeMmu::create_address_space,
PhysFrame::from_aligned, PhysAddr, mmu and the local as_ binding in the comment,
and add a corresponding audit entry to docs/audits/unsafe-log.md describing this
justification.
Comment on lines
+298
to
+305
| // SAFETY: writes `run` bytes from the kernel `src` to the user page. | ||
| // The invariants from [`copy_from_user`] hold with the direction | ||
| // reversed: range validity (`window.validate` + the `run`/`cur` | ||
| // arithmetic), ownership+permission (the probe proved every page is a | ||
| // writable `USER` leaf; re-checked here), disjointness (`src` is a | ||
| // kernel buffer, the destination a userspace page — distinct regions), | ||
| // and no single-core interleaving under the `DAIF`-masked SVC handler. | ||
| // `core::ptr::copy` is the conservative memmove. Audit: UNSAFE-2026-0030. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document why this unsafe write is unavoidable.
The comment covers the invariants, but it still needs the explicit “why unsafe is needed here / why a safe alternative was rejected” part to match the repo's required unsafe-block format.
As per coding guidelines, **/*.rs: All kernel and userspace code must be written in Rust, with every unsafe block documented with comments explaining (a) why it is needed, (b) what invariants it upholds, and (c) why safer alternatives were rejected; audit entries must be logged in docs/audits/unsafe-log.md
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@kernel/src/syscall/user_access.rs` around lines 298 - 305, The unsafe write
must be justified: update the unsafe-block comment around the kernel->user copy
(the block that calls core::ptr::copy and complements copy_from_user) to
explicitly state (a) why unsafe is required here — e.g., copying into user
memory cannot be expressed with safe APIs because we must operate on raw
user-page pointers obtained from page tables and do an in-place memmove across
kernel/user boundary, (b) the precise invariants being upheld (range validity
via window.validate and run/cur arithmetic, ownership/permission re-check of
USER writable leaves from the probe, disjointness of kernel src vs user dst, and
DAIF-masked SVC single-core protection), and (c) why safer alternatives were
rejected (no existing safe wrapper can perform a zero-copy, checked, in-place
copy into a mapped user page with the required permission re-checks and SVC
interrupt masking). Also add an audit entry referencing UNSAFE-2026-0030 into
docs/audits/unsafe-log.md with the same rationale and the function/context names
(mentioning copy_from_user and the core::ptr::copy usage) so the audit can be
traced.
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.
T-025 —
Mmu::translate+ per-task user-access translation (B6 gate #1)Closes the security-critical T-021 carry-forward gate #1 mechanism — the single most important B6 gate. Drives ADR-0038 (Accepted in this branch). Composes ADR-0009 (the
Mmutrait), ADR-0033 (high-half direct map), ADR-0030 (the copy-user contract).The threat this closes
A real EL0 task holding a debug-console capability could pass an in-window kernel VA to
console_write; under B5's bounds-checked int-to-pointer deref the kernel would copy privileged memory to the console — a confused-deputy read. The range bound is necessary but not sufficient: it proves the pointer is in a range, not that it names the task's own memory.Design
Mmu::translate(read-only L0→L3 walk; the deferred ADR-0009 §Open-questions "translation walk query") →(PhysFrame, MappingFlags);NotMapped/BlockMappedon absent/block leaf.QemuVirtMmu::translatereuses the audited walker read-only (UNSAFE-2026-0025 Amendment);vmsav8::descriptor_bits_to_flagsis the inverse decoder (lock-shut, round-trip + property tested).copy_from_user/copy_to_user, now generic over<M: Mmu>): pass 1 probes every spanned page (translate+ requireUSER, +WRITEforcopy_to_user); only if all pass does pass 2 copy, rebasing each frame viaphys_frame_kernel_ptr. Any miss / non-USER/ block page →SyscallError::FaultAddress, never a panic, zero bytes moved (all-or-nothing). The per-pageUSERcheck is the load-bearing boundary; the per-task[entry_va, stack_top_va)window is the cheap first gate.SyscallContextgainsmmu+task_as(generic overM);sys_console_writeprobes the whole range up front so a multi-chunk emit is all-or-nothing.Smoke change (the only trace delta)
The B5
+0x200stub'sconsole_writepasses a kernel.rodataVA → now correctly rejected by theUSER-enforcing translate path (status=0x3FaultAddress,bytes=0, no greeting). This is a positive gate-#1 demonstration (the confused-deputy defence working), not a regression — the SVC still traps +ERETs cleanly (2 SVC exceptions, zero new fault class). A real EL0 task's valid USER buffer is the B6 wire-up.Commit arc
1ed87fd0c79533bb1357ac7f5ef1Mmu::translate+vmsav8decoder (HAL surface)ad943e4SyscallContext<M>+ BSP gate #1 wiring + smokeThe two relayed pre-implementation reviews were verified against the live tree (multi-agent workflow): 8 valid fixes applied (H1 window persistence → scheduler array deferred to T-026; H2 control-plane fail-closed; M1 sim-row reframe; M2 round-trip-over-valid-combos + lock-shut test; M4 mandatory audit AC; two-pass discipline; etc.), 3 skipped with evidence (notably
MmuError::BlockMappedalready exists; the rider past-tense mirrors theMapperFlushprecedent).Gates (all green)
cargo fmt --all --check✅-D warnings✅cargo kernel-build(aarch64) ✅all tasks complete; exactly 2 SVC exceptions, cleanERET, zero new fault class; gate Development #1 rejection shown ✅cargo +nightly miri test --workspace --exclude tyrne-bsp-qemu-virt— 0 UB ✅Audit
UNSAFE-2026-0030 Amendment (per-page translation supersedes the identity-map deref) + UNSAFE-2026-0025 Amendment (read-only
translatecaller). No newunsafeentries.Out of scope (follow-ups)
task_user_windows/task_cap_tablesparallel arrays + fail-closedsyscall_entry).tyrne-user+userland/hellobuild pipeline + the EL0+0x400wire-up smoke.🤖 Generated with Claude Code
Summary by Sourcery
Introduce MMU-level virtual-to-physical translation and use per-task page translation to harden syscall user-memory access.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Documentation
Tests