docs(roadmap): B4 milestone closure trio (business + security + performance) + master-review remediation verification#33
Merged
Conversation
…view remediation B4 (Task loader) flips implementation-complete -> Closed via the closure trio (business retrospective + consolidated security review [Approve] + performance baseline), modelled on the 2026-05-14 B3 closure. T-019 merged 2026-05-16 (PR #31); ADR-0029 Accepted 2026-05-14 (PR #30). The period under review included the 2026-05-22 full-tree master review (verdict: APPROVE the shipped kernel -- 0 code-correctness/security Blockers; issues clustered in CI/doc/ADR) and its remediation PR #32. All 24 Blocker+Major findings were re-verified adversarially against the live tree: 23 confirmed-fixed, 1 partial (MR-009). MR-009 is now fully closed in-branch -- phase-b.md gains a "Miri green = Phase-B exit prerequisite" note (the CI-gate half was already done by PR #32). Closing metrics (reproduced live, HEAD 3ab029f, pinned nightly): - cargo host-test 286/286 (43 hal + 187 kernel + 53 test-hal + 3 doc-tests; was 260 at the T-019 merge, +26 from PR #32); fmt/clippy/kernel-build clean. - QEMU smoke runs the full demo through "tyrne: all tasks complete" with the new "tyrne: image loaded (...)" line; -d int,unimp,guest_errors = 629 events, 100% pre-existing PL011 noise, zero fault classes. - Release perf band p10/p50/p90 = 15.641/17.587/19.150 ms (+5.3-5.7 ms vs B3 -- one-time boot cost of the loader's first post-bootstrap cap_map walks under QEMU TCG; real-hardware projection ~40 us). - Audit log 28 entries (UNSAFE-2026-0027 + 0028 added; 0025/0026 Pending-smoke notes lifted by T-019). Side-effects: current.md refreshed (B4 Closed, milestone -> B5, 260->286, trio in Last reviews); 3 review-type README indexes updated; perf-baseline report added. Next milestone: B5 (syscall boundary) -- ADR-0030 + ADR-0031. Refs: ADR-0013, ADR-0029, ADR-0036 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideDocuments-only PR to close milestone B4 (Task loader) by adding its closure trio artefacts (business, security, performance), updating roadmap state and phase-B exit criteria, and wiring the new reviews into the various indexes and perf-reporting docs, including fixing test-count drift and codifying Miri as a Phase-B exit prerequisite. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR records B4 (Task loader) closure via business, performance, and security review documents, updates index/readme entries, and advances the roadmap to mark B4 Closed and activate B5 with a Phase B Miri exit-quality prerequisite. ChangesB4 Closure Trio and Roadmap Progression
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 unit tests (beta)
Comment |
![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 left some high level feedback:
- The new 2026-05-28 banner in
current.mdis extremely dense and repeats a lot of detail that’s already captured in the three B4 closure docs; consider trimming it to a short summary and linking to the business/security/perf artifacts to keepcurrent.mdmaintainable over time. - A lot of the same B4 metrics (test counts, perf band, QEMU event counts, audit-log size) are now reproduced in several places (current.md, business review, perf review, security review); where possible, prefer a single canonical table plus brief references elsewhere to reduce the risk of future drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new 2026-05-28 banner in `current.md` is extremely dense and repeats a lot of detail that’s already captured in the three B4 closure docs; consider trimming it to a short summary and linking to the business/security/perf artifacts to keep `current.md` maintainable over time.
- A lot of the same B4 metrics (test counts, perf band, QEMU event counts, audit-log size) are now reproduced in several places (current.md, business review, perf review, security review); where possible, prefer a single canonical table plus brief references elsewhere to reduce the risk of future drift.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 formally closes the B4 milestone (Task loader) by introducing the B4 closure retrospective reviews (business, security, and performance baseline) and updating the roadmap documentation to reflect the transition to B5 (Syscall boundary). The updates record the post-remediation metrics, including an increased test count of 286 and the performance baseline under QEMU TCG. Feedback on the security review document suggests simplifying a relative link to the task loader document for consistency with sibling reviews.
| @@ -0,0 +1,116 @@ | |||
| # Security review 2026-05-28 — B4 closure consolidated pass (post-T-019 + master-review remediation) | |||
|
|
|||
| - **Change:** the B4 arc on `main` — [T-019 task loader](../../../analysis/tasks/phase-b/T-019-task-loader.md) merged via PR #31 ([merge `7f876af`](https://github.com/HodeTech/Tyrne/commit/7f876af); 7 bisectable commits `911f2ad`/`5711756`/`ae31bc8`/`196d3fb`/`164522d`/`5b1f153`/`95efd62` + doc/round-fix commits `74694d4`/`5078944`/`eb14c51`), preceded by [ADR-0029](../../../decisions/0029-initial-userspace-image-format.md) (Initial userspace image format, `Accepted` 2026-05-14, PR #30 [merge `e09755d`](https://github.com/HodeTech/Tyrne/commit/e09755d)) — *plus* the **master-review remediation** PR #32 ([merge `50bffe9`](https://github.com/HodeTech/Tyrne/commit/50bffe9)) that closed the 2026-05-22 full-tree review's Blocker+Major backlog (commits `a6e909d` MR-001 / `8063ee2` MR-006/005/019/020 + ADR-0036 / `fbc3d3f` MR-002/003/007/008/009 CI honesty / `59f9309` MR-005/011/017/018 / `57bc2e6` MR-010/018 / `348971e` MR-022/017/018 / `24530fb` MR-012/013/014 / `4e241d9` MR-016/019 / `4141158` MR-015/004 / `a2e7257` D3-005/006/007 + review-round commits `ae8fbd7`/`8ceb4fb`/`c843ecd`), the org migration `cd4cb6e` (cemililik/Tyrne → HodeTech/Tyrne), and the README clarity pass `3ab029f` (HEAD). Period under review: 2026-05-14 → 2026-05-28. | |||
There was a problem hiding this comment.
The relative link to the task loader document can be simplified for consistency and directness. Since this file is located at docs/analysis/reviews/security-reviews/2026-05-28-B4-closure.md, we can reference the task loader file using ../../tasks/phase-b/T-019-task-loader.md instead of going up three levels to the docs/ root and then back down. This matches the link structure used in the sibling business and performance reviews.
Suggested change
| - **Change:** the B4 arc on `main` — [T-019 task loader](../../../analysis/tasks/phase-b/T-019-task-loader.md) merged via PR #31 ([merge `7f876af`](https://github.com/HodeTech/Tyrne/commit/7f876af); 7 bisectable commits `911f2ad`/`5711756`/`ae31bc8`/`196d3fb`/`164522d`/`5b1f153`/`95efd62` + doc/round-fix commits `74694d4`/`5078944`/`eb14c51`), preceded by [ADR-0029](../../../decisions/0029-initial-userspace-image-format.md) (Initial userspace image format, `Accepted` 2026-05-14, PR #30 [merge `e09755d`](https://github.com/HodeTech/Tyrne/commit/e09755d)) — *plus* the **master-review remediation** PR #32 ([merge `50bffe9`](https://github.com/HodeTech/Tyrne/commit/50bffe9)) that closed the 2026-05-22 full-tree review's Blocker+Major backlog (commits `a6e909d` MR-001 / `8063ee2` MR-006/005/019/020 + ADR-0036 / `fbc3d3f` MR-002/003/007/008/009 CI honesty / `59f9309` MR-005/011/017/018 / `57bc2e6` MR-010/018 / `348971e` MR-022/017/018 / `24530fb` MR-012/013/014 / `4e241d9` MR-016/019 / `4141158` MR-015/004 / `a2e7257` D3-005/006/007 + review-round commits `ae8fbd7`/`8ceb4fb`/`c843ecd`), the org migration `cd4cb6e` (cemililik/Tyrne → HodeTech/Tyrne), and the README clarity pass `3ab029f` (HEAD). Period under review: 2026-05-14 → 2026-05-28. | |
| - **Change:** the B4 arc on `main` — [T-019 task loader](../../tasks/phase-b/T-019-task-loader.md) merged via PR #31 ([merge `7f876af`](https://github.com/HodeTech/Tyrne/commit/7f876af); 7 bisectable commits `911f2ad`/`5711756`/`ae31bc8`/`196d3fb`/`164522d`/`5b1f153`/`95efd62` + doc/round-fix commits `74694d4`/`5078944`/`eb14c51`), preceded by [ADR-0029](../../../decisions/0029-initial-userspace-image-format.md) (Initial userspace image format, `Accepted` 2026-05-14, PR #30 [merge `e09755d`](https://github.com/HodeTech/Tyrne/commit/e09755d)) — *plus* the **master-review remediation** PR #32 ([merge `50bffe9`](https://github.com/HodeTech/Tyrne/commit/50bffe9)) that closed the 2026-05-22 full-tree review's Blocker+Major backlog (commits `a6e909d` MR-001 / `8063ee2` MR-006/005/019/020 + ADR-0036 / `fbc3d3f` MR-002/003/007/008/009 CI honesty / `59f9309` MR-005/011/017/018 / `57bc2e6` MR-010/018 / `348971e` MR-022/017/018 / `24530fb` MR-012/013/014 / `4e241d9` MR-016/019 / `4141158` MR-015/004 / `a2e7257` D3-005/006/007 + review-round commits `ae8fbd7`/`8ceb4fb`/`c843ecd`), the org migration `cd4cb6e` (cemililik/Tyrne → HodeTech/Tyrne), and the README clarity pass `3ab029f` (HEAD). Period under review: 2026-05-14 → 2026-05-28. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/analysis/reviews/business-reviews/2026-05-28-B4-closure.md`:
- Line 206: Update the MR-009 bullet so its final state is unambiguous: choose
either “closed” or “pending” and make the whole sentence consistent (e.g., if
closed, remove the clause that says the phase-b exit-bar text is still missing;
if pending, remove the “closed in-branch 2026-05-28” tag). Ensure the text
references MR-009 and the phase-b exit-bar change to phase-b.md consistently and
keep the note about where the exit prerequisite should appear (the
§"Exit-quality prerequisite — Miri" paragraph) or remove that note if marking
MR-009 as pending.
In
`@docs/analysis/reviews/performance-optimization-reviews/2026-05-28-B4-closure.md`:
- Line 223: Update the MR-009 status wording in the Phase-B exit narrative to
reflect that the Miri-as-Phase-B-exit-bar change has been completed in-branch:
edit the sentence that currently says "not yet written" to state that the
Phase-B exit checklist now includes Miri as a blocking CI gate (or add an
explicit historical timestamp noting when the pre-fix wording applied); ensure
you reference MR-009 and the corresponding gate entry in infrastructure.md and
adjust the Phase-B exit bar / phase-b.md text to read as a completed action
rather than pending.
In `@docs/analysis/reviews/security-reviews/2026-05-28-B4-closure.md`:
- Line 105: The MR-009 adjustment text in docs/analysis/... states that the
Phase-B Miri prerequisite ("Miri green = Phase-B exit prerequisite") is not yet
written into phase-b.md, but the PR/cohort indicates this was already added;
update the Adjustment text to match the merged state by editing the MR-009 entry
to reflect that the Phase-B exit checklist in roadmap/phases/phase-b.md now
includes the Miri requirement, remove or mark the "not yet written" note, and
add a brief reference to the merge/commit that closed it (or mark as closed) so
future audits do not reopen this item.
🪄 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: b749c579-9868-49e9-a36a-fbd01e33d51d
📒 Files selected for processing (9)
docs/analysis/reports/perf-baseline-2026-05-28-B4-closure.mddocs/analysis/reviews/business-reviews/2026-05-28-B4-closure.mddocs/analysis/reviews/business-reviews/README.mddocs/analysis/reviews/performance-optimization-reviews/2026-05-28-B4-closure.mddocs/analysis/reviews/performance-optimization-reviews/README.mddocs/analysis/reviews/security-reviews/2026-05-28-B4-closure.mddocs/analysis/reviews/security-reviews/README.mddocs/roadmap/current.mddocs/roadmap/phases/phase-b.md
- security review: make the T-019 link direct (../../../analysis/tasks -> ../../tasks/phase-b/T-019-task-loader.md), matching the business/performance sibling reviews. - MR-009 consistency: the Miri-as-Phase-B-exit-prerequisite item was closed in-branch (phase-b.md gained the "Exit-quality prerequisite -- Miri" paragraph), but several artifacts still described it as "not yet written" / a "standing residual". Reconcile every mention to the closed state -- business §What-we-learned + §Adjustments (and the section heading), security §4 + Verdict + Adjustment heading + the audit-log bullet, performance forward-flag, and both README index rows -- so future audits do not reopen it. The remediation now reads 24/24 throughout. - current.md: trim the dense 2026-05-28 banner to a short summary that links the closure trio (the canonical metrics source) instead of reproducing every number, and trim the Last-completed-milestone bullet's metric reproduction to a one-line headline + canonical-source pointer. Reduces future drift risk (overall review comments 1 + 2). Docs-only changeset; 305/305 relative links verified resolving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
May 29, 2026
current.md + T-021 task file cite PR #34 (base main, 9 commits, bundles T-020 + T-021 in one combined review per the maintainer's call). Matches the project's PR-reference convention (cf. T-019/PR #31, B4/PR #33). Refs: ADR-0030, ADR-0031 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
May 29, 2026
…-021) (#34) * docs(adr): propose ADR-0030/0031 — syscall ABI + initial syscall set ADR-0030 settles the EL0->EL1 syscall calling convention (x8 = number, x0-x5 args, x0 status + x1-x7 payload, SVC #0), the dedicated-status error encoding, and the K2-5 split of IpcError::InvalidCapability into StaleHandle / WrongObjectKind / MissingRight (with the per-subject-cap security argument and the arena-staleness ordering caveat). ADR-0031 fixes the v1 syscall set (send, recv, console_write [capability-gated + release debug-gated], task_yield, task_exit), reserves number 0 as invalid, and pins each call's register layout; every object-naming syscall performs a capability check (P1/P4). Opens T-020 (error taxonomy + Capability/CapObject Debug redaction — the pure-Rust foundation, In Progress) and T-021 (SVC trap trampoline + panic-free dispatcher + copy-from/to-user — Ready, the security-critical hardware-facing half) to ground both ADRs' dependency chains per ADR-0025 Rule 1. Both ADRs land at Proposed; Accept follows in a separate commit. Refs: ADR-0030, ADR-0031 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(adr): accept ADR-0030/0031 after careful re-read + maintainer review Flip ADR-0030 and ADR-0031 Proposed -> Accepted in a commit separate from the propose draft, per write-adr skill section 10. The careful re-read plus a same-day maintainer review surfaced and corrected several drafting issues *before* this Accept — all folded into the proposed bodies above, so the Accepted text is correct from the start (no post-Accept body edit): - an SVC from a B5 EL1 kernel-stub takes the current-EL (VBAR_EL1+0x200) sync vector, not the lower-EL (+0x400) EL0 vector, so the real EL0 round-trip is runtime-verified in B6, not B5; - console_write is capability-gated on a debug-console capability (it was ambient authority, a P1/P4 violation); the release debug-gate is a separate, independent defense-in-depth gate; - the syscall numbers 1..5 are a fixed decision (tests regression-verify them), and the payload registers are x1..x7. Adds the additive ADR-0017 revision rider recording that the IpcError taxonomy is refined (not superseded) and the three-primitive surface is unchanged. Refs: ADR-0030, ADR-0031 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ipc): split IpcError::InvalidCapability into three typed variants Per ADR-0030's K2-5 bundle, replace the collapsed IpcError::InvalidCapability with StaleHandle / WrongObjectKind / MissingRight so the in-kernel and the future userspace error spaces agree and each failure is a distinct, handleable case. Validation now resolves in the order resolve -> type-check -> authority (kind before rights), matching CapError's InvalidHandle/WrongKind/ InsufficientRights shape, across validate_ep_cap, validate_notif_cap, and sched::resolve_ep_cap; the four arena-staleness sites map to StaleHandle. Revealing which check failed is safe for a per-subject, unforgeable capability table (ADR-0030 security argument). Remaps the existing rights/stale test assertions and adds 5 new tests pinning each variant (incl. wrong-kind-with- right, proving kind-before-rights, and a destroyed-endpoint StaleHandle). InvalidTransferCap is intentionally left intact (note C3-008). Updates docs/architecture/ipc.md taxonomy section. Security-relevant (capabilities + IPC). fmt / host-test (194 kernel) / host-clippy / kernel-clippy / kernel-build / miri (no UB) all green. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(cap): redact Capability and CapObject Debug to hide object identity Per ADR-0030 "Security of the taxonomy split" / B5 sub-item 6 (K3-9, security review section 6): a userspace-reachable log path (the future console_write syscall) must never disclose the kernel object a capability names. Replace the derived Debug on Capability with a hand-written impl that shows rights but prints the object as <redacted>, and redact CapObject likewise (kind-only Debug, hiding the wrapped slot index + generation). The individual kernel- object handle types keep their derived Debug for kernel-internal diagnostics (they never cross to userspace; T-021's console_write review gates that). Two host tests pin both redaction layers. Broadens security-model.md's "no unredacted Debug/Display" rule to capabilities. The CapObject redaction was folded in from an adversarial self-review that flagged it as a latent defense-in-depth gap (no current production formatter, but conservative per CLAUDE.md rule 1). Security-relevant. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ipc): pin StaleHandle + WrongObjectKind on ipc_cancel_recv Add two tests so ipc_cancel_recv pins all three split variants (it already had MissingRight): a Task cap carrying RECV proves the kind-before-rights ordering (WrongObjectKind), and a cap whose endpoint was destroyed exercises the arena-staleness branch (StaleHandle). This makes ADR-0030's row-3 verification mapping accurate for cancel_recv (it previously over-claimed cancel coverage). Kernel host tests 194 -> 196. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(roadmap): T-020 In Review; narrow B5 acceptance to current-EL proxy Address the remaining maintainer-review findings (the ADR append-only fix landed via the propose/accept rebase; this commit covers the rest): - Major: phase-b §B5 acceptance over-promised a real EL0->EL1 round-trip, which ADR-0030 shows is impossible at B5 (an EL1 kernel-stub SVC takes the current-EL 0x200 vector, not the lower-EL 0x400 EL0 vector). Narrow B5 to "dispatch mechanism verified via the current-EL kernel-stub" and move the real EL0 0x400 round-trip to the B6 acceptance criteria. - Minor: current.md banner said "In Progress" while the fields said "In Review"; fix the banner and the two broken T-021 links (../). - Move T-020 to In Review in the task index + task doc; record the maintainer-review round and the row-to-verification mapping (now incl. the two new cancel_recv variant tests) in T-020's review history. - Add EL0/EL1, SVC, Syscall, and Syscall ABI glossary entries and note the taxonomy split on the ipc.md architecture status row. Refs: ADR-0030, ADR-0031 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ipc): make wrong-kind tests actually prove kind-before-rights Second-round review found the four *_wrong_object_kind tests handed the cap the operation's own right, so they returned WrongObjectKind under *both* the chosen kind-first order and a hypothetical rights-first regression — i.e. ordering-agnostic, proving nothing (a rights-first flip would not fail them). Fix: each test now uses a wrong-kind cap that also LACKS the required right (CapRights::empty()), the only input that discriminates the order (WrongObjectKind under kind-first, MissingRight under rights-first), so a regression to rights-first now fails the tests. Updates the section comment and T-020 AC#4 to state what each test actually proves; corrects T-020's stale test counts (AC#6 194 -> 196; review history "8 new" -> "9 new"). (The stale-variant references in the Turkish technical-analysis IPC chapter were also refreshed on disk for local reference, but that tree is gitignored, so it is not part of this commit / the repo.) No production code change; fmt / host-test 196 / clippy / build / miri (no UB) all green. Refs: ADR-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(syscalls): EL0→EL1 SVC dispatch — trampoline, panic-free dispatcher, copy-user Land the security-critical hardware-facing half of B5 (T-021): the EL0→EL1 SVC trap path that instantiates ADR-0030's calling convention and ADR-0031's five-syscall v1 set. New architecture-agnostic, panic-free, host-tested kernel `syscall` module: - error.rs: SyscallError composing CapError/IpcError via From, with a stable numeric status encoding (0 = Ok; 1-3 top-level; 0x10x = Cap; 0x20x = Ipc). - abi.rs: SyscallNumber decode (release debug-gate on console_write via cfg!(debug_assertions)), the register frame types, value↔register packing for Message/outcomes, and the Option<CapHandle> null-handle sentinel. - user_access.rs: UserAccessWindow + validated copy_from_user/copy_to_user (range-check-then-copy; wrap and zero-length handled; never derefs an unvalidated user pointer). - dispatch.rs: the panic-free dispatcher + per-syscall handlers + the debug-console capability check; control-plane syscalls (task_yield/exit) return a SyscallEffect directive rather than touching the scheduler. Capability surface: CapObject::DebugConsole (singleton, no handle) + CapRights::CONSOLE_WRITE (bit 7, added to KNOWN_BITS) + CapHandle::from_raw (ABI-decode constructor; reconstructed handles are validated by lookup). BSP (hardware-facing): tyrne_sync_trampoline in vectors.s installed at both VBAR_EL1+0x200 (current-EL, the B5 path) and +0x400 (lower-EL AArch64, the B6 EL0 path) — saves the full x0-x30 + SP_EL0 + ELR_EL1 + SPSR_EL1 frame, routes ESR_EL1.EC==SVC64 to a Rust syscall_entry, else to the existing panic path. SyscallTrapFrame (272 B, #[repr(C)], const-asserted to match the asm). kernel_entry runs an EL1 kernel-stub SVC smoke (console_write + bad-number). Gates: fmt / host-clippy / kernel-clippy / kernel-build clean; host tests 236 (+40); cargo test --release green (the debug-gate release-path tests); cargo miri test --workspace --exclude tyrne-bsp-qemu-virt clean (43+236+53). QEMU smoke (debug): two SVCs taken at the current-EL vector (ESR 0x15/SVC64, EL1→EL1), clean ERET; console_write emits its buffer via the syscall path (status 0x0, 63 bytes); a reserved-invalid number returns BadSyscallNumber (0x1); -d int,unimp,guest_errors shows no new fault class; the cooperative demo still runs to "tyrne: all tasks complete". The real EL0 +0x400 round-trip (EL0↔EL1 transition + copy-user against a separate userspace TTBR0_EL1) is wired but runtime-verified in B6 per ADR-0030 §Simulation. Refs: ADR-0030, ADR-0031 Audit: UNSAFE-2026-0029, UNSAFE-2026-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(syscalls): T-021 review-round follow-up — dispatch tests + compile-time payload guard Apply the actionable findings from the T-021 adversarial review-round (which confirmed no live B5 defect). All changes are test coverage, a behavior- preserving defensive refactor, and B6 forward-gate tracking — no production behavior change (QEMU trace byte-stable; the const-generic emits identical register values). Test coverage (+4 dispatch-level tests; host tests 236 -> 240) closing gaps the review surfaced: - send_with_transfer_cap_then_recv_returns_cap_in_x6 — the x5 transfer-handle decode -> ipc_send cap_take AND ipc_recv -> encode_recv_outcome x6 cap-pack, end-to-end through dispatch (previously untested; verified non-vacuous via a mutation check — breaking the x6 pack makes it fail). - send_with_stale_transfer_handle_returns_invalid_transfer_cap — status 0x205. - recv_with_no_sender_returns_pending_packing — Pending packing (x1=pending, x2..x7 zeroed). - console_write_exactly_one_chunk_emits_all_bytes — the len == CONSOLE_WRITE_CHUNK loop boundary (debug-gated). Hardening (nit): SyscallReturn::with_payload is now a const-generic with_payload::<IDX> with `const { assert!(IDX < 7) }`, turning the (already unreachable-from-untrusted-input) runtime index panic into a compile-time error at the call site — matching the kernel's compile-time-guard idiom. Call sites updated to the ::<N> turbofish. Clarity: the three scattered "unreachable re-validation" comments in sys_console_write consolidated into one inequality-chain proof. Docs: phase-b.md §B6 gains an explicit "T-021 carry-forward gates" list (per-task console_write window + per-page user-VA translation returning FaultAddress not panic; SP_EL1 init on the +0x400 entry; SYSCALL_STUB_TABLE -> current-task table) so B6 cannot miss them; T-021 review history records the round. Gates re-run green: fmt / host-clippy / kernel-clippy / kernel-build clean; host-test 240; test --release green; miri --workspace excl BSP clean (43+240+53, 0 UB); QEMU smoke round-trip byte-stable. Refs: ADR-0030, ADR-0031 Audit: UNSAFE-2026-0029, UNSAFE-2026-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(roadmap): record PR #34 (combined T-020 + T-021 B5 review) current.md + T-021 task file cite PR #34 (base main, 9 commits, bundles T-020 + T-021 in one combined review per the maintainer's call). Matches the project's PR-reference convention (cf. T-019/PR #31, B4/PR #33). Refs: ADR-0030, ADR-0031 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(syscalls): T-021 review-round 2 — overlap-safe copy-user + scope/guard fixes Address the second review-round on PR #34. Fix only still-valid issues; one finding (release-wrap of a const) was refuted but its requested guard kept as a clearer compile-time tripwire; one (test-scaffolding helper) skipped. Soundness (headline): copy_from_user / copy_to_user are SAFE `pub fn`s, so they must be sound for every input — but `UserAccessWindow::validate` proves *bounds*, not *disjointness*, and under the v1 identity map (VA == PA) a caller could pass a user_ptr range that aliases the kernel-owned dst/src slice, making `copy_nonoverlapping`'s non-overlap precondition violable from safe code (UB). Switch both moves to `core::ptr::copy` (memmove), which is correct for any overlap; drop the unprovable "source and destination are disjoint" claim from the SAFETY comments and document why `copy` (not `copy_nonoverlapping`) is the sound choice. Behaviour is identical for the non-overlapping case (all current callers are disjoint), so QEMU/Miri/host evidence is unchanged. UNSAFE-2026-0030 gains an append-only Amendment recording the change (title/anchor preserved). Hardening (compile-time guards, no runtime cost): - abi.rs: a `const _: () = assert!(NULL_CAP_HANDLE > max-packable-handle-word)` locks the sentinel-collision-freedom invariant — a future CapHandle widening that could push a packed word into the sentinel's bit range fails the build. - bsp syscall.rs: an explicit `const _: () = assert!(PMM_EXTENT_END >= PMM_EXTENT_START)` with a clear message in front of SYSCALL_USER_WINDOW_LEN. (The reviewer's "wraps in release" premise is incorrect — the subtraction is a `const`, and const-eval rejects underflow at build time, never wraps — but the named assert gives a clearer failure than a raw const-eval overflow error.) Docs: - T-021 §Informs: scope the ADR-0030 §Simulation discharge — rows 2/4 in full + the mechanism half of rows 0/1/5 via the EL1-stub proxy at the current-EL +0x200 vector; the EL0-runtime half of rows 0/1/5 (the +0x400 vector, the EL0↔EL1 transition, copy-user vs a separate userspace TTBR0_EL1) is deferred to B6, not discharged here. - current.md: remove the blank lines between adjacent banner paragraphs so they form one contiguous `>` blockquote (matches the file's existing multi-paragraph style; resolves markdownlint MD028). Skipped: extracting a SyscallContext test-builder — the scaffolding verbosity is largely forced by the borrow structure (tests declare + later inspect the borrowed locals), so a helper saves only the struct-literal line per test and isn't worth churning the just-reviewed test suite on an in-review PR. Gates re-run green: fmt / host-clippy / kernel-clippy / kernel-build; host-test 240; test --release 233; miri --workspace excl BSP clean (0 UB); QEMU smoke round-trip byte-stable. Refs: ADR-0030, ADR-0031 Audit: UNSAFE-2026-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * audit(syscalls): correct UNSAFE-2026-0030 amendment — disjointness is the soundness basis Review-round on PR #34 (two findings; each verified against current code): 1. UNSAFE-2026-0030 amendment (docs/audits/unsafe-log.md) — VALID, fixed. The amendment added in 2c713c0 (a) lacked the commit SHA the audit-log format wants and (b) over-claimed that switching to `core::ptr::copy` makes the copy "overlap-tolerant". An empirical Miri probe disproved that: an overlapping (user_ptr, kernel-slice) pair is UB *regardless* of the copy primitive — `copy_from_user`'s `dst: &mut [u8]` (and `copy_to_user`'s `src: &[u8]`) parameter is exclusive / shared, so an aliasing access through the exposed `user_ptr` violates that borrow (Stacked Borrows: "not granting access to tag <wildcard> … strongly protected"). The amendment now carries SHA 2c713c0, marks the original `copy_nonoverlapping` §Operation / invariant(3) / rejected-alternatives wording as superseded, and states the true soundness basis: the user/kernel **disjointness** invariant (user_ptr = userspace, kernel slice = distinct allocation in v1 / separate AS in B6), under which both `copy` and `copy_nonoverlapping` are sound. `core::ptr::copy` is kept as the conservative primitive. The copy_from_user / copy_to_user SAFETY comments are corrected to match (invariant 3 = disjointness, not "overlap-tolerant"). 2. Add overlapping-copy regression tests — SKIPPED, with reason. The requested tests assert "overlapping copies are allowed", but overlap is UB here (see above — Miri-confirmed via a temporary probe, now removed), independent of `copy` vs `copy_nonoverlapping`. Such tests would (a) break the Miri gate and (b) codify an unsound expectation. The real invariant is disjointness, which the existing tests + the structural user/kernel split already cover; an overlapping call correctly fails under Miri's borrow model. No code-behaviour change (the `core::ptr::copy` calls are unchanged; only SAFETY comments + the audit amendment text). Gates: fmt / host-clippy / kernel-clippy / kernel-build clean; host-test 240; miri (syscall) 0 UB. Production code is byte-identical to 2c713c0, already validated with full miri + test --release + QEMU smoke. Refs: ADR-0030, ADR-0031 Audit: UNSAFE-2026-0030 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <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.
B4 milestone closure trio
Closes the B4 (Task loader) milestone: implementation-complete (T-019, 2026-05-16, PR #31; ADR-0029 Accepted 2026-05-14, PR #30) → Closed, via the standard closure trio modelled on the 2026-05-14 B3 closure.
Artifacts
docs/analysis/reviews/business-reviews/2026-05-28-B4-closure.mddocs/analysis/reviews/security-reviews/2026-05-28-B4-closure.md— verdict: Approve (eight axes pass)docs/analysis/reviews/performance-optimization-reviews/2026-05-28-B4-closure.md(re-baseline) +docs/analysis/reports/perf-baseline-2026-05-28-B4-closure.mdMaster-review remediation verification
The period included the 2026-05-22 full-tree master review (APPROVE the shipped kernel; issues clustered in CI/doc/ADR — 0 kernel-correctness/security Blockers) and its remediation PR #32. All 24 Blocker+Major findings were re-verified adversarially against the live tree: 23 confirmed-fixed, 1 partial (MR-009). MR-009 is now fully closed in-branch —
phase-b.mdgains a "Miri green = Phase-B exit prerequisite" note (the CI-gate half was already in PR #32).Closing metrics (reproduced live, HEAD
3ab029f, pinned nightly)cargo host-testtyrne: all tasks complete;-d int,unimp,guest_errors629 events (100 % pre-existing PL011 noise, zero fault classes)Pending QEMU smoke verificationnotes lifted by T-019)Side-effects (per the conduct-review skill)
current.md→ B4 Closed, active milestone B5, test count 260 → 286, trio added to Last reviews.Correctly deferred (not left behind)
B5 (ADR-0030 syscall ABI + ADR-0031 initial syscall set) is the next milestone (maintainer-sequenced). Trigger-deferred carry-forwards — B5+ MemoryRegion cap, PL011 init BSP task, BSP host-test crate, ADR-0033/0034 placeholders — remain open with their unfired triggers (documented in the business retro §Adjustments).
Docs-only changeset; 305 / 305 relative links verified resolving.
🤖 Generated with Claude Code
Summary by Sourcery
Close the B4 task-loader milestone via its full business, security, and performance closure trio, record the associated master-review remediation status and metrics, and advance the roadmap and documentation toward the B5 syscall boundary work.
Documentation:
Summary by CodeRabbit