Skip to content

Remove toyos-untrusted; each decoder bounds the words it is handed - #965

Merged
Japabu merged 1 commit into
mainfrom
wt/toyos-remove-untrusted
Oct 21, 2026
Merged

Japabu merged 1 commit into
mainfrom
wt/toyos-remove-untrusted

Conversation

@Japabu

@Japabu Japabu commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #963 (the transport move). Its base is wt/toyos-sdk-transport, so the diff shows this change alone.

The owner decided (2026-10-21) to remove toyos-untrusted and keep every boundary check. Untrusted<T> only made a bound something to notice in review; it guaranteed nothing:

  • map let a closure narrow the value, or copy it out, before the widening compare.
  • A trivial bound was an unconditional exit: at_most(u32::MAX), which is what blockring's opaque did.

Every check it carried is a single comparison. Each now sits at the decode that produces the validated type, written in plain integers and compared before any narrowing. No replacement wrapper is introduced.

Site by site

site before after
kernel/src/syscall/dispatch.rs (inventory, counters, partition blocks, badge, TRACE_FLOOD, thread name) Untrusted::new(aN).at_most(B) if aN > B { EINVAL }, on the u64 register; TRACE_FLOOD is a match guard that falls through to the existing EINVAL arm
kernel/src/drivers/virtio.rs cap_subwindow, parse index/at_most → CapWindow::Refused(Refused) get(usize::from(bar)), then size > window || offset > window - size, giving CapWindow::NoBar/PastBar with their own Display
kernel/src/drivers/virtio.rs parse_used Untrusted<u32> id/len plain u32 words; still the only way to a DescSlot
kernel/src/drivers/xhci/mod.rs device map(wrapping_sub(1)).index(..), sound only because the wrap missed the bound checked_sub(1)?, then < dev_blocks
toyos-virtio poll_used UsedRefusal::Head(Refused), Written { len: Refused } Head { id, size }, Written { head, len, writable }; the Display keeps the prefix netstack's test matches
toyos-vdagent control and stream index/at_most Event::ALL.get, id >= max_nr_ports, usize::try_from(size) filtered by MAX_CHUNK/MAX_DATA
toyos-transport clamp(Untrusted), pop -> [Untrusted<u32>; E], Run::decode(Untrusted, Untrusted), Inflight::answer(Untrusted) clamp(u32); pop -> [u32; E], the peer's words for the protocol's decoder; Run::decode(u32, u32) leaves the bound to Geometry::run, which already refuses every run past the arena, overflow included (the <= slots pre-check was redundant); answer(u32)
toyos-blockring Request/Completion::decode is(..) chains, opaque; the client re-wrapped an already-decoded tag to call answer match on the op constants, reserved words != 0 refused; opaque and the re-wrap deleted

