diff --git a/Cargo.lock b/Cargo.lock index 73f4c9e18da..bdc4d7057b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1981,7 +1981,6 @@ dependencies = [ "toyos-symbols", "toyos-tco", "toyos-tsc", - "toyos-untrusted", "toyos-usbhid", "toyos-userbound", "toyos-wallclock", @@ -4857,20 +4856,12 @@ version = "0.1.0" dependencies = [ "loom", "rustc-std-workspace-core", - "toyos-untrusted", ] [[package]] name = "toyos-tsc" version = "0.1.0" -[[package]] -name = "toyos-untrusted" -version = "0.1.0" -dependencies = [ - "rustc-std-workspace-core", -] - [[package]] name = "toyos-update" version = "0.1.0" @@ -4901,9 +4892,6 @@ dependencies = [ [[package]] name = "toyos-vdagent" version = "0.1.0" -dependencies = [ - "toyos-untrusted", -] [[package]] name = "toyos-vdagent-bridge" @@ -4917,7 +4905,6 @@ name = "toyos-virtio" version = "0.1.0" dependencies = [ "toyos-device-memory", - "toyos-untrusted", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index f1405cf2d56..a05fee81c99 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,7 +71,6 @@ members = [ "lib/tmpdir", "lib/trace", "lib/tsc", - "lib/untrusted", "lib/update", "lib/userbound", "lib/vdagent", diff --git a/build/sdkversion.rs b/build/sdkversion.rs index 22cb6d2ba0d..3c9c0a6868b 100644 --- a/build/sdkversion.rs +++ b/build/sdkversion.rs @@ -22,7 +22,6 @@ pub struct Crate { /// The published crates, in the order a publisher must take them. pub const PUBLISHED: &[Crate] = &[ Crate { name: "toyos-abi", dir: "sdk/abi" }, - Crate { name: "toyos-untrusted", dir: "lib/untrusted" }, Crate { name: "toyos-transport", dir: "sdk/transport" }, Crate { name: "toyos-keymap", dir: "sdk/keymap" }, Crate { name: "toyos-font", dir: "sdk/font" }, diff --git a/build/sysroot.rs b/build/sysroot.rs index 4932efe775f..6ec7b64ce54 100644 --- a/build/sysroot.rs +++ b/build/sysroot.rs @@ -65,14 +65,13 @@ use crate::toolchain::{self, host_triple, GuestTarget, Owner, Role, GUEST_TARGET use whole_toolchain::{whole, Whole}; /// The per-worktree sources that end up inside a sysroot: std links the -/// `toyos-abi`, `toyos`, `toyos-transport` and `toyos-untrusted` [`std_patch`] -/// hands it and compiles its ToyOS backend from `sdk/std`, and -/// `libtoyos_c.a` is `sdk/libc` with `toyos-elf` and `toyos-osrelease`. -pub const SYSROOT_SOURCES: [&str; 9] = [ +/// `toyos-abi`, `toyos` and `toyos-transport` [`std_patch`] hands it and +/// compiles its ToyOS backend from `sdk/std`, and `libtoyos_c.a` is `sdk/libc` +/// with `toyos-elf` and `toyos-osrelease`. +pub const SYSROOT_SOURCES: [&str; 8] = [ "sdk/abi/src", "sdk/toyos/src", "sdk/transport/src", - "lib/untrusted/src", "sdk/std", "lib/elf/src", "sdk/osrelease/src", @@ -82,11 +81,10 @@ pub const SYSROOT_SOURCES: [&str; 9] = [ /// Their manifests, and the lockfile and cargo configuration libc is built /// under: the features, versions and flags of the same build. -pub(crate) const SYSROOT_MANIFESTS: [&str; 9] = [ +pub(crate) const SYSROOT_MANIFESTS: [&str; 8] = [ "sdk/abi/Cargo.toml", "sdk/toyos/Cargo.toml", "sdk/transport/Cargo.toml", - "lib/untrusted/Cargo.toml", "lib/elf/Cargo.toml", "sdk/osrelease/Cargo.toml", "sdk/libc/Cargo.toml", @@ -97,8 +95,7 @@ pub(crate) const SYSROOT_MANIFESTS: [&str; 9] = [ /// Of [`SYSROOT_MANIFESTS`], the crates std names by version, which std's /// lockfile resolves with the fork's own through [`std_patch`]: what of a /// worktree can move a freestanding target's dependency versions. -const STD_MANIFESTS: [&str; 4] = - ["sdk/abi/Cargo.toml", "sdk/toyos/Cargo.toml", "sdk/transport/Cargo.toml", "lib/untrusted/Cargo.toml"]; +const STD_MANIFESTS: [&str; 3] = ["sdk/abi/Cargo.toml", "sdk/toyos/Cargo.toml", "sdk/transport/Cargo.toml"]; /// One of the crates std names by version, in a worktree. pub(crate) struct StdCrate { @@ -1279,7 +1276,7 @@ pub(crate) mod tests { /// The manifests [`std_crates`] reads, in a worktree at `root` that has /// no other source: what a bootstrap run is handed std's patch from. pub(crate) fn std_manifests(root: &Path) { - for (manifest, name) in STD_MANIFESTS.into_iter().zip(["toyos-abi", "toyos", "toyos-transport", "toyos-untrusted"]) { + for (manifest, name) in STD_MANIFESTS.into_iter().zip(["toyos-abi", "toyos", "toyos-transport"]) { write(&root.join(manifest), &format!("[package]\nname = \"{name}\"\n")); } } @@ -1322,8 +1319,6 @@ pub(crate) mod tests { format!("patch.crates-io.toyos.path=\"{at}/sdk/toyos\""), "--config".to_string(), format!("patch.crates-io.toyos-transport.path=\"{at}/sdk/transport\""), - "--config".to_string(), - format!("patch.crates-io.toyos-untrusted.path=\"{at}/lib/untrusted\""), ], ); for name in ["work tree", "work\"tree", "work\\tree", "work\ttree"] { diff --git a/build/toolchain.rs b/build/toolchain.rs index 39aec9473cc..25907e1aa84 100644 --- a/build/toolchain.rs +++ b/build/toolchain.rs @@ -825,7 +825,7 @@ mod tests { let registry = |name: &str, version: &str| { format!("/home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/{name}-{version}/src/lib.rs") }; - let rest = format!("{} {}", ours("sdk/transport"), ours("lib/untrusted")); + let rest = ours("sdk/transport"); let built_from = |abi: &str, toyos: &str| { fs::write(built.join("dist/libstd.d"), format!("{}/dist/libstd.rlib: {abi} {toyos} {rest}\n", built.display())) .unwrap(); diff --git a/issues/a-crate-new-to-the-published-set-reds-every-publish-until-someone-puts-it-up-by-hand.md b/issues/a-crate-new-to-the-published-set-reds-every-publish-until-someone-puts-it-up-by-hand.md index 4dfffc8cda9..9f35678eda2 100644 --- a/issues/a-crate-new-to-the-published-set-reds-every-publish-until-someone-puts-it-up-by-hand.md +++ b/issues/a-crate-new-to-the-published-set-reds-every-publish-until-someone-puts-it-up-by-hand.md @@ -40,7 +40,10 @@ adds to `PUBLISHED` is not on crates.io. Owner: the owner's crates.io account for the first versions; `build/sdkversion.rs` for the rule. -**Exit**: `toyos-untrusted` and `toyos-transport` are on crates.io, each naming -`publish.yml` its trusted publisher; a `publish` run at main's tip and the next +`toyos-untrusted` has since been removed from the tree and from `PUBLISHED`, +so `toyos-transport` is the one crate still owed. + +**Exit**: `toyos-transport` is on crates.io, naming `publish.yml` its trusted +publisher; a `publish` run at main's tip and the next nightly's `release` are green; and `PUBLISHED`'s doc comment says a crate joins it only once crates.io holds it under that publisher. diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 4d2c908f539..10def215757 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -499,7 +499,6 @@ toyos-rootimage = { path = "../lib/rootimage" } toyos-quiesce = { path = "../lib/quiesce" } toyos-random = { path = "../lib/random" } toyos-symbols = { path = "../lib/symbols" } -toyos-untrusted = { path = "../lib/untrusted" } toyos-usbhid = { path = "usbhid" } toyos-userbound = { path = "../lib/userbound" } toyos-wallclock = { path = "../lib/wallclock" } diff --git a/kernel/dma/Cargo.toml b/kernel/dma/Cargo.toml index 7a4e29b17f4..5111c3b5cf2 100644 --- a/kernel/dma/Cargo.toml +++ b/kernel/dma/Cargo.toml @@ -1,6 +1,5 @@ -# A member of the host workspace (root `Cargo.toml`), like toyos-userbound, -# toyos-untrusted and toyos-pci: it is pure, it has no dependencies, and its -# tests run on the host. +# A member of the host workspace (root `Cargo.toml`), like toyos-userbound and +# toyos-pci: it is pure, it has no dependencies, and its tests run on the host. # # What lives here is where a typed access into a DMA region lands and whether it # may land there. The kernel's `mm::Dma` is the only caller; it is a crate diff --git a/kernel/pci/src/bar.rs b/kernel/pci/src/bar.rs index 7ab7c6770ad..527ca28e95a 100644 --- a/kernel/pci/src/bar.rs +++ b/kernel/pci/src/bar.rs @@ -19,12 +19,7 @@ //! these bits are the device's, and a field that says "this is not memory" is //! not a field to decode past. //! -//! **Why this is a decode and not a `toyos_untrusted::Untrusted`.** That type -//! is for a number used as an *index* or a *length*, where the fix is a -//! comparison with a bound. Nothing here is indexed and nothing is sized: the -//! defect was reading the register as the wrong *kind* of thing, and the fix is -//! a type that has no address in it until the encoding said there was one. -//! [`Width`] is that type — a caller cannot reach an address without having +//! [`Width`] is the decode — a caller cannot reach an address without having //! been handed a [`Memory`], and it cannot be handed one for an I/O BAR. //! //! **What this does not bound is the address.** A BAR that says memory names a diff --git a/kernel/src/drivers/virtio.rs b/kernel/src/drivers/virtio.rs index f8dee90b0e0..b4f3b82e9f8 100644 --- a/kernel/src/drivers/virtio.rs +++ b/kernel/src/drivers/virtio.rs @@ -1,7 +1,5 @@ use crate::arch::barrier; -use toyos_untrusted::{Refused, Untrusted}; - use crate::mm::Mmio; use super::pci::PciDevice; use crate::mm::policy::MmioPolicy; @@ -92,8 +90,21 @@ struct VirtioPciConfig { /// Why a virtio PCI capability names no config window, or the window it names. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CapWindow { - Refused(Refused), Unmapped, + NoBar { index: u8 }, + PastBar { offset: u32, length: u32, window: u64 }, +} + +impl core::fmt::Display for CapWindow { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Unmapped => write!(f, "names a BAR no window was mapped for"), + Self::NoBar { index } => write!(f, "names BAR {index}, past the six a header has"), + Self::PastBar { offset, length, window } => { + write!(f, "names {length:#x} bytes at {offset:#x}, past its BAR's {window:#x}") + } + } + } } /// The required capability a device's chain never produced a window for; its @@ -168,15 +179,16 @@ fn cap_subwindow( offset: u32, length: u32, ) -> Result { - let bar_idx = Untrusted::new(bar_byte).index(mapped_bars.len()).map_err(CapWindow::Refused)?; - let bar = mapped_bars[bar_idx].as_ref().ok_or(CapWindow::Unmapped)?; + let slot = mapped_bars.get(usize::from(bar_byte)).ok_or(CapWindow::NoBar { index: bar_byte })?; + let bar = slot.as_ref().ok_or(CapWindow::Unmapped)?; // The mapped window is the BAR's advertised size or the ceiling, never under 16 bytes. let window = bar.size(); - let size = (length as u64).max(4); - // length ≤ window keeps `window - size` from underflowing and offset + size inside the window. - Untrusted::new(length).at_most(window).map_err(CapWindow::Refused)?; - let offset = Untrusted::new(offset).at_most(window - size).map_err(CapWindow::Refused)?; - Ok(bar.subregion(offset, size)) + let size = u64::from(length).max(4); + // size ≤ window keeps `window - size` from underflowing and offset + size inside the window. + if size > window || u64::from(offset) > window - size { + return Err(CapWindow::PastBar { offset, length, window }); + } + Ok(bar.subregion(u64::from(offset), size)) } impl VirtioPciConfig { @@ -191,7 +203,10 @@ impl VirtioPciConfig { for cap in pci_dev.capabilities() { if cap.id() != PCI_CAP_ID_VENDOR { continue; } // The BAR index is the device's; one past the six the array holds skips the capability rather than indexing it. - let Ok(bar_idx) = Untrusted::new(cap.read_u8(4)).index(mapped_bars.len()) else { continue }; + let bar_idx = usize::from(cap.read_u8(4)); + if bar_idx >= mapped_bars.len() { + continue; + } if mapped_bars[bar_idx].is_none() { // A capability naming a non-memory or unsizable BAR maps to no window. match pci_dev.memory_bar(bar_idx as u8) { @@ -226,7 +241,7 @@ impl VirtioPciConfig { let mmio = match cap_subwindow(&mapped_bars, cap.read_u8(4), cap.read_u32(8), cap.read_u32(12)) { Ok(mmio) => mmio, Err(CapWindow::Unmapped) => continue, - Err(CapWindow::Refused(why)) => { + Err(why) => { log!("VirtIO: PCI {:02x}:{:02x}.{} capability {why} — skipped", pci_dev.bus, pci_dev.dev, pci_dev.func); continue; @@ -354,13 +369,12 @@ impl<'pool> Virtqueue<'pool> { USED_RING_OFF + i as usize * USED_ELEM_SIZE } - /// The two fields of a used element, returned as [`Untrusted`] so no caller can use them without naming a bound. - /// The only reads of a used element's fields in this queue; nothing else here touches them unwrapped. - fn used_ring_id(&self, i: u16) -> Untrusted { - Untrusted::new(self.used.read(self.used_elem_at(i))) + /// The two fields of a used element, as the device wrote them: [`Self::parse_used`] is their only reader. + fn used_ring_id(&self, i: u16) -> u32 { + self.used.read(self.used_elem_at(i)) } - fn used_ring_len(&self, i: u16) -> Untrusted { - Untrusted::new(self.used.read(self.used_elem_at(i) + 4)) + fn used_ring_len(&self, i: u16) -> u32 { + self.used.read(self.used_elem_at(i) + 4) } /// The initial pool of descriptor slots; call once after construction. @@ -462,17 +476,15 @@ impl<'pool> Virtqueue<'pool> { /// What a used-ring element must satisfy: a head inside this queue's table, at a published /// chain, written no further than that chain. Refused rather than clamped: there is nothing /// here to recover from a forged completion. - fn parse_used(&self, id: Untrusted, len: Untrusted) -> Option<(DescSlot, u32)> { + fn parse_used(&self, id: u32, len: u32) -> Option<(DescSlot, u32)> { // `chain_bytes` is exactly `size` long: the descriptor table's own bound, not a constant beside it. - let head = id.index(self.chain_bytes.len()).ok()?; + let head = usize::try_from(id).ok().filter(|&head| head < self.chain_bytes.len())?; let chain = self.chain_bytes[head]; - if chain == 0 { + if chain == 0 || len > chain { return None; } - let written = len.at_most(chain as u64).ok()?; - // Exact: `index` proved `head < size`, a `u16`, and `at_most` that `written` is no more - // than `chain`, a `u32`. - Some((DescSlot(head as u16), written as u32)) + // Exact: `head < size`, a `u16`. + Some((DescSlot(head as u16), len)) } /// Submit a descriptor chain and block until the device completes it, returning the recovered `DescSlot`. diff --git a/kernel/src/drivers/xhci/mod.rs b/kernel/src/drivers/xhci/mod.rs index c906403e550..fba836a0a50 100644 --- a/kernel/src/drivers/xhci/mod.rs +++ b/kernel/src/drivers/xhci/mod.rs @@ -21,7 +21,6 @@ use crate::mm::Dma; use crate::log; use super::pci::PciDevice; use crate::sync::{OwedGuard, OwedLock}; -use toyos_untrusted::Untrusted; use toyos_xhci::job::{Await, Outcome, Outstanding, Stages, CC_SHORT_PACKET, CC_SUCCESS}; use toyos_xhci::port::{self as portmachine, GaveUp, Gone, PortState, Reset, Step}; use toyos_xhci::call::AfterBreak; @@ -530,10 +529,11 @@ impl Layout { } /// The block for a 1-based slot id, or `None` when the pool has no room; `slot_id` is untrusted, from the controller's own answer. - /// - /// `wrapping_sub` is sound only because `index` bounds the result: slot 0 wraps to `u8::MAX`, never `< dev_blocks`. fn device(&self, slot_id: u8) -> Option { - let index = Untrusted::new(slot_id).map(|v| v.wrapping_sub(1)).index(self.dev_blocks).ok()?; + let index = usize::from(slot_id.checked_sub(1)?); + if index >= self.dev_blocks { + return None; + } Some(self.dev_base + index * DEV_STRIDE) } diff --git a/kernel/src/mm/dma.rs b/kernel/src/mm/dma.rs index e4ae299869f..0081a1370ff 100644 --- a/kernel/src/mm/dma.rs +++ b/kernel/src/mm/dma.rs @@ -129,7 +129,7 @@ impl<'pool, D: Discipline> Dma<'pool, D> { unsafe { copy_nonoverlapping(self.base.add(offset), dst.as_mut_ptr(), dst.len()) } } - // Panics, not `Result`: every offset/length here is the driver's own arithmetic, never a device-chosen number (those arrive as `Untrusted` and are bounded before becoming an offset). + // Panics, not `Result`: every offset/length here is the driver's own arithmetic, never a device-chosen number (those are bounded at their decode before becoming an offset). #[inline] fn check(self, offset: usize, len: usize) { if let Err(why) = toyos_dma::within(offset, len, self.size) { diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index abb8c5d08aa..78b68c1c410 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -17,7 +17,6 @@ use toyos_abi::handle::{RawHandle, Rights}; #[cfg(feature = "test-actuators")] use toyos_abi::syscall::debug_action as DA; use toyos_abi::syscall::*; -use toyos_untrusted::Untrusted; use super::HANDLE_LEN; #[cfg(feature = "test-actuators")] @@ -268,9 +267,10 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> // A census of the hardware and of who drives it, so it costs // Rights::INVENTORY. The count is bounded before it becomes a window. SYS_DEVICE_INVENTORY => { - let Ok(count) = Untrusted::new(a3).at_most(MAX_INVENTORY_RECORDS) else { + let count = a3; + if count > MAX_INVENTORY_RECORDS { return SyscallError::InvalidArgument.to_u64(); - }; + } let len = count * toyos_abi::inventory::RECORD_BYTES as u64; let Some(mut buf) = ctx.user_bytes_mut(UserAddr::new(a2), len) else { return bad_addr }; sys_device_inventory(RawHandle(a1 as u32), &mut buf) @@ -278,9 +278,10 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> // What every CPU counts, so it costs Rights::COUNTERS; a CPU per record, // so the count is bounded by the most CPUs there can be. SYS_COUNTERS => { - let Ok(count) = Untrusted::new(a3).at_most(crate::scheduler::MAX_CPUS as u64) else { + let count = a3; + if count > crate::scheduler::MAX_CPUS as u64 { return SyscallError::InvalidArgument.to_u64(); - }; + } let len = count * toyos_abi::counters::RECORD_BYTES as u64; let Some(mut buf) = ctx.user_bytes_mut(UserAddr::new(a2), len) else { return bad_addr }; sys_counters(RawHandle(a1 as u32), &mut buf) @@ -405,11 +406,8 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> // The count is bounded before it becomes a window, and a transfer of // nothing is refused rather than sent to a device as a zero-length command. SYS_PARTITION_READ | SYS_PARTITION_WRITE => { - let Ok(count) = Untrusted::new(a4).at_most(toyos_abi::part::MAX_BLOCKS_PER_CALL as u64) - else { - return SyscallError::InvalidArgument.to_u64(); - }; - if count == 0 { + let count = a4; + if count == 0 || count > toyos_abi::part::MAX_BLOCKS_PER_CALL as u64 { return SyscallError::InvalidArgument.to_u64(); } let len = count * toyos_abi::part::BLOCK_BYTES as u64; @@ -477,10 +475,8 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> SYS_PORT_CREATE => sys_port_create(), SYS_PORT_MINT => { // Copied in before any handle is looked up: the kernel holds the badge, never the caller's page. - let Ok(len) = Untrusted::new(a3).at_most(MAX_BADGE as u64) else { - return SyscallError::InvalidArgument.to_u64(); - }; - if len == 0 { + let len = a3; + if len == 0 || len > MAX_BADGE as u64 { return SyscallError::InvalidArgument.to_u64(); } let Some(bytes) = ctx.user_bytes(UserAddr::new(a2), len) else { return bad_addr }; @@ -602,10 +598,7 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> // bound and its stale answer are the shipped paths. DA::COUNTERS_DEAF => crate::counters::deaf::stage(a2), DA::COUNTERS_HEAR => crate::counters::deaf::end(), - DA::TRACE_FLOOD => match Untrusted::new(a2).at_most(DA::TRACE_FLOOD_MOST) { - Ok(count) => crate::trace::flood(count), - Err(_) => SyscallError::InvalidArgument.to_u64(), - }, + DA::TRACE_FLOOD if a2 <= DA::TRACE_FLOOD_MOST => crate::trace::flood(a2), // An interrupt inside a syscall's body, which nothing a guest does puts there on demand. DA::RING0_TIMER_IN_SYSCALL => ring0_timer_in_syscall(), DA::ACPI_FIRMWARE_LOCK => crate::arch::acpi_mode::debug_firmware_lock(a2), @@ -627,11 +620,11 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> SYS_SET_THREAD_NAME => { // Refused by name, not clamped: a length past the bound is the caller's // bad argument, not a shorter name to silently write. - let Ok(len) = Untrusted::new(a2).at_most(process::THREAD_NAME_LEN as u64) else { + if a2 > process::THREAD_NAME_LEN as u64 { return SyscallError::InvalidArgument.to_u64(); - }; - let len = len as usize; - let Some(bytes) = ctx.user_bytes(UserAddr::new(a1), len as u64) else { + } + let len = a2 as usize; + let Some(bytes) = ctx.user_bytes(UserAddr::new(a1), a2) else { return bad_addr; }; let mut name = [0u8; process::THREAD_NAME_LEN]; diff --git a/lib/blockring/src/client.rs b/lib/blockring/src/client.rs index 7f69ca411d1..73c0485f2a1 100644 --- a/lib/blockring/src/client.rs +++ b/lib/blockring/src/client.rs @@ -36,7 +36,7 @@ use alloc::collections::{BTreeMap, VecDeque}; use alloc::vec::Vec; -use toyos_transport::{Inflight, Run, Untrusted, Violation}; +use toyos_transport::{Inflight, Run, Violation}; use crate::entry::{Completion, Op, Request, Status}; use crate::layout::DEPTH; @@ -196,7 +196,7 @@ impl Client { /// What the server answered. pub fn complete(&mut self, completion: Completion) -> Result<(), Violation> { - let sent = self.wire.answer(Untrusted::new(completion.tag))?; + let sent = self.wire.answer(completion.tag)?; let q = sent.queued; match (q.kind, completion.status) { (Kind::User(ticket), Status::Ok) => { diff --git a/lib/blockring/src/entry.rs b/lib/blockring/src/entry.rs index a6fcf280543..af9ee697dc1 100644 --- a/lib/blockring/src/entry.rs +++ b/lib/blockring/src/entry.rs @@ -4,7 +4,7 @@ //! request is bounded here against the arena and the partition, and a word //! this protocol does not define is a refusal, never a default. -use toyos_transport::{Run, Untrusted}; +use toyos_transport::Run; use crate::layout::{ARENA, CQE_WORDS, MAX_REQUEST_BLOCKS, SQE_WORDS}; @@ -57,22 +57,18 @@ impl Request { /// `partition_blocks`: every block it names is inside both the arena and /// the partition, a transfer moves at least one block and at most /// [`MAX_REQUEST_BLOCKS`], and a flush names nothing. - pub fn decode(words: [Untrusted; SQE_WORDS], partition_blocks: u64) -> Result { + pub fn decode(words: [u32; SQE_WORDS], partition_blocks: u64) -> Result { let [op, tag, lba_low, lba_high, blocks, arena, reserved @ ..] = words; - let tag = opaque(tag); let refused = Refused::Malformed { tag }; - if !reserved.iter().all(|word| word.is(0)) { + if reserved.iter().any(|&word| word != 0) { return Err(refused); } - let lba = u64::from(opaque(lba_low)) | (u64::from(opaque(lba_high)) << 32); - let transfer: fn(Run, u64) -> Op = if op.is(READ) { - |run, lba| Op::Read { run, lba } - } else if op.is(WRITE) { - |run, lba| Op::Write { run, lba } - } else if op.is(FLUSH) && lba == 0 && blocks.is(0) && arena.is(0) { - return Ok(Self { op: Op::Flush, tag }); - } else { - return Err(refused); + let lba = u64::from(lba_low) | (u64::from(lba_high) << 32); + let transfer: fn(Run, u64) -> Op = match op { + READ => |run, lba| Op::Read { run, lba }, + WRITE => |run, lba| Op::Write { run, lba }, + FLUSH if lba == 0 && blocks == 0 && arena == 0 => return Ok(Self { op: Op::Flush, tag }), + _ => return Err(refused), }; let run = Run::decode(arena, blocks, &ARENA).map_err(|_| refused)?; let blocks = run.count(); @@ -83,15 +79,6 @@ impl Request { } } -/// A word every value of which means something: a tag its answer echoes, or -/// half of an `lba` the partition bounds whole. -fn opaque(word: Untrusted) -> u32 { - word.at_most(u32::MAX.into()) - .ok() - .and_then(|word| u32::try_from(word).ok()) - .expect("every u32 is at most u32::MAX") -} - /// What a completion says of its request. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Status { @@ -139,13 +126,13 @@ impl Completion { /// `None` for words no server of this protocol writes: the client treats a /// server that wrote them as one that broke the session. - pub fn decode(words: [Untrusted; CQE_WORDS]) -> Option { + pub fn decode(words: [u32; CQE_WORDS]) -> Option { let [tag, status, reserved @ ..] = words; - if !reserved.iter().all(|word| word.is(0)) { + if reserved.iter().any(|&word| word != 0) { return None; } - let status = Status::ALL.into_iter().find(|s| status.is(s.word()))?; - Some(Self { tag: opaque(tag), status }) + let status = Status::ALL.into_iter().find(|s| s.word() == status)?; + Some(Self { tag, status }) } } @@ -159,10 +146,6 @@ mod tests { ARENA.run(first, count).unwrap() } - fn peer(words: [u32; N]) -> [Untrusted; N] { - words.map(Untrusted::new) - } - #[test] fn a_request_survives_its_words() { let last = ARENA.slots() - MAX_REQUEST_BLOCKS; @@ -171,10 +154,10 @@ mod tests { Request { op: Op::Write { run: run(last, MAX_REQUEST_BLOCKS), lba: 0 }, tag: u32::MAX }, Request { op: Op::Flush, tag: 0 }, ] { - assert_eq!(Request::decode(peer(request.encode()), PARTITION), Ok(request)); + assert_eq!(Request::decode(request.encode(), PARTITION), Ok(request)); } let wide = Request { op: Op::Read { run: run(3, 2), lba: 1 << 40 }, tag: 1 }; - assert_eq!(Request::decode(peer(wide.encode()), u64::MAX), Ok(wide)); + assert_eq!(Request::decode(wide.encode(), u64::MAX), Ok(wide)); } /// Every field a hostile client can set out of range is refused, and the @@ -202,7 +185,7 @@ mod tests { ]; for (what, words) in cases { assert_eq!( - Request::decode(peer(words), PARTITION), + Request::decode(words, PARTITION), Err(Refused::Malformed { tag: 42 }), "{what} was not refused" ); @@ -213,9 +196,9 @@ mod tests { fn a_completion_survives_its_words_and_refuses_what_it_does_not_define() { for status in Status::ALL { let c = Completion { tag: 9, status }; - assert_eq!(Completion::decode(peer(c.encode())), Some(c)); + assert_eq!(Completion::decode(c.encode()), Some(c)); } - assert_eq!(Completion::decode(peer([1, 5, 0, 0])), None); - assert_eq!(Completion::decode(peer([1, 0, 1, 0])), None); + assert_eq!(Completion::decode([1, 5, 0, 0]), None); + assert_eq!(Completion::decode([1, 0, 1, 0]), None); } } diff --git a/lib/blockring/src/model.rs b/lib/blockring/src/model.rs index 433755e272f..3f431ee1daa 100644 --- a/lib/blockring/src/model.rs +++ b/lib/blockring/src/model.rs @@ -35,7 +35,7 @@ use core::sync::atomic::Ordering; use std::collections::HashSet; use toyos_blockhold::Holds; -use toyos_transport::{Consumer, Place, Producer, Untrusted, Violation, Word}; +use toyos_transport::{Consumer, Place, Producer, Violation, Word}; use crate::client::{Client, Outcome, Ticket, MAX_ATTEMPTS}; use crate::entry::{Completion, Op, Request}; @@ -156,7 +156,7 @@ impl Queues { } /// The oldest request, as the server's ring gives it. - fn take(&mut self) -> Option<[Untrusted; SQE_WORDS]> { + fn take(&mut self) -> Option<[u32; SQE_WORDS]> { let want = self.sq.pop_front()?; let words = self.server.0.pop(&self.page).expect("an honest client's tail"); let words = words.expect("the ring holds what the queue does"); @@ -174,7 +174,7 @@ impl Queues { /// The oldest completion, as the client's ring gives it. fn read(&mut self) -> Option { let want = self.cq.pop_front()?; - let words: [Untrusted; CQE_WORDS] = + let words: [u32; CQE_WORDS] = self.client.1.pop(&self.page).expect("an honest server's tail").expect("the ring holds what the queue does"); self.client.1.release(&self.page); assert_eq!(Completion::decode(words), Some(want), "the ring gave the client what the queue holds"); diff --git a/lib/blockring/src/server.rs b/lib/blockring/src/server.rs index ef94f3e3737..c2745355485 100644 --- a/lib/blockring/src/server.rs +++ b/lib/blockring/src/server.rs @@ -18,7 +18,6 @@ use alloc::vec::Vec; use toyos_blockhold::{Holds, Writer}; -use toyos_transport::Untrusted; use crate::entry::{Completion, Op, Refused, Request, Status}; use crate::layout::SQE_WORDS; @@ -81,7 +80,7 @@ impl ServerSession { /// not let it make, are answered at once and never reach the device: the /// second would make one tag two requests, and the client could not tell /// which answer was whose. - pub fn take(&mut self, words: [Untrusted; SQE_WORDS]) -> Taken { + pub fn take(&mut self, words: [u32; SQE_WORDS]) -> Taken { let request = match Request::decode(words, self.blocks) { Ok(request) => request, Err(Refused::Malformed { tag }) => { @@ -141,11 +140,11 @@ mod tests { use super::*; use crate::layout::ARENA; - fn write(tag: u32) -> [Untrusted; SQE_WORDS] { - Request { op: Op::Write { run: ARENA.run(0, 1).unwrap(), lba: 0 }, tag }.encode().map(Untrusted::new) + fn write(tag: u32) -> [u32; SQE_WORDS] { + Request { op: Op::Write { run: ARENA.run(0, 1).unwrap(), lba: 0 }, tag }.encode() } - fn flush(tag: u32) -> [Untrusted; SQE_WORDS] { - Request { op: Op::Flush, tag }.encode().map(Untrusted::new) + fn flush(tag: u32) -> [u32; SQE_WORDS] { + Request { op: Op::Flush, tag }.encode() } #[test] @@ -201,7 +200,7 @@ mod tests { assert_eq!(session.take(write(1)), Taken::Answer(Completion { tag: 1, status: Status::ReadOnly })); assert_eq!(session.inflight().len(), 0); let read = Request { op: Op::Read { run: ARENA.run(0, 1).unwrap(), lba: 0 }, tag: 2 }; - assert!(matches!(session.take(read.encode().map(Untrusted::new)), Taken::Issue(_))); + assert!(matches!(session.take(read.encode()), Taken::Issue(_))); assert!(matches!(session.take(flush(3)), Taken::Issue(_))); } } diff --git a/lib/untrusted/Cargo.toml b/lib/untrusted/Cargo.toml deleted file mode 100644 index 89f5e1cd396..00000000000 --- a/lib/untrusted/Cargo.toml +++ /dev/null @@ -1,24 +0,0 @@ -# A member of the host workspace (root `Cargo.toml`), like toyos-pci and -# toyos-gpt: it is pure, it has no dependencies, and its tests run on the host. -# -# What lives here is one type. Every other pure crate in this tree decodes one -# *format* a device or a peer published — a capability structure, a GPT header, -# an ELF file — and each of them reaches, at the bottom, for the same sentence: -# this number crossed a trust boundary, so it is not an index until something -# compared it with the table. That sentence had no type, so it was written by -# hand where somebody remembered to and not written where nobody did. - -[package] -name = "toyos-untrusted" -description = "A number that crossed a trust boundary, and the four ways it gets out." -version = "0.1.0" # never bumped; crates.io's is assigned on publish (build/sdkversion.rs) -edition = "2021" -license = "MIT OR Apache-2.0" -repository = "https://github.com/ToyOSOrg/ToyOS" - -[features] -# std links it through `toyos-transport`. -rustc-dep-of-std = ["core"] - -[dependencies] -core = { version = "1.0.0", optional = true, package = "rustc-std-workspace-core" } diff --git a/lib/untrusted/src/lib.rs b/lib/untrusted/src/lib.rs deleted file mode 100644 index f3aa2d5858e..00000000000 --- a/lib/untrusted/src/lib.rs +++ /dev/null @@ -1,377 +0,0 @@ -//! A number that crossed a trust boundary, and the four ways it gets out. -//! -//! CLAUDE.md's law is *"input that crossed a trust boundary is never trusted -//! and never panics the kernel — it is refused"*, and the recorded failures of -//! it are all one shape: a `u32` a device wrote, or a `u16` a peer wrote, was -//! carried in a plain integer and then used as an index, a length or an -//! address. A plain integer records nothing about where it came from, so the -//! comparison that should stand between it and the array is a thing an author -//! has to remember. Six filed issues are six authors who did not. -//! -//! [`Untrusted`] is that provenance, in the type. It has **no** arithmetic, -//! no `Deref`, no `From`, no cast and no accessor: the only functions on it -//! that return a bare integer take the bound to check against and answer -//! [`Result`]. So a value that came off DMA cannot reach an index without the -//! comparison, and a reviewer does not have to notice that it should — -//! the compiler refuses the code. -//! -//! **This is not a second vocabulary.** `UserAddr` (`kernel/src/mm/mod.rs`) is -//! the same idea for the *address* half of the same boundary, and says so: -//! *"the type's name is a claim, and this is the only constructor that makes it -//! true of a number userland chose"*. What was missing was the value half. -//! `toyos-pci`'s module doc already states the principle for one format — -//! *"a function that names a reserved BAR indicator is not a function to -//! truncate into range"* — and this is the noun that sentence was reaching for. -//! -//! # What it costs -//! -//! Nothing at runtime. [`Untrusted`] is `#[repr(transparent)]` over `T` and -//! every method is `#[inline]`, so it compiles to the integer it wraps; the -//! checks are the compare-and-branch a correct call site had to perform anyway, -//! and the incorrect ones were only cheaper because they were wrong. -//! -//! # What it does not do -//! -//! It does not decide *what* the bound is — that is the driver's knowledge and -//! it stays at the driver. It carries no source: a value a device wrote and a -//! value a peer wrote are the same kind of not-known-yet, and splitting them -//! would put the interesting question ("which of these is reachable from -//! userland?") into a type where it cannot be answered instead of into the -//! module header where it can. -//! -//! `no_std`, no allocation, no dependencies, `forbid(unsafe_code)`. - -#![no_std] -#![forbid(unsafe_code)] - -use core::fmt; - -/// The integer widths a boundary hands over. Sealed: the point of the type is -/// that the set of things it can be is closed. -mod sealed { - pub trait Sealed {} -} - -/// An integer that can be widened losslessly for comparison. -pub trait Raw: sealed::Sealed + Copy + fmt::Debug { - /// This value as a `u64`, which every bound below is expressed in. Lossless - /// for every implementor, which is what makes a comparison against a bound - /// mean what it says. - fn widen(self) -> u64; -} - -macro_rules! raw { - ($($t:ty),*) => { $( - impl sealed::Sealed for $t {} - impl Raw for $t { - #[inline] - fn widen(self) -> u64 { self as u64 } - } - )* }; -} - -raw!(u8, u16, u32, u64); - -/// Why a value that crossed a boundary is not the thing it was asked to be. -/// -/// Carries both numbers, so the caller's message can name the noun and this can -/// name the disagreement — `msix::Unusable`'s division of labour, which is what -/// keeps a refusal readable without every bound owning its own error enum. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Refused { - /// It was asked to be an index into a table of `len` entries and is not. - PastTable { value: u64, len: u64 }, - /// It was asked to be at most `bound` and is more. - PastBound { value: u64, bound: u64 }, - /// It was asked to be exactly `wanted`. - NotExactly { value: u64, wanted: u64 }, -} - -impl fmt::Display for Refused { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::PastTable { value, len } => write!(f, "{value} indexes no entry of {len}"), - Self::PastBound { value, bound } => write!(f, "{value} is past the bound of {bound}"), - Self::NotExactly { value, wanted } => write!(f, "{value} where {wanted} was required"), - } - } -} - -/// A number this side of the machine did not choose. -/// -/// Wrap one at the read that produces it — the volatile load out of a used -/// ring, the config-space dword, the header field — and it stays wrapped until -/// something compares it with a bound. There is deliberately no way to unwrap -/// it without one: -/// -/// ```compile_fail -/// # use toyos_untrusted::Untrusted; -/// let len = Untrusted::new(4096u32); -/// let n: u32 = len.into(); // no `From`, no `Into` -/// ``` -/// -/// ```compile_fail -/// # use toyos_untrusted::Untrusted; -/// let id = Untrusted::new(3u32); -/// let next = id + 1; // no arithmetic -/// ``` -/// -/// ```compile_fail -/// # use toyos_untrusted::Untrusted; -/// let id = Untrusted::new(3u32); -/// let table = [0u8; 16]; -/// let entry = table[id as usize]; // no cast -/// ``` -/// -/// ```compile_fail -/// # use toyos_untrusted::Untrusted; -/// let id = Untrusted::new(3u32); -/// if id < 16 { } // no ordering against a bare integer -/// ``` -/// -/// What it is for, in one line each: -/// -/// ``` -/// # use toyos_untrusted::{Untrusted, Refused}; -/// let table = [0u8; 16]; -/// // The device named descriptor 3 of 16 — an index, once compared. -/// assert_eq!(Untrusted::new(3u32).index(table.len()), Ok(3)); -/// // And descriptor 0x1_0003, which narrows to 3 and is not. -/// assert_eq!( -/// Untrusted::new(0x1_0003u32).index(table.len()), -/// Err(Refused::PastTable { value: 0x1_0003, len: 16 }), -/// ); -/// ``` -#[derive(Clone, Copy, PartialEq, Eq)] -#[repr(transparent)] -pub struct Untrusted(T); - -impl Untrusted { - /// Wrap a value that came from outside. - /// - /// Unrestricted on purpose: *making* one of these is always sound, and a - /// constructor nobody could call would only move the laundering to whoever - /// could. What is restricted is getting a number back out. - #[inline] - pub const fn new(value: T) -> Self { - Self(value) - } - - /// As an index into a table of `len` entries. - /// - /// The one exit that produces a `usize`, and the reason: a `usize` is what - /// indexes, so every path to one goes past this comparison. `len` is the - /// table's own length — `slice.len()`, or the queue size the descriptor - /// table was allocated from — never a constant retyped beside it. - #[inline] - pub fn index(self, len: usize) -> Result { - let value = self.0.widen(); - if value >= len as u64 { - return Err(Refused::PastTable { value, len: len as u64 }); - } - Ok(value as usize) - } - - /// As a count, size or length of at most `bound`. - /// - /// Inclusive, because the values this guards are byte counts and a buffer - /// entirely filled is the ordinary case. - #[inline] - pub fn at_most(self, bound: u64) -> Result { - let value = self.0.widen(); - if value > bound { - return Err(Refused::PastBound { value, bound }); - } - Ok(value) - } - - /// As exactly `wanted` — for a field a protocol fixes, where anything else - /// is not a value to clamp but a peer that is not speaking the protocol. - #[inline] - pub fn exactly(self, wanted: T) -> Result { - let (value, wanted) = (self.0.widen(), wanted.widen()); - if value != wanted { - return Err(Refused::NotExactly { value, wanted }); - } - Ok(value) - } - - /// Whether it is this value. Answering a `bool` decides nothing about - /// memory, so this is not an exit — the caller learns only what it already - /// wrote down. - #[inline] - pub fn is(self, value: T) -> bool { - self.0.widen() == value.widen() - } - - /// Decode it — mask a field out, shift, narrow — and keep the provenance. - /// - /// **The escape hatch that is not one.** Arithmetic on a value from outside - /// is ordinary and necessary; what must not happen is the *result* becoming - /// trusted because a computation happened to it. So this exists, and it - /// returns another [`Untrusted`]. - #[inline] - pub fn map(self, f: impl FnOnce(T) -> U) -> Untrusted { - Untrusted(f(self.0)) - } -} - -/// Prints the number, so a refusal can name what arrived. -/// -/// Formatting is not an exit: it produces text, and no caller indexes with -/// text. Without it every refusal would have to launder the value to say what -/// it refused, which is the one thing that would make laundering necessary. -impl fmt::Debug for Untrusted { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "Untrusted({:?})", self.0) - } -} - -impl fmt::Display for Untrusted { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, f) - } -} - -impl fmt::LowerHex for Untrusted { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::LowerHex::fmt(&self.0, f) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn an_index_is_below_the_table_it_indexes() { - assert_eq!(Untrusted::new(0u32).index(16), Ok(0)); - assert_eq!(Untrusted::new(15u32).index(16), Ok(15)); - assert_eq!( - Untrusted::new(16u32).index(16), - Err(Refused::PastTable { value: 16, len: 16 }) - ); - } - - /// The defect this type is named for. `desc_to_rx` is `[u8; 16]` and the - /// used ring's id is a `u32`; the code that read it wrote `id as u16`, so - /// `0x1_0003` became descriptor 3 — a real entry — and every check written - /// after the cast agreed. Widening rather than narrowing is the whole - /// difference, and it is not a thing a call site can get wrong here. - #[test] - fn a_value_whose_low_bits_are_in_range_is_still_out_of_range() { - assert_eq!( - Untrusted::new(0x1_0003u32).index(16), - Err(Refused::PastTable { value: 0x1_0003, len: 16 }) - ); - assert_eq!( - Untrusted::new(0xFFFF_0000u32).index(16), - Err(Refused::PastTable { value: 0xFFFF_0000, len: 16 }) - ); - } - - #[test] - fn an_empty_table_indexes_nothing() { - assert_eq!( - Untrusted::new(0u32).index(0), - Err(Refused::PastTable { value: 0, len: 0 }) - ); - } - - /// A length is inclusive of its bound and an index is not, and the two - /// exits are separate so that a call site cannot pick the wrong one by - /// writing `len - 1` and getting it wrong at the boundary. - #[test] - fn a_length_may_equal_its_bound_where_an_index_may_not() { - assert_eq!(Untrusted::new(256u32).at_most(256), Ok(256)); - assert_eq!( - Untrusted::new(257u32).at_most(256), - Err(Refused::PastBound { value: 257, bound: 256 }) - ); - assert_eq!( - Untrusted::new(256u32).index(256), - Err(Refused::PastTable { value: 256, len: 256 }) - ); - } - - #[test] - fn every_width_widens_rather_than_wrapping() { - assert_eq!(Untrusted::new(u8::MAX).at_most(255), Ok(255)); - assert_eq!(Untrusted::new(u16::MAX).at_most(65_535), Ok(65_535)); - assert_eq!(Untrusted::new(u32::MAX).at_most(u32::MAX as u64), Ok(u32::MAX as u64)); - assert_eq!(Untrusted::new(u64::MAX).at_most(u64::MAX), Ok(u64::MAX)); - assert_eq!( - Untrusted::new(u64::MAX).index(16), - Err(Refused::PastTable { value: u64::MAX, len: 16 }) - ); - } - - #[test] - fn exactly_refuses_by_naming_both_numbers() { - assert_eq!(Untrusted::new(12u32).exactly(12), Ok(12)); - assert_eq!( - Untrusted::new(13u32).exactly(12), - Err(Refused::NotExactly { value: 13, wanted: 12 }) - ); - } - - /// Deciding a `bool` reaches no memory, so equality against a constant the - /// caller wrote is not a way out — and it must keep working, because a - /// driver comparing a status word against a known code is the ordinary - /// thing to do with a value it does not trust. - #[test] - fn asking_whether_it_is_a_known_value_is_not_an_exit() { - assert!(Untrusted::new(0u32).is(0)); - assert!(!Untrusted::new(1u32).is(0)); - } - - /// The property the compile-fail cases above state negatively: a decode - /// produces another untrusted value, so the bound is still owed after it. - #[test] - fn decoding_keeps_the_provenance() { - let bar = Untrusted::new(0xFEBD_4004u32); - let low_bits = bar.map(|v| (v & 0xF) as u8); - assert!(low_bits.is(4)); - assert_eq!( - bar.map(|v| v >> 4).index(16), - Err(Refused::PastTable { value: 0x0FEB_D400, len: 16 }) - ); - } - - /// Two `Untrusted`s compare, because neither side of that comparison is a - /// claim about memory. This is what lets a driver notice that the device - /// said the same thing twice. - #[test] - fn two_untrusted_values_compare_with_each_other() { - assert_eq!(Untrusted::new(7u32), Untrusted::new(7u32)); - assert_ne!(Untrusted::new(7u32), Untrusted::new(8u32)); - } - - #[test] - fn a_refusal_says_which_number_and_which_bound() { - extern crate std; - use std::string::ToString; - assert_eq!( - Refused::PastTable { value: 0x1_0003, len: 16 }.to_string(), - "65539 indexes no entry of 16" - ); - assert_eq!( - Refused::PastBound { value: 4097, bound: 4096 }.to_string(), - "4097 is past the bound of 4096" - ); - assert_eq!( - Refused::NotExactly { value: 13, wanted: 12 }.to_string(), - "13 where 12 was required" - ); - } - - /// `#[repr(transparent)]` and nothing else in the struct: the type is the - /// integer, so adopting it at a site in a hot path costs that site nothing. - #[test] - fn it_is_the_integer_it_wraps() { - use core::mem::{align_of, size_of}; - assert_eq!(size_of::>(), size_of::()); - assert_eq!(align_of::>(), align_of::()); - assert_eq!(size_of::>(), size_of::()); - } -} diff --git a/lib/vdagent/Cargo.toml b/lib/vdagent/Cargo.toml index 0e3dac86b81..17eb27823bc 100644 --- a/lib/vdagent/Cargo.toml +++ b/lib/vdagent/Cargo.toml @@ -11,9 +11,5 @@ edition = "2021" license = "MIT OR Apache-2.0" publish = false -[dependencies] -# The type a host word stays in until it has met its bound. -toyos-untrusted = { path = "../untrusted" } - [lib] doctest = false diff --git a/lib/vdagent/src/control.rs b/lib/vdagent/src/control.rs index fd411e93f19..8deca61deef 100644 --- a/lib/vdagent/src/control.rs +++ b/lib/vdagent/src/control.rs @@ -6,8 +6,6 @@ //! "without a NUL terminator", and QEMU 11.1.1 ends it with one; the port is //! the declared [`crate::PORT_ID`] instead. -use toyos_untrusted::Untrusted; - /// One control message's bytes. pub const BYTES: usize = 8; @@ -83,13 +81,10 @@ pub fn decode(bytes: &[u8], max_nr_ports: u32) -> Result { let id = u32::from_le_bytes([message[0], message[1], message[2], message[3]]); let event = u16::from_le_bytes([message[4], message[5]]); let value = u16::from_le_bytes([message[6], message[7]]); - let event = Untrusted::new(event) - .index(Event::ALL.len()) - .map(|at| Event::ALL[at]) - .map_err(|_| Refused::Event { event })?; - Untrusted::new(id) - .index(max_nr_ports as usize) - .map_err(|_| Refused::Port { id, max_nr_ports })?; + let event = *Event::ALL.get(usize::from(event)).ok_or(Refused::Event { event })?; + if id >= max_nr_ports { + return Err(Refused::Port { id, max_nr_ports }); + } Ok(Control { id, event, value }) } diff --git a/lib/vdagent/src/stream.rs b/lib/vdagent/src/stream.rs index ed1e22b3d08..0f68b369312 100644 --- a/lib/vdagent/src/stream.rs +++ b/lib/vdagent/src/stream.rs @@ -18,8 +18,6 @@ use alloc::vec::Vec; -use toyos_untrusted::Untrusted; - use crate::wire; /// `VD_AGENT_MAX_DATA_SIZE`: the most one chunk carries. @@ -151,9 +149,7 @@ impl Decoder { self.chunk_filled = 0; let port = word(&self.chunk, 0); let size = word(&self.chunk, 4); - self.chunk_left = Untrusted::new(size) - .at_most(MAX_CHUNK as u64) - .map_err(|_| Unframed { size })? as usize; + self.chunk_left = usize::try_from(size).ok().filter(|&n| n <= MAX_CHUNK).ok_or(Unframed { size })?; self.ours = port == wire::CLIENT_PORT; if !self.ours { each(Err(Refusal::OtherPort { port })); @@ -242,10 +238,9 @@ fn admit(protocol: u32, kind: u32, size: u32) -> Result { if protocol != wire::PROTOCOL { return Err(Refusal::Protocol { protocol }); } - if Untrusted::new(size).at_most(MAX_DATA as u64).is_err() { + let Some(len) = usize::try_from(size).ok().filter(|&n| n <= MAX_DATA) else { return Err(Refusal::TooLarge { kind, size }); - } - let len = size as usize; + }; let (admitted, fits) = match kind { wire::ANNOUNCE_CAPABILITIES => (Kind::Capabilities, len >= 8), wire::GRAB => (Kind::Grab, len >= 4 && len.is_multiple_of(4) && (len - 4) / 4 <= MAX_GRAB_TYPES), diff --git a/lib/virtio/Cargo.toml b/lib/virtio/Cargo.toml index 805797f1afd..3a114c31650 100644 --- a/lib/virtio/Cargo.toml +++ b/lib/virtio/Cargo.toml @@ -20,9 +20,6 @@ license = "MIT OR Apache-2.0" publish = false [dependencies] -# The type a used-ring element's two fields stay in until each has met its -# bound. -toyos-untrusted = { path = "../untrusted" } # `Registers` and `DmaBuffers`: the boundary every userland driver is written # against, and the order of its two barriers. toyos-device-memory = { path = "../device-memory" } diff --git a/lib/virtio/src/queue.rs b/lib/virtio/src/queue.rs index 87aae8a855d..4504996fc73 100644 --- a/lib/virtio/src/queue.rs +++ b/lib/virtio/src/queue.rs @@ -21,8 +21,6 @@ use alloc::vec; use alloc::vec::Vec; -use toyos_untrusted::{Refused, Untrusted}; - use toyos_device_memory::DmaBuffers; /// §2.7: "The maximum Queue Size value is 32768." @@ -118,14 +116,14 @@ pub struct Used { pub enum UsedRefusal { /// An element's `id` is past the descriptor table. The element is taken /// and names no chain. - Head(Refused), + Head { id: u32, size: u16 }, /// An element's `id` is a head no chain is in flight at: one the device /// already gave back, or one never published. The element is taken. NoChain { head: u16 }, /// An element's `len` is more than its chain's device-writable elements /// hold (§2.7.8). The element is taken and its chain stays in flight: its /// buffers are the device's still, and are not handed up. - Written { head: u16, len: Refused }, + Written { head: u16, len: u32, writable: u32 }, /// The used index stands further on than the chains made available: the /// device says it used buffers it was never given, so no element from the /// last one taken on is one this driver can tell from a stale one. @@ -137,14 +135,16 @@ pub enum UsedRefusal { impl core::fmt::Display for UsedRefusal { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - Self::Head(why) => write!(f, "a used element's head {why}"), + Self::Head { id, size } => { + write!(f, "a used element's head {id} is past the queue's {size} descriptors") + } Self::NoChain { head } => { write!(f, "a used element names head {head}, where no chain is in flight") } - Self::Written { head, len } => write!( + Self::Written { head, len, writable } => write!( f, "the used element for head {head} claims more bytes than its chain may be \ - written: {len}" + written: {len} where it may take {writable}" ), Self::Jumped { used, taken, available } => write!( f, @@ -345,24 +345,25 @@ impl Virtqueue { let at = self.parts.used + RING_ENTRIES + (self.last_used % self.size) as usize * USED_ELEM_BYTES; - let id = Untrusted::new(self.mem.read32(at)); - let len = Untrusted::new(self.mem.read32(at + USED_ELEM_LEN)); + let id = self.mem.read32(at); + let len = self.mem.read32(at + USED_ELEM_LEN); self.last_used = self.last_used.wrapping_add(1); - let first = id.index(self.slots.len()).map_err(UsedRefusal::Head)?; - // Exact: `index` proved it below the table's length, a `u16`. + let Some(first) = usize::try_from(id).ok().filter(|&first| first < self.slots.len()) else { + return Err(UsedRefusal::Head { id, size: self.size }); + }; + // Exact: below the table's length, a `u16`. let head = first as u16; let chain = self.slots[first]; if chain.descs == 0 { return Err(UsedRefusal::NoChain { head }); } - let written = len - .at_most(chain.writable as u64) - .map_err(|len| UsedRefusal::Written { head, len })?; + if len > chain.writable { + return Err(UsedRefusal::Written { head, len, writable: chain.writable }); + } for slot in &mut self.slots[first..first + chain.descs as usize] { *slot = Slot::default(); } - // Exact: `at_most` proved it no more than `chain.writable`, a `u32`. - Ok(Some(Used { head, written: written as u32 })) + Ok(Some(Used { head, written: len })) } } diff --git a/lib/virtio/src/tests.rs b/lib/virtio/src/tests.rs index b437035d020..aa4fa463116 100644 --- a/lib/virtio/src/tests.rs +++ b/lib/virtio/src/tests.rs @@ -4,7 +4,6 @@ use std::collections::BTreeMap; use std::vec::Vec; -use toyos_untrusted::Refused; use crate::pci::{ status, Layout, Live, Offer, Refusal, Setup, Source, VendorCap, WalkRefusal, @@ -731,7 +730,7 @@ fn a_head_past_the_table_is_refused() { ring.use_chain(id, 0); assert_eq!( queue.poll_used(), - Err(UsedRefusal::Head(Refused::PastTable { value: id as u64, len: SIZE as u64 })) + Err(UsedRefusal::Head { id, size: SIZE }) ); } assert_eq!(queue.poll_used(), Ok(None)); @@ -783,10 +782,7 @@ fn more_bytes_than_the_chain_may_be_written_is_refused() { ring.use_chain(0, len); assert_eq!( queue.poll_used(), - Err(UsedRefusal::Written { - head: 0, - len: Refused::PastBound { value: len as u64, bound: 64 } - }) + Err(UsedRefusal::Written { head: 0, len, writable: 64 }) ); ring.use_chain(0, 64); assert_eq!(queue.poll_used(), Ok(Some(Used { head: 0, written: 64 }))); @@ -795,7 +791,7 @@ fn more_bytes_than_the_chain_may_be_written_is_refused() { ring.use_chain(2, 1); assert_eq!( queue.poll_used(), - Err(UsedRefusal::Written { head: 2, len: Refused::PastBound { value: 1, bound: 0 } }) + Err(UsedRefusal::Written { head: 2, len: 1, writable: 0 }) ); ring.use_chain(2, 0); assert_eq!(queue.poll_used(), Ok(Some(Used { head: 2, written: 0 }))); @@ -851,7 +847,7 @@ fn a_refused_element_does_not_hide_the_ones_behind_it() { ring.use_chain(0x55, 0); ring.use_chain(1, 64); ring.use_chain(2, 3); - assert!(matches!(queue.poll_used(), Err(UsedRefusal::Head(_)))); + assert!(matches!(queue.poll_used(), Err(UsedRefusal::Head { .. }))); assert_eq!(queue.poll_used(), Ok(Some(Used { head: 1, written: 64 }))); assert_eq!(queue.poll_used(), Ok(Some(Used { head: 2, written: 3 }))); assert_eq!(queue.poll_used(), Ok(None)); @@ -1005,15 +1001,9 @@ fn a_hostile_device_is_believed_only_where_the_model_agrees() { taken = taken.wrapping_add(1); let chain = u16::try_from(id).ok().and_then(|head| flying.get(&head).map(|c| (head, *c))); let want = match chain { - _ if id >= SIZE as u32 => Err(UsedRefusal::Head(Refused::PastTable { - value: id as u64, - len: SIZE as u64, - })), + _ if id >= SIZE as u32 => Err(UsedRefusal::Head { id, size: SIZE }), None => Err(UsedRefusal::NoChain { head: id as u16 }), - Some((head, (_, writable))) if len > writable => Err(UsedRefusal::Written { - head, - len: Refused::PastBound { value: len as u64, bound: writable as u64 }, - }), + Some((head, (_, writable))) if len > writable => Err(UsedRefusal::Written { head, len, writable }), Some((head, _)) => { flying.remove(&head); believed += 1; @@ -1021,7 +1011,7 @@ fn a_hostile_device_is_believed_only_where_the_model_agrees() { } }; match want { - Err(UsedRefusal::Head(_)) => refused[0] += 1, + Err(UsedRefusal::Head { .. }) => refused[0] += 1, Err(UsedRefusal::NoChain { .. }) => refused[1] += 1, Err(UsedRefusal::Written { .. }) => refused[2] += 1, Err(UsedRefusal::Jumped { .. }) | Ok(_) => {} diff --git a/rust b/rust index 73f63e2441b..e1c5e93d342 160000 --- a/rust +++ b/rust @@ -1 +1 @@ -Subproject commit 73f63e2441b93b979229144b58033954ffb1d40d +Subproject commit e1c5e93d34236ee3e57d510920f94c0256c647eb diff --git a/sdk/libc/Cargo.lock b/sdk/libc/Cargo.lock index 2d766086466..f3b061534dd 100644 --- a/sdk/libc/Cargo.lock +++ b/sdk/libc/Cargo.lock @@ -59,13 +59,6 @@ version = "0.1.0" [[package]] name = "toyos-transport" version = "0.1.0" -dependencies = [ - "toyos-untrusted", -] - -[[package]] -name = "toyos-untrusted" -version = "0.1.0" [[package]] name = "windows-link" diff --git a/sdk/transport/Cargo.toml b/sdk/transport/Cargo.toml index 3665df21744..8bfca088cb6 100644 --- a/sdk/transport/Cargo.toml +++ b/sdk/transport/Cargo.toml @@ -17,7 +17,7 @@ repository = "https://github.com/ToyOSOrg/ToyOS" [features] # std links it through `toyos`, whose stream end is a session's client. -rustc-dep-of-std = ["core", "toyos-untrusted/rustc-dep-of-std"] +rustc-dep-of-std = ["core"] # Declared, never enabled by any build that ships: each reverts one decision so # the model that refuses it is shown able to red at all. `build/ci.rs`'s # `CONTROLS` runs each and demands the named test's own FAILED line. @@ -61,7 +61,6 @@ head-only = [] [dependencies] core = { version = "1.0.0", optional = true, package = "rustc-std-workspace-core" } -toyos-untrusted = { path = "../../lib/untrusted" } [dev-dependencies] loom = "0.7" diff --git a/sdk/transport/src/arena.rs b/sdk/transport/src/arena.rs index d85fbe9fe9c..63686e3247a 100644 --- a/sdk/transport/src/arena.rs +++ b/sdk/transport/src/arena.rs @@ -6,7 +6,7 @@ use core::fmt; -use crate::{Untrusted, Violation}; +use crate::Violation; /// Bytes `offset..offset + len` of the region. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -69,12 +69,8 @@ impl fmt::Debug for Run { impl Run { /// The run a peer's two words name. - pub fn decode(first: Untrusted, count: Untrusted, geometry: &Geometry) -> Result { - let bounded = |word: Untrusted| word.at_most(u64::from(geometry.slots)).ok()?.try_into().ok(); - bounded(first) - .zip(bounded(count)) - .and_then(|(first, count)| geometry.run(first, count)) - .ok_or(Violation::Run) + pub fn decode(first: u32, count: u32, geometry: &Geometry) -> Result { + geometry.run(first, count).ok_or(Violation::Run) } pub fn first(&self) -> u32 { @@ -108,11 +104,11 @@ mod tests { /// last slot is a run and one past it is not, however the sum wraps. #[test] fn a_run_a_peer_names_is_inside_the_arena() { - let run = Run::decode(Untrusted::new(510), Untrusted::new(1), &PAGES).unwrap(); + let run = Run::decode(510, 1, &PAGES).unwrap(); assert_eq!(run.span(), Span { offset: 0x1000 + 510 * 4096, len: 4096 }); for (first, count) in [(510, 2), (0, 0), (511, 1), (1, u32::MAX), (u32::MAX, 1), (0, 512)] { assert_eq!( - Run::decode(Untrusted::new(first), Untrusted::new(count), &PAGES), + Run::decode(first, count, &PAGES), Err(Violation::Run), "{first}+{count}" ); diff --git a/sdk/transport/src/inflight.rs b/sdk/transport/src/inflight.rs index 9a45176bdb9..b692dce6097 100644 --- a/sdk/transport/src/inflight.rs +++ b/sdk/transport/src/inflight.rs @@ -12,7 +12,7 @@ use core::fmt; -use crate::{Untrusted, Violation}; +use crate::Violation; #[derive(Clone, PartialEq, Eq, Hash)] struct Slot { @@ -53,10 +53,10 @@ impl Inflight { /// What the peer's `tag` answers; a tag nothing is in flight under is a /// violation. - pub fn answer(&mut self, tag: Untrusted) -> Result { - let index = tag.map(|t| t & Self::INDEX_MASK).index(D).map_err(|_| Violation::Tag)?; + pub fn answer(&mut self, tag: u32) -> Result { + let index = usize::try_from(tag & Self::INDEX_MASK).map_err(|_| Violation::Tag)?; let slot = self.slots.get_mut(index).ok_or(Violation::Tag)?; - if !tag.map(|t| t.wrapping_shr(Self::INDEX_BITS)).is(slot.seq) { + if tag.wrapping_shr(Self::INDEX_BITS) != slot.seq { return Err(Violation::Tag); } slot.value.take().ok_or(Violation::Tag) @@ -110,18 +110,18 @@ mod tests { let a = inflight.insert("a").unwrap(); let b = inflight.insert("b").unwrap(); assert_eq!(inflight.insert("c"), Err("c")); - assert_eq!(inflight.answer(Untrusted::new(a)), Ok("a")); - assert_eq!(inflight.answer(Untrusted::new(a)), Err(Violation::Tag), "answered twice"); + assert_eq!(inflight.answer(a), Ok("a")); + assert_eq!(inflight.answer(a), Err(Violation::Tag), "answered twice"); let c = inflight.insert("c").unwrap(); assert_ne!(c, a, "the slot's next tag is not its last"); let mut held: Vec<&str> = inflight.values().copied().collect(); held.sort_unstable(); assert_eq!(held, ["b", "c"], "what is in flight, and only that"); assert_eq!(inflight.tags().collect::>(), [c, b], "the tags in flight, by slot"); - assert_eq!(inflight.answer(Untrusted::new(a)), Err(Violation::Tag), "a replay answers nothing"); - assert_eq!(inflight.answer(Untrusted::new(2)), Err(Violation::Tag), "an index past the table"); - assert_eq!(inflight.answer(Untrusted::new(b)), Ok("b")); - assert_eq!(inflight.answer(Untrusted::new(c)), Ok("c")); + assert_eq!(inflight.answer(a), Err(Violation::Tag), "a replay answers nothing"); + assert_eq!(inflight.answer(2), Err(Violation::Tag), "an index past the table"); + assert_eq!(inflight.answer(b), Ok("b")); + assert_eq!(inflight.answer(c), Ok("c")); assert!(inflight.insert("d").is_ok() && inflight.insert("e").is_ok(), "every slot is free again"); } @@ -131,25 +131,25 @@ mod tests { fn a_slot_refilled_past_sixteen_bits_refuses_its_first_tag() { let mut inflight = Inflight::<(), 64>::new(); let first = inflight.insert(()).unwrap(); - inflight.answer(Untrusted::new(first)).unwrap(); + inflight.answer(first).unwrap(); for _ in 0..u16::MAX { let tag = inflight.insert(()).unwrap(); - inflight.answer(Untrusted::new(tag)).unwrap(); + inflight.answer(tag).unwrap(); } inflight.insert(()).unwrap(); - assert_eq!(inflight.answer(Untrusted::new(first)), Err(Violation::Tag)); + assert_eq!(inflight.answer(first), Err(Violation::Tag)); } #[test] fn an_end_answers_every_tag_once_and_a_late_completion_nothing() { let mut inflight = Inflight::::new(); let tags: Vec = (0..3).map(|n| inflight.insert(n).unwrap()).collect(); - assert_eq!(inflight.answer(Untrusted::new(tags[1])), Ok(1)); + assert_eq!(inflight.answer(tags[1]), Ok(1)); let mut ended = Vec::new(); inflight.end(|tag, value| ended.push((tag, value))); assert_eq!(ended, [(tags[0], 0), (tags[2], 2)]); for tag in tags { - assert_eq!(inflight.answer(Untrusted::new(tag)), Err(Violation::Tag)); + assert_eq!(inflight.answer(tag), Err(Violation::Tag)); } let mut again = 0; inflight.end(|_, _| again += 1); diff --git a/sdk/transport/src/lib.rs b/sdk/transport/src/lib.rs index eb3f6779c43..7ba61e6677e 100644 --- a/sdk/transport/src/lib.rs +++ b/sdk/transport/src/lib.rs @@ -12,7 +12,7 @@ //! [`Span`], once: no reference to the peer's bytes is formed. //! //! **What the peer writes is untrusted until decoded.** An entry comes out as -//! [`Untrusted`] words; a peer's cursor is bounded against the ring before it +//! the peer's words, for the protocol's decoder to bound; a peer's cursor is bounded against the ring before it //! is believed, and the variant of [`Violation`] names what it claimed; a run //! is bounded against the [`Geometry`], and a tag against the [`Inflight`] //! table. An end keeps its own cursor locally and only stores the shared one, @@ -38,7 +38,6 @@ use core::sync::atomic::{AtomicU32, Ordering}; pub use arena::{Geometry, Run, Span}; pub use inflight::Inflight; pub use queue::{Consumer, Place, Producer}; -pub use toyos_untrusted::Untrusted; /// One shared 32-bit word of a region: an atomic over the mapping, or a /// model's. @@ -75,9 +74,9 @@ fn word(page: &[W; N], at: usize) -> &W { } /// A distance the peer's cursor claims, believed only up to `cap`. -fn clamp(claimed: Untrusted, cap: u32, broken: Violation) -> Result { +fn clamp(claimed: u32, cap: u32, broken: Violation) -> Result { let cap = if cfg!(feature = "no-clamp") { u32::MAX } else { cap }; - claimed.at_most(u64::from(cap)).ok().and_then(|n| u32::try_from(n).ok()).ok_or(broken) + if claimed > cap { Err(broken) } else { Ok(claimed) } } /// The peer wrote what no peer of the protocol writes. The session is over; diff --git a/sdk/transport/src/queue.rs b/sdk/transport/src/queue.rs index 865c66ac991..a9b3afed68a 100644 --- a/sdk/transport/src/queue.rs +++ b/sdk/transport/src/queue.rs @@ -8,7 +8,7 @@ use core::sync::atomic::Ordering; -use crate::{clamp, word, Untrusted, Violation, Word, PUBLISH}; +use crate::{clamp, word, Violation, Word, PUBLISH}; /// Where a queue of `E`-word entries, `D` deep, is among `N` words: the word /// its consumer stores its head in, the word its producer stores its tail in, @@ -66,13 +66,13 @@ impl Place { /// How far past `released` the producer's tail is: at most `D`. fn published(&self, page: &[W; N], released: u32) -> Result { let tail = word(page, self.tail).load(Ordering::Acquire); - clamp(Untrusted::new(tail).map(|t| t.wrapping_sub(released)), D, Violation::TailPastDepth) + clamp(tail.wrapping_sub(released), D, Violation::TailPastDepth) } /// How far behind `published` the consumer's head is: at most `D`. fn unreleased(&self, page: &[W; N], published: u32) -> Result { let head = word(page, self.head).load(Ordering::Acquire); - clamp(Untrusted::new(head).map(|h| published.wrapping_sub(h)), D, Violation::HeadPastTail) + clamp(published.wrapping_sub(head), D, Violation::HeadPastTail) } } @@ -145,8 +145,9 @@ impl Consumer { Self { place, local: 0, released: 0, ready: 0 } } - /// The next published entry, or `None` for none. - pub fn pop(&mut self, page: &[W; N]) -> Result; E]>, Violation> { + /// The next published entry, or `None` for none. Its words are the peer's, + /// for the protocol's decoder to bound before any is believed. + pub fn pop(&mut self, page: &[W; N]) -> Result, Violation> { if self.ready == 0 { let published = self.place.published(page, self.released)?; self.ready = published.saturating_sub(self.local.wrapping_sub(self.released)); @@ -154,9 +155,9 @@ impl Consumer { return Ok(None); } } - let mut words = [Untrusted::new(0); E]; + let mut words = [0; E]; for (out, shared) in words.iter_mut().zip(self.place.entry(page, self.local)) { - *out = Untrusted::new(shared.load(Ordering::Relaxed)); + *out = shared.load(Ordering::Relaxed); } self.local = self.local.wrapping_add(1); self.ready = self.ready.wrapping_sub(1); @@ -189,20 +190,16 @@ mod tests { (Producer::new(page, PLACE), Consumer::new(page, PLACE)) } - fn plain(words: Option<[Untrusted; 2]>) -> Option<[u32; 2]> { - words.map(|w| w.map(|w| w.at_most(u32::MAX.into()).unwrap() as u32)) - } - #[test] fn an_entry_is_nobodys_before_it_is_published() { let page = page(); let (mut tx, mut rx) = ends(&page); assert_eq!(tx.push(&page, [3, 4]), Ok(true)); - assert_eq!(rx.pop(&page).map(plain), Ok(None)); + assert_eq!(rx.pop(&page), Ok(None)); assert!(tx.publish(&page)); assert!(!tx.publish(&page), "nothing new, nothing published"); - assert_eq!(rx.pop(&page).map(plain), Ok(Some([3, 4]))); - assert_eq!(rx.pop(&page).map(plain), Ok(None)); + assert_eq!(rx.pop(&page), Ok(Some([3, 4]))); + assert_eq!(rx.pop(&page), Ok(None)); } /// The ring wraps many times over, and space is exactly what the consumer @@ -223,7 +220,7 @@ mod tests { } tx.publish(&page); for _ in 0..1 + round % 7 { - let Some(words) = plain(rx.pop(&page).unwrap()) else { break }; + let Some(words) = rx.pop(&page).unwrap() else { break }; assert_eq!(words, [popped, !popped], "entries come out whole, in the order they went in"); popped += 1; } diff --git a/sdk/transport/src/stream.rs b/sdk/transport/src/stream.rs index 6ea71516689..193a2b09ac0 100644 --- a/sdk/transport/src/stream.rs +++ b/sdk/transport/src/stream.rs @@ -29,7 +29,7 @@ use core::sync::atomic::Ordering; -use crate::{clamp, word, Span, Untrusted, Violation, Word, PUBLISH}; +use crate::{clamp, word, Span, Violation, Word, PUBLISH}; /// A sleeper's exchange: what the waker published before clearing the want is /// seen by the look that follows. @@ -113,8 +113,8 @@ impl Place { /// How far the peer's `claimed` position is from this end's, by `apart`: /// at most the ring. A word past the lap names no position, so it is /// farther than any ring. - fn reach(claimed: Untrusted, apart: impl FnOnce(u32) -> u32, broken: Violation) -> Result { - clamp(claimed.map(|at| if at < Self::LAP { apart(at) } else { u32::MAX }), C, broken) + fn reach(claimed: u32, apart: impl FnOnce(u32) -> u32, broken: Violation) -> Result { + clamp(if claimed < Self::LAP { apart(claimed) } else { u32::MAX }, C, broken) } /// The region's bytes `len` on from position `at`, in ring order: the @@ -148,7 +148,7 @@ impl Producer { /// How many bytes past the tail may be written, the consumer's head read /// again. pub fn room(&mut self, page: &[W; N]) -> Result { - let head = Untrusted::new(word(page, self.place.head).load(TAKE_BACK)); + let head = word(page, self.place.head).load(TAKE_BACK); let published = self.published; let unreleased = Place::::reach(head, |head| Place::::distance(head, published), Violation::HeadPastTail)?; @@ -198,7 +198,7 @@ impl Producer { /// `ended`. With neither, nothing is exchanged and the want waits for the /// next publish. pub fn ring_back(&self, page: &[W; N], ended: bool) -> Result { - let head = Untrusted::new(word(page, self.place.head).load(TAKE_BACK)); + let head = word(page, self.place.head).load(TAKE_BACK); let published = self.published; let held = Place::::reach(head, |head| Place::::distance(head, published), Violation::HeadPastTail)?; @@ -227,7 +227,7 @@ impl Consumer { /// How many bytes past the head are published, the producer's tail read /// again. pub fn ready(&mut self, page: &[W; N]) -> Result { - let tail = Untrusted::new(word(page, self.place.tail).load(Ordering::Acquire)); + let tail = word(page, self.place.tail).load(Ordering::Acquire); let released = self.released; let published = Place::::reach(tail, |tail| Place::::distance(released, tail), Violation::TailPastDepth)?; @@ -276,7 +276,7 @@ impl Consumer { /// or `fatal`. With neither, nothing is exchanged and the want waits for /// the next release. pub fn ring_back(&self, page: &[W; N], fatal: bool) -> Result { - let tail = Untrusted::new(word(page, self.place.tail).load(Ordering::Acquire)); + let tail = word(page, self.place.tail).load(Ordering::Acquire); let released = self.released; let unreleased = Place::::reach(tail, |tail| Place::::distance(released, tail), Violation::TailPastDepth)?; diff --git a/sdk/transport/tests/loom.rs b/sdk/transport/tests/loom.rs index 4fca91b59e7..b898f11f0ba 100644 --- a/sdk/transport/tests/loom.rs +++ b/sdk/transport/tests/loom.rs @@ -19,7 +19,7 @@ use std::rc::Rc; use loom::cell::Cell; use loom::sync::atomic::AtomicU32; use loom::sync::{Arc, Notify}; -use toyos_transport::{stream, Consumer, Place, Producer, Untrusted, Violation, Word}; +use toyos_transport::{stream, Consumer, Place, Producer, Violation, Word}; /// A loom atomic as a region word: the trait is this crate's and the type is /// loom's, so the two meet through a wrapper. @@ -54,10 +54,6 @@ fn entry(n: u32) -> [u32; E] { [100 + n, 7 * n + 3] } -fn plain(words: [Untrusted; E]) -> [u32; E] { - words.map(|w| w.at_most(u32::MAX.into()).unwrap() as u32) -} - /// Two entries published one at a time, read by the other end as they /// arrive: each is whole, in order, and exactly what was written. #[test] @@ -70,7 +66,7 @@ fn a_published_entry_is_read_whole() { let mut read = Vec::new(); while read.len() < 2 { match rx.pop(&consumer_page).expect("the producer keeps the protocol") { - Some(words) => read.push(plain(words)), + Some(words) => read.push(words), None => loom::thread::yield_now(), } } diff --git a/tests/guest/rust-tests/Cargo.lock b/tests/guest/rust-tests/Cargo.lock index 3602c06f71e..53fd760422a 100644 --- a/tests/guest/rust-tests/Cargo.lock +++ b/tests/guest/rust-tests/Cargo.lock @@ -997,20 +997,12 @@ dependencies = [ [[package]] name = "toyos-transport" version = "0.1.0" -dependencies = [ - "toyos-untrusted", -] - -[[package]] -name = "toyos-untrusted" -version = "0.1.0" [[package]] name = "toyos-virtio" version = "0.1.0" dependencies = [ "toyos-device-memory", - "toyos-untrusted", ] [[package]] diff --git a/tests/guest/rust-tests/src/bin/abuse_thread_name.rs b/tests/guest/rust-tests/src/bin/abuse_thread_name.rs index 3059d3ec74f..c8e98f4e358 100644 --- a/tests/guest/rust-tests/src/bin/abuse_thread_name.rs +++ b/tests/guest/rust-tests/src/bin/abuse_thread_name.rs @@ -1,8 +1,5 @@ -//! `SYS_SET_THREAD_NAME` used to clamp an oversized name with `(a2 as -//! usize).min(THREAD_NAME_LEN)` and set the truncated prefix — a silent -//! clamp. `Untrusted::at_most` replaced it with a refusal, which is a -//! behaviour change worth its own gate: this proves the refusal actually -//! fires, rather than the clamp it replaced. +//! `SYS_SET_THREAD_NAME` refuses a name longer than `THREAD_NAME_LEN` rather +//! than setting its truncated prefix: this proves the refusal fires. //! //! `toyos_abi::syscall::set_thread_name` throws its own return value away — //! it always has, and that is not this test's to fix — so the return code @@ -80,11 +77,8 @@ fn main() { ); println!(" at the bound: accepted, and reads back whole"); - // One byte over: before this conversion this call silently set the first - // THREAD_NAME_LEN bytes of `over` — indistinguishable, from calling code - // that does not check the (discarded) return value, from asking for - // `at_bound` again. `Untrusted::at_most` refuses it instead, and this is - // the only way to see that from userland: the name is still `at_bound`. + // One byte over is refused, and the only way to see that from userland is + // that the name is still `at_bound`, not a truncated `over`. let mut over = [b'b'; THREAD_NAME_LEN + 1]; over[THREAD_NAME_LEN] = b'!'; syscall::set_thread_name(&over);