Also removed:

  • the crate itself, its workspace member, its PUBLISHED entry (build/sdkversion.rs), and its std sources, manifests and [patch] in build/sysroot.rs (plus that file's tests)
  • the rustc-dep-of-std hop in sdk/transport/Cargo.toml
  • three lockfiles' entries
  • stale prose in kernel/pci/src/bar.rs, kernel/dma/Cargo.toml, kernel/src/mm/dma.rs and abuse_thread_name.rs

The #960 record a-crate-new-to-the-published-set-... now owes only toyos-transport. Nothing is published to crates.io.

Fork. rust/library/Cargo.lock drops toyos-untrusted in fork commit e1c5e93d342, pushed fast-forward to the fork's main; this PR moves the rust gitlink to it. The lockfile is what cargo itself resolves: after both image builds the file was byte-identical to the edit (cmp), and std compiled toyos-transport with no toyos-untrusted.

Net: 36 files, +184/−682, most of it the deleted crate.

Checks

check command tree exit log
host suite (87 steps, including transport's loom CONTROLS, e.g. no-clamp) cargo run -- --ci host 6699efb 0 ci-host.log
x86-64 and AArch64 images cargo run -- --build-only --arch <a> 10b6bd5d4's code 0, 0 x86_64, aarch64
QEMU x86-64 rows cargo test --test toyos-build -- nvme_disk_boots the_opening_exchange_with_qemus_agent_completes the_compositor_drives_the_virtio_gpu usb_keyboard_rollover console_image_boots 6699efb 0 guest.log
negative controls controls.sh 10b6bd5d4 each red (101) controls.log, maxdata in full

Why these rows:

  • Every QEMU boot's console is the kernel's virtio console, so each row runs cap_subwindow and parse_used.
  • nvme_disk_boots reads its volume through blockring's decoders.
  • The vdagent row runs the vdagent decode.
  • The virtio-gpu row runs toyos-virtio's used ring.
  • usb_keyboard_rollover binds a device through the kernel xHCI device() lookup.

The controls cover the checks whose removal would otherwise land unseen. Each mutation was applied, built, run red and then restored:

  • blockring's reserved-word refusal
  • Inflight::answer's sequence check
  • virtio's Written bound
  • virtio's head bound, narrowed through as u16 (the defect the crate was named for)
  • vdagent's port bound
  • vdagent's MAX_DATA bound

The transport's cursor clamp already has its own loom control (no-clamp), which the host suite runs.

One host-suite run is red and is published as such:

  • Two inflight reds: the controls script restored files with mv, which brings back their old mtime, so cargo reused the mutated artifacts. The script now touches each restored file.
  • One netstack red, real: I had reworded UsedRefusal::Head's Display, and netstack's a_used_head_past_the_table_ends_the_driver matches its prefix. 6699efb restores the prefix, and the green run above is on that tree.

Metal: requested, not run

T14 RUN REQUESTED (prepared with --metal-readback, which touches no machine): two boots.

  • shared-debug: pci_capability_walk, the kernel's crafted capability windows (15/15), and trace_record_cost (TRACE_FLOOD's guard).
  • testcases: abuse_thread_name (the thread-name refusal) and stream_handles (std streams over the transport).

These are the kernel bounds and std paths that QEMU does not cover. request

Risks

  • Fork pin pushed first. The fork's main is now at e1c5e93d342, while ToyOS main still pins 73f63e2441b until this lands. A build of the fork tip against today's ToyOS main would find the lockfile one entry short. Cargo re-resolves it unless --locked is passed.
  • Stack order. sdk: the transport moves to sdk/transport #963 must land first, or this PR is retargeted to main.

🤖 Generated with Claude Code

The owner decided (2026-10-21) to remove `toyos-untrusted` and keep every
boundary check. `Untrusted<T>` made a bound a review prompt, not a guarantee:
`map` let a closure narrow or copy the value out before the widening compare,
and a trivial bound (`at_most(u32::MAX)`, blockring's `opaque`) was an
unconditional exit. Every check it carried is one comparison, so each is now
written at the decode that makes the validated type, in plain integers,
compared before any narrowing.

- Kernel syscalls: the six register bounds (inventory, counters, partition
  blocks, badge, TRACE_FLOOD, thread name) are direct comparisons of the
  `u64` argument.
- Kernel virtio: `cap_subwindow` refuses with its own `CapWindow::NoBar` and
  `PastBar`; `parse_used` takes the device's two words and stays the only way
  to a `DescSlot`.
- Kernel xHCI: slot 0 is `checked_sub(1)`, not a wrap the bound happened to
  catch.
- `toyos-virtio`: `UsedRefusal::Head { id, size }` and
  `Written { head, len, writable }` carry the numbers, not a borrowed
  `Refused`.
- `toyos-vdagent`: the event, port, chunk and data bounds are direct.
- `toyos-transport`: `Consumer::pop` returns the peer's `[u32; E]`;
  `Run::decode` takes two words and leaves the bound to `Geometry::run`,
  which already refuses every run past the arena, overflow included;
  `Inflight::answer` takes the tag. The `rustc-dep-of-std` hop to the crate
  goes.
- `toyos-blockring`: `Request::decode` and `Completion::decode` match on
  constants and refuse reserved words directly; `opaque` and the client's
  re-wrap of an already-decoded tag go.

The crate, its workspace membership, its `PUBLISHED` entry and std's
`[patch]`, sources and manifests for it are deleted, and the Rust fork's
`library/Cargo.lock` drops it (fork e1c5e93d342). The record of the crates
crates.io lacks now owes only `toyos-transport`.

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

Japabu commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator, T14 at 6699efb (2 boots, image hashes verified): pci_capability_walk, trace_record_cost, abuse_thread_name, stream_handles — 4 passed, 0 failed. Review of #963/#965 is running.

@Japabu

Japabu commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #965 at 6699efb (round 1; diff against origin/wt/toyos-sdk-transport)

Architecture. This matches the owner's decision of 2026-10-21: remove toyos-untrusted, keep every boundary check and the validated types, and add no replacement wrapper.

  • Every decoder still produces its specific type: DescSlot (kernel parse_used is still the only constructor reachable from device words), Run, Request, Completion, Event, the Inflight slot value, and Used/UsedRefusal.
  • Each check is now a plain comparison at the decode.
  • Consumer::pop hands raw [u32; E] to the protocol's decoder, which was already the real boundary.
  • No generic helper replaces the wrapper.
  • Production code shrinks by about 500 lines, about 377 of them the crate. Tests shrink only through deleted peer/plain conversion helpers and the crate's own widening tests. Those tests covered comparisons that are now visible at each site and are held by the callers' tests and the controls below.

Caller-by-caller check against the pre-removal review's table. I compared each site before and after. No check was lost, and no check was moved after a narrowing cast.

  • dispatch.rs, six sites. Each compares the u64 register before use. The partition and badge sites keep their == 0 refusal. SET_THREAD_NAME casts a2 as usize only after the bound. TRACE_FLOOD becomes a guard that falls through to the debug match's _ => InvalidArgument (lines 601/607), the same answer as before.
  • Kernel virtio cap_subwindow. get(usize::from(bar_byte)) replaces the index. size > window is equivalent to the old length <= window given window >= 16, and it is stricter in general. offset <= window - size is compared in u64.
  • Kernel virtio parse. It skips bar_idx >= 6 before indexing, and bar_idx as u8 comes after that check.
  • Kernel parse_used. The head is bounded against chain_bytes.len() (= size: u16) before as u16. len > chain compares two u32s, and chain_bytes is Vec<u32>.
  • xHCI device. checked_sub(1)? then < dev_blocks. It no longer relies on the wrap.
  • toyos-virtio poll_used. The head is bounded before as u16, and len > chain.writable is checked before any slot is cleared. Netstack's should_panic prefix "a used element's head" still matches.
  • vdagent. Event::ALL.get(usize::from(event)) widens, id >= max_nr_ports is checked, and usize::try_from(size) is filtered by MAX_CHUNK/MAX_DATA before use as a length.
  • transport. clamp compares u32 against u32. published, unreleased and reach do the same wrapping arithmetic as before, now without map.
  • Run::decode. It drops the <= slots pre-check. Geometry::run refuses count == 0 and first.checked_add(count)? > slots, and checks the multiplies, so overflow is still refused (the test's (1, u32::MAX) and (u32::MAX, 1) cases).
  • Inflight::answer. The tag is masked, which is the decode, not a narrowing. Then come slots.get_mut and an equality check of the full tag's sequence bits.
  • blockring. match on the constants. A FLUSH with any non-zero field falls to _ => refused. Reserved words use any(!= 0). The opaque laundering and the client's re-wrap are gone.

Fork pin. rust moves 73f63e244 → e1c5e93d342.

  • e1c5e93d342 is a direct child of 73f63e244 on origin/main of ToyOSOrg/rust.
  • It changes only library/Cargo.lock (−8 lines: the toyos-untrusted package and its edge).
  • No other lockfile or manifest names toyos-untrusted: git grep outside issues/ is empty for toyos-untrusted, toyos_untrusted, lib/untrusted and Untrusted.
  • The root, sdk/libc and tests/guest/rust-tests lockfiles drop it in this PR.

This is coherent under implementer.md's fork rules.

Evidence (4369e57f9 round1), inspected.

  • Tested revisions. --ci host and the five QEMU rows ran at 6699efb, EXIT=0. The images and controls ran at 10b6bd5d4. I confirmed that 10b6bd5d4 differs from 6699efb by exactly one Display string in lib/virtio/src/queue.rs, so that evidence still applies.
  • Negative controls. Each of the six reds the named test with exit 101: reserved word, inflight sequence, virtio Written, virtio head narrowed as u16, vdagent port, and vdagent MAX_DATA (whose full log shows a_declared_size_past_the_bound_... FAILED). The as u16 control is the right one for the defect class the crate was named for. The host suite also runs the cursor clamp's own no-clamp loom control.
  • The published red. In the red run's summary, the two red steps are the host workspace (the inflight pair, produced by the restored-mtime artifacts of the inflight-seq mutation) and netstack (the Display prefix). Both explanations are consistent with the code and the controls script, and the green run is on the fixed tree.
  • Metal. The T14 run at 6699efb (orchestrator comment) passed 4/4: pci_capability_walk, trace_record_cost, abuse_thread_name and stream_handles.
  • Gap, not blocking. The kernel's own parse_used, cap_subwindow, xHCI device and syscall bounds have no host test or control of their own. Each is a single comparison that inspection establishes. The metal rows cover cap_subwindow and the two syscall refusals.

Findings

BLOCKER: none.

SUGGESTION (optional):

  • sdk/transport/src/lib.rs:15: this line is 112 columns in a header wrapped at about 78. The edit was not reflowed. Rewrap the paragraph.
  • kernel/src/drivers/xhci/mod.rs:531: "or None when the pool has no room" no longer covers every case, because slot 0 now also gives None explicitly through checked_sub. A short fix: "None for slot 0 or one past the pool".

Process notes for the orchestrator

LAND

Base automatically changed from wt/toyos-sdk-transport to main October 21, 2026 00:33
@Japabu
Japabu marked this pull request as ready for review October 21, 2026 00:35
@Japabu
Japabu enabled auto-merge October 21, 2026 00:35
@Japabu
Japabu added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit dbfd388 Oct 21, 2026
4 checks passed
@Japabu
Japabu deleted the wt/toyos-remove-untrusted branch October 21, 2026 01:33
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