diff --git a/issues/a-moved-handle-is-always-re-movable.md b/issues/a-moved-handle-is-always-re-movable.md index 7d775aafbce..4a24361ff79 100644 --- a/issues/a-moved-handle-is-always-re-movable.md +++ b/issues/a-moved-handle-is-always-re-movable.md @@ -6,7 +6,7 @@ opened: 2026-08-10 # A handle that can be moved can always be moved on -Both move paths — `SpawnArgs::endow` and `SYS_HANDLE_SEND` — require +Both move paths — `SpawnArgs::endow` and `SYS_MESSAGE_SEND` — require `Rights::TRANSFER` on the handle being moved, and both carry the source's rights into the destination unchanged. So every handle that arrives by a move arrives carrying `TRANSFER`, and its new holder can move it on. There is no way to @@ -33,7 +33,7 @@ recording. ## What would fix it A rights word per moved handle, on **both** paths: `EndowEntry` gains one and -`SYS_HANDLE_SEND` takes `[TransferEntry { handle, rights }]` instead of +`SYS_MESSAGE_SEND` takes `[TransferEntry { handle, rights }]` instead of `[RawHandle]`, with `RIGHTS_UNCHANGED` as the wire encoding `SYS_HANDLE_DUP` already uses. The endowment design argues against a rights word on the endow path — "a second place to shrink rights and the first one already @@ -51,8 +51,8 @@ trigger: *"with `grant` owner-only, the set that can ever map is exactly the set the owner named, so revocation has no caller today … It stops being sound the moment the reachable set is no longer exactly what the owner named — if delegation or re-grant is reintroduced, or when `SYS_HANDLE_SEND` makes a grant -transferable."* `SYS_HANDLE_SEND` exists, so the trigger has fired: the reachable -set is no longer what the owner named. +transferable."* Handles move by `SYS_MESSAGE_SEND`, so the trigger has fired: the +reachable set is no longer what the owner named. That leaves two answers and the rights word above is the cheaper one. **Revocation proper is rejected by name** in the capability-handle design — shm revoke, @@ -70,5 +70,5 @@ folded). The rights model cannot spell "you may map this and nobody else may", so the property the capability design claims is not the property the code has — soundd's per-client ring is re-sendable by its client. Owed by whoever next opens the handle ABI: a rights word on `EndowEntry` **and** on -`SYS_HANDLE_SEND`, on one ABI-only PR, since doing either alone makes the two +`SYS_MESSAGE_SEND`, on one ABI-only PR, since doing either alone makes the two move verbs disagree. diff --git a/issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md b/issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md new file mode 100644 index 00000000000..04bf46ac1cc --- /dev/null +++ b/issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md @@ -0,0 +1,46 @@ +--- +status: owner +kind: question +opened: 2026-10-21 +--- + +# A pipe write of a few bytes can tear between two writers + +`pipe::try_write` (`kernel/src/pipe.rs`) writes `min(len, free space)` and +`sys_write` (`kernel/src/syscall/io.rs`) returns that short count. It waits +only while the ring is full, and the caller's write loop sends the rest later. +So whenever the reader lags and the ring has less free space than a write, +even a 40-byte line is split. Another writer's bytes can land between the two +parts. POSIX promises a pipe write of at most `PIPE_BUF` bytes is never +interleaved with another's, and `sdk/libc/include/limits.h` declares no +`PIPE_BUF` at all. + +Every multi-writer pipe has this gap: `{ a & b; } | c`, `make -j | tee log`, +and a stdout stream several children share, whose lines tear in one log +while the file server behind it is busy. A one-byte write, such as a +jobserver token, cannot tear. Record boundaries matter to any +multi-writer pipe, so the fix is native, not POSIX's alone. + +Only the pipe can keep such a write whole: its writers are separate +processes holding one pipe and sharing no lock, so no userland library can +order them. + +The change is one branch in the pipe's write path, plus libc's constant, and +it costs two semantics. A write of at most `PIPE_BUF` waits for room for the +whole write (`pipe::has_space(id, len)`) where today it takes part of the +ring, and a non-blocking one (`sys_write_nonblock`, +`kernel/src/syscall/io.rs`) answers `WouldBlock` where today it answers a +short count, as POSIX has `O_NONBLOCK` do. Writes longer than `PIPE_BUF` +behave as today. + +Owner: the kernel's pipe. It is the owner's call because it changes the +kernel's write semantics (ruling M: no kernel change for POSIX, so the case +for it is the native one above). + +## Exit + +A pipe write of at most `PIPE_BUF` bytes lands whole or waits for room, a +non-blocking one lands whole or answers `WouldBlock`, libc declares +`PIPE_BUF`, and a host test of the pipe shows two writers' writes of at most +`PIPE_BUF` never interleave while a reader lags. Longer writes may still be +split. diff --git a/issues/a-region-keeps-the-address-space-of-every-process-that-ended-with-it-mapped.md b/issues/a-region-keeps-the-address-space-of-every-process-that-ended-with-it-mapped.md index 41b360d430a..0f90bbf3d3d 100644 --- a/issues/a-region-keeps-the-address-space-of-every-process-that-ended-with-it-mapped.md +++ b/issues/a-region-keeps-the-address-space-of-every-process-that-ended-with-it-mapped.md @@ -41,5 +41,5 @@ while the parent still holds the handle. whose exit ends a mapping at the close of its process's last handle and so reaches this one wherever the process closed that handle, before it ended or in its teardown. It does not reach a process that mapped the region and moved -its handle on by `SYS_HANDLE_SEND` (`sys_handle_send`, +its handle on by `SYS_MESSAGE_SEND` (`sys_message_send`, `kernel/src/syscall/ipc.rs`), which is no close. diff --git a/issues/a-stream-cannot-move-between-processes.md b/issues/a-stream-cannot-move-between-processes.md index ecce1a6e0a6..481690bb5e0 100644 --- a/issues/a-stream-cannot-move-between-processes.md +++ b/issues/a-stream-cannot-move-between-processes.md @@ -30,7 +30,7 @@ either side. Stage 3 makes std refuse both loudly instead. Every path std offers to move a raw handle into another process answers that refusal before anything moves: a spawn's stdio slots, `CommandExt::inherit_handle` into any slot, `CommandExt::endow` and -`Command::provide`, each starting no child. `toyos::ipc`'s `send_handles` +`Command::provide`, each starting no child. `toyos::ipc`'s `send_with` takes only `OwnedHandle`s, and a stream's handle becomes one only through `OwnedHandle::from_raw`, whose contract stage 3 says excludes a handle `toyos::net` answers for: the type refuses it. And `toyos::net` refuses by @@ -40,7 +40,7 @@ as an inetd-style server does through its child's stdio, then fails loudly on ToyOS where today it works, and one that takes a moved stream back as a `TcpStream`, as socket activation does and as no ToyOS std builds today, is refused, though root `CLAUDE.md` says existing Rust just works. The bare -`dup`, `SYS_HANDLE_SEND` and `SYS_SPAWN` syscalls are not checked. +`dup`, `SYS_MESSAGE_SEND` and `SYS_SPAWN` syscalls are not checked. Owner: the orchestrator, who stages `issues/netstack-moves-every-byte-through-two-kernel-pipes.md`. diff --git a/issues/four-shares-take-a-file-server.md b/issues/four-shares-take-a-file-server.md index 639c02eb933..c252d9b580e 100644 --- a/issues/four-shares-take-a-file-server.md +++ b/issues/four-shares-take-a-file-server.md @@ -29,6 +29,17 @@ never had one start past 3 served connections, 1 waiting on its hello or 0 streams, nor more than 5 of DATA's connections served at once. A parallel build (cargo with rustc as its direct children) is the unmeasured load. +Each writer streaming to a file at once holds one of its share's 16 streams, +so a `make -j16` whose jobs each stream to a log of their own holds all 16, +and separate stdout and stderr files per job need 32. Inferred, not +measured: the share's 32 served connections, spent by every process of the +session at one per directory, are probably the first bound such a build +reaches. Raising `STREAM_SHARE` doubles the uncharged rings a share can pin, +2 MiB each (`issues/nothing-charges-kernel-memory-to-a-process.md`), so it +waits for one of two exits: a measured parallel build on ToyOS shows which +bound it reaches and by how much, or kernel memory is charged and a share's +streams are bounded by bytes rather than by count. + Owner: the file server. ## Exit condition diff --git a/issues/std-says-a-launch-moves-its-handles-to-the-launcher-even-when-the-move-is-refused.md b/issues/std-says-a-launch-moves-its-handles-to-the-launcher-even-when-the-move-is-refused.md index 27fa18d1f98..0dda673637a 100644 --- a/issues/std-says-a-launch-moves-its-handles-to-the-launcher-even-when-the-move-is-refused.md +++ b/issues/std-says-a-launch-moves-its-handles-to-the-launcher-even-when-the-move-is-refused.md @@ -7,7 +7,7 @@ opened: 2026-10-05 # std says a launch moves its handles to the launcher, even when the move is refused `toyos::launch::launch` consumes every handle a `Launch` names: the kernel -moves them to the launcher, or refuses the move and `Connection::send_handles` +moves them to the launcher, or refuses the move and `Connection::send_with` closes them in the caller (`sdk/toyos/src/ipc.rs`, `sdk/toyos/src/launch.rs`). The prose of std's ToyOS backend about the same handles says something else: diff --git a/issues/the-capability-end-state-is-twelve-answers.md b/issues/the-capability-end-state-is-twelve-answers.md index d991ea2b13a..e1de26d3867 100644 --- a/issues/the-capability-end-state-is-twelve-answers.md +++ b/issues/the-capability-end-state-is-twelve-answers.md @@ -226,7 +226,7 @@ process has. is created with `Rights::NONE` there because "every bit on a `SysCap` is an authority init decides per program" — the machine's only one is minted with `TRANSFER` at `kernel/src/loader/mod.rs:946`. The mechanisms are -`SYS_HANDLE_SEND`/`SYS_HANDLE_RECV` over a `Connection`, where the connection +`SYS_MESSAGE_SEND`/`SYS_HANDLE_RECV` over a `Connection`, where the connection and every handle must carry `TRANSFER`, no handle may be named twice, and the connection may not be sent over itself (`4a98107f^:kernel/src/arch/syscall.rs:2086`, `:2090`); and `SpawnArgs::endow`. Nothing else crosses a process boundary — the diff --git a/kernel/pure/lib.rs b/kernel/pure/lib.rs index 41098a89349..83953977488 100644 --- a/kernel/pure/lib.rs +++ b/kernel/pure/lib.rs @@ -14,6 +14,7 @@ pub mod clock; pub mod cpu; pub mod dirnodes; pub mod iova; +pub mod message; pub mod microcode; pub mod mtrr; pub mod owners; diff --git a/kernel/pure/message.rs b/kernel/pure/message.rs new file mode 100644 index 00000000000..e02d3f7b32d --- /dev/null +++ b/kernel/pure/message.rs @@ -0,0 +1,264 @@ +//! Where each batch of handles a connection carries sits in its byte stream. +//! +//! A message is bytes and the handles they announce, sent together: the bytes +//! go into the stream whole, and the batch is pinned at the byte just past +//! them. The reader's reads never cross a pinned batch it has not taken, and +//! it takes a batch only where it is pinned. So a batch is never received +//! without its bytes, nor its bytes read past without the batch noticed. +//! +//! Positions count every byte the stream ever carried, never wrapping: a +//! `u64` of bytes outlives any connection. + +#![forbid(unsafe_code)] + +use alloc::collections::VecDeque; + +/// What a read at the reader's position may take now. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Window { + /// Up to this many bytes; zero is nothing to read yet. + Bytes(usize), + /// A batch is pinned exactly here, and the read must take it first. + AtBatch, +} + +/// Why a message is refused before its bytes are written. Nothing of it is +/// written or moved; room in the stream and the reader are the stream's to answer. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Refused { + /// Handles with no bytes announcing them. + Empty, + /// The reader holds as many batches as it may. + QueueFull, +} + +/// The batches a reader has not taken, each at the stream position it +/// follows, in the order they were sent. +pub struct Batches { + pinned: VecDeque<(u64, B)>, +} + +impl Default for Batches { + fn default() -> Self { + Self::new() + } +} + +impl Batches { + pub const fn new() -> Self { + Self { pinned: VecDeque::new() } + } + + pub fn len(&self) -> usize { + self.pinned.len() + } + + pub fn is_empty(&self) -> bool { + self.pinned.is_empty() + } + + /// Whether a message of `len` bytes, carrying a batch or not, may be + /// pinned here, `cap` batches at most. + pub fn admit(&self, len: usize, carries: bool, cap: usize) -> Result<(), Refused> { + if carries && len == 0 { + return Err(Refused::Empty); + } + if carries && self.pinned.len() >= cap { + return Err(Refused::QueueFull); + } + Ok(()) + } + + /// Pin `batch` at `at`, the position just past the bytes that announce it. + /// + /// # Panics + /// When `at` is before the last batch's position: positions only grow, so + /// that is a caller that wrote out of order. + pub fn pin(&mut self, at: u64, batch: B) { + if let Some((last, _)) = self.pinned.back() { + assert!(*last <= at, "message: a batch pinned at {at} after one at {last}"); + } + self.pinned.push_back((at, batch)); + } + + /// What a read at `read` may take of the `available` bytes the stream holds. + pub fn window(&self, read: u64, available: usize) -> Window { + match self.pinned.front() { + Some((at, _)) if *at == read => Window::AtBatch, + Some((at, _)) => { + let before = at.checked_sub(read).expect("message: the reader read past a pinned batch"); + Window::Bytes(available.min(usize::try_from(before).unwrap_or(usize::MAX))) + } + None => Window::Bytes(available), + } + } + + /// The batch pinned at `read`, if any, without taking it. + pub fn at(&self, read: u64) -> Option<&B> { + self.pinned.front().filter(|(at, _)| *at == read).map(|(_, b)| b) + } + + /// Take the batch pinned at `read`; none elsewhere. + pub fn take(&mut self, read: u64) -> Option { + match self.pinned.front() { + Some((at, _)) if *at == read => self.pinned.pop_front().map(|(_, b)| b), + _ => None, + } + } + + /// Every batch still pinned, for its end's release. + pub fn drain(&mut self) -> impl Iterator + '_ { + self.pinned.drain(..).map(|(_, b)| b) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use alloc::vec::Vec; + + /// A stream and its batches as the kernel holds them: the bytes are + /// numbered by their position, so a read says exactly which it took. + struct Stream { + batches: Batches, + bytes: VecDeque, + written: u64, + read: u64, + ring: usize, + } + + impl Stream { + fn new(ring: usize) -> Self { + Self { batches: Batches::new(), bytes: VecDeque::new(), written: 0, read: 0, ring } + } + + /// One send, whole or not at all, as the kernel makes it under the + /// queue lock and then the pipe lock; `None` is a stream without room. + fn send(&mut self, len: usize, batch: Option) -> Option> { + if let Err(refused) = self.batches.admit(len, batch.is_some(), 4) { + return Some(Err(refused)); + } + if len > self.ring - self.bytes.len() { + return None; + } + for _ in 0..len { + self.bytes.push_back(self.written); + self.written += 1; + } + if let Some(b) = batch { + self.batches.pin(self.written, b); + } + Some(Ok(())) + } + + fn read(&mut self, want: usize) -> Result, Window> { + match self.batches.window(self.read, self.bytes.len()) { + Window::AtBatch => Err(Window::AtBatch), + Window::Bytes(n) => { + let got: Vec = self.bytes.drain(..n.min(want)).collect(); + self.read += got.len() as u64; + Ok(got) + } + } + } + } + + #[test] + fn a_batch_is_taken_only_where_its_bytes_end_and_no_read_crosses_it() { + let mut s = Stream::new(64); + s.send(5, Some(7)).unwrap().unwrap(); + s.send(3, None).unwrap().unwrap(); + assert_eq!(s.batches.take(s.read), None, "a batch taken before its bytes were read"); + assert_eq!(s.read(100).unwrap(), [0, 1, 2, 3, 4], "a read crossed the batch"); + assert_eq!(s.read(100), Err(Window::AtBatch), "a read went on past an untaken batch"); + assert_eq!(s.batches.take(s.read), Some(7)); + assert_eq!(s.read(100).unwrap(), [5, 6, 7]); + } + + /// A refusal writes no byte and pins no batch. + #[test] + fn a_refused_send_leaves_nothing_behind() { + let mut s = Stream::new(8); + assert_eq!(s.send(0, Some(1)), Some(Err(Refused::Empty))); + assert_eq!((s.bytes.len(), s.batches.len()), (0, 0)); + for b in 0..4 { + s.send(1, Some(b)).unwrap().unwrap(); + } + assert_eq!(s.send(1, Some(9)), Some(Err(Refused::QueueFull))); + assert_eq!((s.bytes.len(), s.batches.len()), (4, 4)); + assert_eq!(s.send(1, None), Some(Ok(())), "a message with no handles waits on no queue"); + } + + /// xorshift64*: one seed is one run. + struct Rng(u64); + + impl Rng { + fn next(&mut self) -> u64 { + self.0 ^= self.0 >> 12; + self.0 ^= self.0 << 25; + self.0 ^= self.0 >> 27; + self.0.wrapping_mul(0x2545_F491_4F6C_DD1D) + } + + fn below(&mut self, n: u64) -> u64 { + self.next() % n + } + } + + /// Random sends and reads of random sizes against the order they were + /// sent: every batch reaches the reader exactly after the last byte of its + /// own message and before any byte of the next, and every byte arrives + /// once, in order. + #[test] + fn every_batch_arrives_between_its_own_bytes_and_the_next_messages() { + let mut rng = Rng(0x9E37_79B9_7F4A_7C15); + let mut s = Stream::new(256); + // What the reader must meet: per message, its bytes then its batch. + let mut sent: VecDeque<(u64, Option)> = VecDeque::new(); + let mut next_byte = 0u64; + let mut taken = 0; + for op in 0..20_000u32 { + if rng.below(2) == 0 { + let len = 1 + rng.below(40) as usize; + let batch = (rng.below(3) == 0).then_some(op); + if s.send(len, batch) == Some(Ok(())) { + sent.push_back((s.written, batch)); + } + } else { + match s.read(1 + rng.below(50) as usize) { + Ok(got) => { + for byte in got { + assert_eq!(byte, next_byte, "a byte out of order"); + next_byte += 1; + // A message ending before this byte with a batch must have been taken. + while sent.front().is_some_and(|(end, batch)| *end <= byte && batch.is_none()) { + sent.pop_front(); + } + assert!( + !sent.front().is_some_and(|(end, b)| *end <= byte && b.is_some()), + "byte {byte} was read past a batch pinned at {}", + sent.front().unwrap().0 + ); + } + } + Err(Window::AtBatch) => { + let (end, batch) = sent.pop_front().expect("a batch the model never sent"); + assert_eq!(end, s.read, "a batch met away from the end of its bytes"); + assert_eq!(s.batches.take(s.read), batch, "the batch taken is not its message's"); + taken += 1; + } + Err(Window::Bytes(_)) => unreachable!(), + } + } + } + assert!(taken > 1000, "the run took only {taken} batches"); + } + + #[test] + #[should_panic(expected = "pinned at 3 after one at 5")] + fn a_batch_pinned_before_the_last_is_a_caller_bug() { + let mut b = Batches::new(); + b.pin(5, 1); + b.pin(3, 2); + } +} diff --git a/kernel/pure/pipe.rs b/kernel/pure/pipe.rs index 76b2b39ac16..960d1d38669 100644 --- a/kernel/pure/pipe.rs +++ b/kernel/pure/pipe.rs @@ -83,9 +83,10 @@ impl Holders { available > 0 || !self.held(End::Write) } - /// A write answers now: it takes bytes, or is refused for want of a reader. - pub fn writable(&self, space: u32) -> bool { - space > 0 || !self.held(End::Read) + /// A write that needs `want` bytes at once, at least one, answers now: it + /// takes them, or is refused for want of a reader. + pub fn writable(&self, space: u32, want: u32) -> bool { + space >= want.max(1) || !self.held(End::Read) } /// The end opposite `end` has no holder, whatever the ring holds. @@ -127,7 +128,7 @@ mod tests { let mut pipe = made(); assert_eq!(pipe.release(End::Read), Released::EndGone); assert!(pipe.other_end_gone(End::Write), "the reader left with room for {space} byte(s)"); - assert!(pipe.writable(space)); + assert!(pipe.writable(space, 1)); assert!(!pipe.other_end_gone(End::Read), "a writer that is held was told gone"); } } @@ -138,8 +139,8 @@ mod tests { fn readable_and_writable_say_nothing_of_the_other_end() { let pipe = made(); assert!(pipe.readable(1) && !pipe.other_end_gone(End::Read)); - assert!(pipe.writable(1) && !pipe.other_end_gone(End::Write)); - assert!(!pipe.readable(0) && !pipe.writable(0)); + assert!(pipe.writable(1, 1) && !pipe.other_end_gone(End::Write)); + assert!(!pipe.readable(0) && !pipe.writable(0, 1)); } /// A handle duplicated, or moved to another process, is a second diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index 43b84b67a06..d2dc509d7d9 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -514,7 +514,12 @@ pub fn try_read(object: &KObjectRef, buf: &mut UserBytesMut) -> Option { match object { KObjectRef::File(f) => read_file(f, buf), KObjectRef::PipeRead(r) => pipe::try_read(r.id(), buf).map(|n| n as u64), - KObjectRef::Connection(c) => pipe::try_read(c.rx(), buf).map(|n| n as u64), + // A batch pinned where the reader is answers the read, so no byte + // after it is read until the batch is taken. + KObjectRef::Connection(c) => c.read(buf).map(|read| match read { + Ok(n) => n as u64, + Err(crate::pipe::AtBatch) => SyscallError::HandlesPending.to_u64(), + }), KObjectRef::Device(_) => unreachable!("a device claim is read by `read_device`"), KObjectRef::Console(_) => { let mut count = 0usize; @@ -807,7 +812,7 @@ pub fn ftruncate(object: &KObjectRef, size: u64) -> u64 { pub fn has_data(object: &KObjectRef) -> bool { match object { KObjectRef::PipeRead(r) => pipe::has_data(r.id()), - KObjectRef::Connection(c) => pipe::has_data(c.rx()), + KObjectRef::Connection(c) => c.has_data(), KObjectRef::Console(_) => serial::has_data(), KObjectRef::Acceptor(a) => a.has_pending(), KObjectRef::Process(p) => p.finished(), @@ -849,8 +854,8 @@ pub fn read_posts_are_readiness(object: &KObjectRef) -> bool { pub fn has_space(object: &KObjectRef) -> bool { match object { - KObjectRef::PipeWrite(w) => pipe::has_space(w.id()), - KObjectRef::Connection(c) => pipe::has_space(c.tx()), + KObjectRef::PipeWrite(w) => pipe::has_space(w.id(), 1), + KObjectRef::Connection(c) => pipe::has_space(c.tx(), 1), KObjectRef::File(_) | KObjectRef::Null(_) => true, KObjectRef::Console(_) => crate::log::console::has_room(), KObjectRef::PipeRead(_) | KObjectRef::Device(_) | KObjectRef::Acceptor(_) diff --git a/kernel/src/object/port.rs b/kernel/src/object/port.rs index add11bdc67c..6ea1a2ea453 100644 --- a/kernel/src/object/port.rs +++ b/kernel/src/object/port.rs @@ -175,7 +175,7 @@ impl ZeroHandles for Acceptor { core::mem::take(&mut queue.pending) }; // Nobody will ever hold this inbox's read end, so a queued - // `SYS_HANDLE_SEND` on it must say `Gone`, not queue. + // `SYS_MESSAGE_SEND` on it must say `Gone`, not queue. for connection in &queued { connection.inbox.close_now(); } diff --git a/kernel/src/object/service.rs b/kernel/src/object/service.rs index 6bb9317b3d0..a44a3779436 100644 --- a/kernel/src/object/service.rs +++ b/kernel/src/object/service.rs @@ -1,52 +1,95 @@ //! A connection: two pipes for the bytes, two handle queues for the handles. -use alloc::collections::VecDeque; use alloc::sync::Arc; use alloc::vec::Vec; +use kernel::message::{Batches, Refused, Window}; use toyos_abi::syscall::{SyscallError, MAX_QUEUED_BATCHES}; -use crate::pipe::{PipeId, PipeReader, PipeWriter}; +use crate::pipe::{self, AtBatch, PipeId, PipeReader, PipeWriter, WholeWrite}; +use crate::user_ptr::{UserBytes, UserBytesMut}; use crate::sync::Lock; use super::handle::HandleEntry; use super::port::{PendingConnection, Stamp}; use super::{Held, KObjectVariant, ObjectCore, ZeroHandles}; -/// Handles in flight in one direction of a connection; `None` once the reader is gone. -pub struct HandleQueue(Lock>>>); +/// Handles in flight in one direction of a connection, each batch pinned at +/// the stream position it follows (`kernel::message`); `None` once the reader +/// is gone. +/// +/// **Its lock is taken before the pipe's**, by the sender for the whole of a +/// message and by the reader for a read or a receive, so a reader that has +/// read a message's bytes finds its batch pinned. +pub struct HandleQueue(Lock>>>); + +/// Why a message was not sent; nothing of it was. +pub enum SendRefused { + /// No room for the whole message now: wait on the write watch. + NoRoom, + Error(SyscallError), +} impl HandleQueue { fn open() -> Arc { - Arc::new(Self(Lock::new(Some(VecDeque::new())))) + Arc::new(Self(Lock::new(Some(Batches::new())))) } - /// A refusal hands the batch back: dropped here, it would be capabilities destroyed silently. - fn push(&self, batch: Vec) -> Result<(), (Vec, SyscallError)> { + /// Write `bytes` to `tx` and pin `batch` just past them, whole or not at + /// all. A refusal hands the batch back: dropped here, it would be + /// capabilities destroyed silently. + fn send(&self, tx: PipeId, bytes: &UserBytes, batch: Vec) -> Result<(), (Vec, SendRefused)> { let mut guard = self.0.lock(); - let Some(queue) = guard.as_mut() else { return Err((batch, SyscallError::Gone)) }; - if queue.len() >= MAX_QUEUED_BATCHES { - return Err((batch, SyscallError::ResourceExhausted)); + let Some(batches) = guard.as_mut() else { return Err((batch, SendRefused::Error(SyscallError::Gone))) }; + let carries = !batch.is_empty(); + match batches.admit(bytes.len(), carries, MAX_QUEUED_BATCHES) { + Ok(()) => {} + Err(Refused::Empty) => return Err((batch, SendRefused::Error(SyscallError::InvalidArgument))), + Err(Refused::QueueFull) => return Err((batch, SendRefused::Error(SyscallError::ResourceExhausted))), + } + match pipe::write_whole(tx, bytes) { + WholeWrite::Wrote(end) => { + if carries { + batches.pin(end, batch); + } + Ok(()) + } + WholeWrite::NoRoom => Err((batch, SendRefused::NoRoom)), + WholeWrite::BrokenPipe => Err((batch, SendRefused::Error(SyscallError::Gone))), + WholeWrite::NoMemory => Err((batch, SendRefused::Error(SyscallError::ResourceExhausted))), } - queue.push_back(batch); - Ok(()) } - /// Refuses without taking when the batch is wider than `cap`, leaving it queued to retry. - fn pop_bounded(&self, cap: usize) -> Result>, SyscallError> { - // One acquisition covers the check and the take: the batch measured is the batch returned. + /// A read of `rx`, stopping at the next pinned batch. + fn read(&self, rx: PipeId, buf: &mut UserBytesMut) -> Option> { + let guard = self.0.lock(); + pipe::try_read_within(rx, buf, |read, available| match guard.as_ref() { + Some(batches) => batches.window(read, available), + None => Window::Bytes(available), + }) + } + + /// Take the batch pinned at `rx`'s read position, refusing without taking + /// when it is wider than `cap`, and leaving it pinned when `room` refuses. + fn take(&self, rx: PipeId, cap: usize, room: impl FnOnce(usize) -> bool) -> Result>, SyscallError> { let mut guard = self.0.lock(); - let Some(queue) = guard.as_mut() else { return Ok(None) }; - match queue.front() { - None => Ok(None), - Some(batch) if batch.len() > cap => Err(SyscallError::InvalidArgument), - Some(_) => Ok(queue.pop_front()), + let Some(batches) = guard.as_mut() else { return Ok(None) }; + let Some(read) = pipe::read_position(rx) else { return Ok(None) }; + let Some(width) = batches.at(read).map(Vec::len) else { return Ok(None) }; + if width > cap { + return Err(SyscallError::InvalidArgument); + } + if !room(width) { + return Err(SyscallError::ResourceExhausted); } + Ok(batches.take(read)) } - /// How wide the oldest batch is, without taking it; the front this reports is what `pop_bounded` takes. - fn front_width(&self) -> Option { - self.0.lock().as_ref()?.front().map(Vec::len) + /// Whether a batch is pinned where the reader is, which is something to receive. + fn pinned_here(&self, rx: PipeId) -> bool { + let guard = self.0.lock(); + let Some(batches) = guard.as_ref() else { return false }; + pipe::read_position(rx).is_some_and(|read| batches.at(read).is_some()) } /// Drops the batches outside the lock: releasing a handle can run another object's hook. @@ -118,21 +161,24 @@ impl ConnectionEnd { self.tx } - /// See [`HandleQueue::push`]: a refusal comes back with the batch. - pub fn send( - &self, - batch: Vec, - ) -> Result<(), (Vec, SyscallError)> { - self.outbox.push(batch) + /// See [`HandleQueue::send`]: a refusal comes back with the batch. + pub fn send(&self, bytes: &UserBytes, batch: Vec) -> Result<(), (Vec, SendRefused)> { + self.outbox.send(self.tx, bytes, batch) + } + + /// See [`HandleQueue::read`]. + pub fn read(&self, buf: &mut UserBytesMut) -> Option> { + self.inbox.read(self.rx, buf) } - pub fn recv_bounded(&self, cap: usize) -> Result>, SyscallError> { - self.inbox.pop_bounded(cap) + /// See [`HandleQueue::take`]. + pub fn recv(&self, cap: usize, room: impl FnOnce(usize) -> bool) -> Result>, SyscallError> { + self.inbox.take(self.rx, cap, room) } - /// See [`HandleQueue::front_width`]. - pub fn peek_width(&self) -> Option { - self.inbox.front_width() + /// Bytes to read, the peer's end, or a batch pinned where the reader is. + pub fn has_data(&self) -> bool { + pipe::has_data(self.rx) || self.inbox.pinned_here(self.rx) } } diff --git a/kernel/src/object/shm.rs b/kernel/src/object/shm.rs index b5b36ba1df5..4e3b1e47e03 100644 --- a/kernel/src/object/shm.rs +++ b/kernel/src/object/shm.rs @@ -1,6 +1,6 @@ //! A region of memory more than one process can see. //! -//! Holding a handle allows mapping it; giving one away is `SYS_HANDLE_SEND`. +//! Holding a handle allows mapping it; giving one away is `SYS_MESSAGE_SEND`. //! The handle count owns the mappings and the `Arc` count owns the pages: the //! last handle takes the list of mappings away for good and tears each one //! down, and the pages are freed when the last `Arc` goes, always later since diff --git a/kernel/src/pipe.rs b/kernel/src/pipe.rs index 396c50dcf5b..822b9ccf1df 100644 --- a/kernel/src/pipe.rs +++ b/kernel/src/pipe.rs @@ -1,5 +1,6 @@ use crate::mm::pmm; +use kernel::message::Window; use kernel::pipe::{End, Holders, Released}; use toyos_abi::ring::Ring; @@ -123,6 +124,10 @@ struct Pipe { left: [Arc; 2], /// Set when a write should hand the next reader transient RT priority — covers readers already runnable, which the wake-time boost misses. rt_boost_pending: bool, + /// Bytes ever written and ever read: the stream positions a connection's + /// batches are pinned at (`kernel::message`), which never wrap. + written: u64, + read: u64, } // SAFETY: `Backing::ring`'s raw pointer and its owning `PhysPage` move together in the same struct, and the `PIPES` lock is the only way to reach a `Pipe`, serializing all access to it. @@ -138,6 +143,8 @@ impl Pipe { write_watch: Arc::new(Watch::new()), left: [Arc::new(Watch::new()), Arc::new(Watch::new())], rt_boost_pending: false, + written: 0, + read: 0, } } @@ -212,21 +219,42 @@ pub fn map_page(pipe_id: PipeId) -> Option { } pub fn try_read(pipe_id: PipeId, buf: &mut UserBytesMut) -> Option { + try_read_within(pipe_id, buf, |_, available| Window::Bytes(available)) + .map(|read| read.expect("a read with no window stops nowhere")) +} + +/// A read stopped at a batch pinned where the reader is. +#[derive(Debug)] +pub struct AtBatch; + +/// [`try_read`], taking only what `window` allows of the bytes the ring +/// holds, given the reader's stream position: [`AtBatch`] where it says the +/// reader is at a pinned batch. +pub fn try_read_within( + pipe_id: PipeId, + buf: &mut UserBytesMut, + window: impl FnOnce(u64, usize) -> Window, +) -> Option> { let (result, boost) = with_pipes_mut(|pipes| { let Some(pipe) = pipes.get_mut(pipe_id) else { return (None, false) }; - if pipe.available() > 0 { - let len = buf.len(); + let limit = match window(pipe.read, pipe.available() as usize) { + Window::AtBatch => return (Some(Err(AtBatch)), false), + Window::Bytes(limit) => limit, + }; + if pipe.available() > 0 && limit > 0 { + let len = buf.len().min(limit); let n = pipe .backing .as_mut() .expect("available() > 0 implies a ring") .ring .read(len, |off, src| buf.write_run(off, &src)); + pipe.read += n as u64; let boost = pipe.rt_boost_pending; pipe.rt_boost_pending = false; - (Some(n), boost) - } else if !pipe.holders.held(End::Write) { - (Some(0), false) + (Some(Ok(n)), boost) + } else if !pipe.holders.held(End::Write) && pipe.available() == 0 { + (Some(Ok(0)), false) } else { (None, false) } @@ -238,6 +266,39 @@ pub fn try_read(pipe_id: PipeId, buf: &mut UserBytesMut) -> Option { result } +/// The stream position the reader is at: every byte read so far. +pub fn read_position(pipe_id: PipeId) -> Option { + with_pipes(|pipes| pipes.get(pipe_id).map(|p| p.read)) +} + +/// What [`write_whole`] did: all of it, or nothing. +pub enum WholeWrite { + /// Every byte, ending at this stream position. + Wrote(u64), + /// Not enough room for all of it now. + NoRoom, + BrokenPipe, + NoMemory, +} + +/// Write all of `buf` or none of it, under the one lock hold. +pub fn write_whole(pipe_id: PipeId, buf: &UserBytes) -> WholeWrite { + with_pipes_mut(|pipes| { + let Some(pipe) = pipes.get_mut(pipe_id) else { return WholeWrite::BrokenPipe }; + if !pipe.holders.held(End::Read) { + return WholeWrite::BrokenPipe; + } + let Some(backing) = pipe.back() else { return WholeWrite::NoMemory }; + if (backing.ring.space() as usize) < buf.len() { + return WholeWrite::NoRoom; + } + let n = backing.ring.write(buf.len(), |off, mut dst| buf.read_run(off, &mut dst)); + assert_eq!(n, buf.len(), "pipe: a ring with room for a write took less of it"); + pipe.written += n as u64; + WholeWrite::Wrote(pipe.written) + }) +} + pub enum PipeWrite { Wrote(usize), BrokenPipe, @@ -255,7 +316,9 @@ pub fn try_write(pipe_id: PipeId, buf: &UserBytes) -> Option { return Some(PipeWrite::NoMemory); }; if backing.ring.space() > 0 { - Some(PipeWrite::Wrote(backing.ring.write(buf.len(), |off, mut dst| buf.read_run(off, &mut dst)))) + let n = backing.ring.write(buf.len(), |off, mut dst| buf.read_run(off, &mut dst)); + pipe.written += n as u64; + Some(PipeWrite::Wrote(n)) } else { None } @@ -268,10 +331,10 @@ pub fn has_data(pipe_id: PipeId) -> bool { }) } -pub fn has_space(pipe_id: PipeId) -> bool { - with_pipes(|pipes| { - pipes.get(pipe_id).is_some_and(|p| p.holders.writable(p.space())) - }) +/// Whether a write needing `want` bytes at once answers now. +pub fn has_space(pipe_id: PipeId, want: usize) -> bool { + let want = u32::try_from(want).unwrap_or(u32::MAX); + with_pipes(|pipes| pipes.get(pipe_id).is_some_and(|p| p.holders.writable(p.space(), want))) } /// Whether the end opposite `end` of this pipe has no holder left. diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index 5174514df7d..024e79a9cba 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -35,7 +35,7 @@ use super::io::{ sys_null, sys_random, sys_read, sys_read_nonblock, sys_write, sys_write_nonblock, }; use super::ipc::{ - sys_accept, sys_handle_recv, sys_handle_send, sys_inbox_setup, + sys_accept, sys_handle_recv, sys_inbox_setup, sys_message_send, sys_inbox_submit, sys_namespace_build, sys_namespace_open, sys_pipe, sys_pipe_map, sys_port_badge, sys_port_create, sys_port_mint, sys_shm_create, sys_shm_map, }; @@ -450,17 +450,26 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> sys_trace_read(&ctx, RawHandle(a1 as u32), UserAddr::new(a2), &mut out, a4 as usize) } SYS_ACCEPT => sys_accept(RawHandle(a1 as u32)), - SYS_HANDLE_SEND => { - if a3 as usize > MAX_TRANSFER_HANDLES { + SYS_MESSAGE_SEND => { + let Ok(args) = ctx.copy_in::(UserAddr::new(a2)) else { return bad_addr }; + let unknown_flags = args.flags & !MESSAGE_NONBLOCK != 0; + if unknown_flags || args.count as usize > MAX_TRANSFER_HANDLES || args.len as usize > MAX_MESSAGE_LEN { return SyscallError::InvalidArgument.to_u64(); } - let Some(handles) = (a3 as usize) + let Some(bytes) = ctx.user_bytes(UserAddr::new(args.bytes), args.len) else { return bad_addr }; + let Some(handles) = (args.count as usize) .checked_mul(HANDLE_LEN) - .and_then(|len| ctx.user_bytes(UserAddr::new(a2), len as u64)) + .and_then(|len| ctx.user_bytes(UserAddr::new(args.handles), len as u64)) else { return bad_addr; }; - sys_handle_send(RawHandle(a1 as u32), &handles, a3 as usize) + sys_message_send( + RawHandle(a1 as u32), + &bytes, + &handles, + args.count as usize, + args.flags & MESSAGE_NONBLOCK != 0, + ) } SYS_HANDLE_RECV => { if a3 as usize > MAX_TRANSFER_HANDLES { diff --git a/kernel/src/syscall/io.rs b/kernel/src/syscall/io.rs index 2320dc99fc8..d231a185eba 100644 --- a/kernel/src/syscall/io.rs +++ b/kernel/src/syscall/io.rs @@ -74,7 +74,7 @@ pub(super) fn sys_write(h: RawHandle, buf: &UserBytes) -> u64 { 0, class, Deadline::never(), - || pipe::has_space(id), + || pipe::has_space(id, 1), ) .is_err() { diff --git a/kernel/src/syscall/ipc.rs b/kernel/src/syscall/ipc.rs index a9470793b69..d52b47b5ee8 100644 --- a/kernel/src/syscall/ipc.rs +++ b/kernel/src/syscall/ipc.rs @@ -12,6 +12,7 @@ use alloc::vec::Vec; use crate::watch; use crate::mm::policy::Prot; use crate::object::{ops, port, KObjectRef}; +use crate::object::service::SendRefused; use crate::time::Deadline; use crate::user_ptr::SyscallContext; use crate::UserAddr; @@ -359,8 +360,17 @@ pub(super) fn sys_shm_map(h: RawHandle) -> u64 { } } -/// Move handles to the peer of a connection, all or nothing; rights, including `TRANSFER`, travel unchanged. -pub(super) fn sys_handle_send(conn_h: RawHandle, handles: &crate::user_ptr::UserBytes, count: usize) -> u64 { +/// Write a message's bytes to a connection and move its handles to the peer, +/// together or not at all ([`toyos_abi::syscall::message_send`]). Rights, +/// `TRANSFER` included, travel unchanged. Waits for room in the stream unless +/// `nonblock`; the copy is the caller's, bounded by `MAX_MESSAGE_LEN`. +pub(super) fn sys_message_send( + conn_h: RawHandle, + bytes: &crate::user_ptr::UserBytes, + handles: &crate::user_ptr::UserBytes, + count: usize, + nonblock: bool, +) -> u64 { let mut wanted = [RawHandle(0); MAX_TRANSFER_HANDLES]; for (i, slot) in wanted.iter_mut().enumerate().take(count) { let mut raw = [0u8; HANDLE_LEN]; @@ -368,32 +378,52 @@ pub(super) fn sys_handle_send(conn_h: RawHandle, handles: &crate::user_ptr::User *slot = RawHandle(u32::from_ne_bytes(raw)); } let wanted = &wanted[..count]; + // `TRANSFER` on the connection only when it carries handles: bytes alone + // are a write, which `WRITE` already allows. + let need = if count == 0 { Rights::WRITE } else { Rights::WRITE.union(Rights::TRANSFER) }; - let sent = process::with_process_data(|data| { - let conn = data - .handles - .get::(conn_h, Rights::TRANSFER)?; - for (i, h) in wanted.iter().enumerate() { - if *h == conn_h || wanted[..i].contains(h) { - return Err(SyscallError::InvalidArgument.into()); + loop { + let sent = process::with_process_data(|data| { + let conn = data.handles.get::(conn_h, need)?; + // Every handle is checked before anything is written or moved. + for (i, h) in wanted.iter().enumerate() { + if *h == conn_h || wanted[..i].contains(h) { + return Err(SyscallError::InvalidArgument.into()); + } + let rights = data.handles.rights_of(*h)?; + if !rights.contains(Rights::TRANSFER) { + return Err(SyscallError::PermissionDenied.into()); + } } - let rights = data.handles.rights_of(*h)?; - if !rights.contains(Rights::TRANSFER) { - return Err(SyscallError::PermissionDenied.into()); + // A refusal restores every handle at its own number, so it is invisible. + let moved = data.handles.transfer(wanted, |batch| conn.send(bytes, batch)); + Ok::<_, crate::object::Refusal>((conn.tx(), moved)) + }); + let tx = match sent { + Ok((tx, Ok(()))) => { + process::wake_pipe_readers(tx); + return 0; } + Ok((tx, Err(SendRefused::NoRoom))) if !nonblock => tx, + Ok((_, Err(SendRefused::NoRoom))) => return SyscallError::WouldBlock.to_u64(), + Ok((_, Err(SendRefused::Error(e)))) => return e.to_u64(), + Err(e) => return e.refuse(), + }; + // Outside every lock: room appears as the reader drains, and its read posts this watch. + let Some(watch) = pipe::write_watch(tx) else { return SyscallError::Gone.to_u64() }; + let len = bytes.len(); + if watch::wait_until(&crate::scheduler::Parkable::at_entry(), &watch, 0, WaitClass::Ipc, Deadline::never(), || { + pipe::has_space(tx, len) + }) + .is_err() + { + return cancelled(); } - // The peer's queue can still refuse; transfer restores every entry at its own number so a refusal is invisible. - data.handles - .transfer(wanted, |batch| conn.send(batch)) - .map_err(crate::object::Refusal::Error) - }); - match sent { - Ok(()) => 0, - Err(e) => e.refuse(), } } -/// Take the oldest batch the peer sent. Never blocks; zero means none queued. +/// Take the batch pinned at the reader's place in the stream. Never blocks; +/// zero means no batch sits here. pub(super) fn sys_handle_recv( conn_h: RawHandle, out: &mut crate::user_ptr::UserBytesMut, @@ -403,16 +433,8 @@ pub(super) fn sys_handle_recv( let conn = data .handles .get::(conn_h, Rights::READ)?; - // The front measured here is the front recv_bounded takes: only the peer pushes, and only to the far end. - // Width is measured before the batch is taken: refusing after popping would drop already-taken handles, leaking capabilities nobody could ask for again. - let Some(width) = conn.peek_width() else { return Ok(0) }; - if width > cap { - return Err(SyscallError::InvalidArgument.into()); - } - if !data.handles.has_room(width) { - return Err(SyscallError::ResourceExhausted.into()); - } - let Some(batch) = conn.recv_bounded(cap)? else { return Ok(0) }; + // Width and room are measured before the batch is taken: refusing after taking would drop handles nobody could ask for again. + let Some(batch) = conn.recv(cap, |width| data.handles.has_room(width))? else { return Ok(0) }; let count = batch.len(); for (i, entry) in batch.into_iter().enumerate() { let h = data.handles.install(entry).expect("room was asked for first"); diff --git a/lib/blockserve/src/link.rs b/lib/blockserve/src/link.rs index 102009d6dc0..9a581b166d5 100644 --- a/lib/blockserve/src/link.rs +++ b/lib/blockserve/src/link.rs @@ -134,9 +134,9 @@ impl Link { RxStep::Idle => return handed, RxStep::Eof => "its process ended", RxStep::Malformed => "a frame this protocol does not make", - RxStep::Frame { msg_type: disk::MSG_SESSION, payload_len } => { + RxStep::Frame { msg_type: disk::MSG_SESSION, payload_len, .. } => { let session = disk::Session::decode(self.rx.payload(payload_len)); - let handles = conn.recv_handles_exact::<2>(); + let handles = self.rx.take_handles::<2>(); match (session, handles) { (Some(session), Some([conn, region])) => { let conn = Connection::received(conn); diff --git a/lib/logkeeper-api/src/lib.rs b/lib/logkeeper-api/src/lib.rs index 59e8bac9341..0a18aad8da1 100644 --- a/lib/logkeeper-api/src/lib.rs +++ b/lib/logkeeper-api/src/lib.rs @@ -24,7 +24,7 @@ pub fn read() -> Result { } let boot_so_far: u64 = conn.recv_payload(&header).map_err(|e| format!("logkeeper's answer is not one `u64`: {e:?}"))?; - let [raw] = conn.recv_handles_exact::<1>().ok_or("logkeeper's answer carried no pipe")?; + let [raw] = conn.recv_handles_exact::<1>(&header).ok_or("logkeeper's answer carried no pipe")?; // SAFETY: the kernel moved this handle into this table with the frame // that names it, and nothing else answers for it. let pipe = unsafe { Pipe::from_raw(raw) }; diff --git a/sdk/abi/src/syscall.rs b/sdk/abi/src/syscall.rs index c272cdc8840..c21157f8edc 100644 --- a/sdk/abi/src/syscall.rs +++ b/sdk/abi/src/syscall.rs @@ -162,13 +162,8 @@ pub const SYS_NAMESPACE_BUILD: u64 = 101; /// no other place to ask, and a name it was not given resolves to nothing. pub const SYS_NAMESPACE_OPEN: u64 = 102; -/// Move handles to the peer of a connection. See [`handle_send`]. -/// -/// The batch is queued on the connection, not interleaved with its bytes, so -/// **handles are sent before the frame that announces them** and a receiver -/// that has the frame already has the handles. -pub const SYS_HANDLE_SEND: u64 = 103; -/// Take the oldest batch of handles the peer sent. See [`handle_recv`]. +/// Take the batch of handles at the reader's place in the connection's bytes. +/// See [`handle_recv`]. pub const SYS_HANDLE_RECV: u64 = 104; /// Make a shared-memory region, sized up to whole [`SHM_PAGE`]s. /// See [`shm_create`]. @@ -294,6 +289,9 @@ pub const SYS_NULL: u64 = 130; pub const SYS_DEVICE_DMA_LIMIT: u64 = 131; /// Let a claimed PCI function master the bus. See [`device_dma_start`]. pub const SYS_DEVICE_DMA_START: u64 = 132; +/// Write a message's bytes to a connection and move the handles it carries, +/// together or not at all. See [`message_send`]. +pub const SYS_MESSAGE_SEND: u64 = 133; /// Bins in the per-process syscall profile — one for every number this ABI /// issues, and one at the end for every number it does not. @@ -301,13 +299,13 @@ pub const SYS_DEVICE_DMA_START: u64 = 132; /// **The profile's parts sum to its total, and that is the whole requirement.** /// A bin array narrower than the ABI reaches drops calls out of the line while /// the total goes on counting them. -pub const SYSCALL_PROFILE_BINS: usize = 134; +pub const SYSCALL_PROFILE_BINS: usize = 135; /// Where a number this ABI does not issue is counted. Merging is a degradation /// a reader can see in the line; dropping is one nobody can. pub const SYSCALL_PROFILE_OTHER: usize = SYSCALL_PROFILE_BINS - 1; -const _: () = assert!(SYS_DEVICE_DMA_START < SYSCALL_PROFILE_OTHER as u64); +const _: () = assert!(SYS_MESSAGE_SEND < SYSCALL_PROFILE_OTHER as u64); pub const WNOHANG: u64 = 1; @@ -484,6 +482,9 @@ pub enum SyscallError { /// the same rule the storage layer already obeys applies here: a dead peer /// must not be indistinguishable from a handle that was never there. Gone = 10, + /// A read of a connection reached a batch of handles the peer sent with + /// the bytes before it: take the batch ([`handle_recv`]) before reading on. + HandlesPending = 11, } impl SyscallError { @@ -508,6 +509,7 @@ impl SyscallError { 8 => Some(Self::NotSupported), 9 => Some(Self::Io), 10 => Some(Self::Gone), + 11 => Some(Self::HandlesPending), _ => Some(Self::Unknown), } } @@ -527,6 +529,7 @@ impl core::fmt::Display for SyscallError { Self::NotSupported => f.write_str("not supported"), Self::Io => f.write_str("the device did not complete the transfer"), Self::Gone => f.write_str("the other end is gone"), + Self::HandlesPending => f.write_str("handles the peer sent wait at this point of the stream"), } } } @@ -1863,7 +1866,7 @@ pub fn accept(acceptor: RawHandle) -> Result { check(syscall(SYS_ACCEPT, acceptor.0 as u64, 0, 0, 0)).map(|v| RawHandle(v as u32)) } -/// Handles one [`handle_send`] may carry. +/// Handles one [`message_send`] may carry. /// /// Policy on the primitive: a caller asking for one more is refused by name /// and never truncated to fit. @@ -1872,31 +1875,62 @@ pub const MAX_TRANSFER_HANDLES: usize = 8; /// Batches one direction of a connection may hold unreceived. pub const MAX_QUEUED_BATCHES: usize = 16; -/// Move `handles` to the peer of `conn`. -/// -/// Each handle must carry [`Rights::TRANSFER`], and so must `conn`. The move is -/// all-or-nothing: a refusal leaves every handle where it was. +/// The most bytes one [`message_send`] writes. Policy on the primitive: the +/// bytes are copied whole under the connection's locks, so the copy is +/// bounded, and every frame a protocol here sends is far smaller. +pub const MAX_MESSAGE_LEN: usize = 16 * 1024; + +/// [`message_send`] answers `WouldBlock` rather than waiting for room. +pub const MESSAGE_NONBLOCK: u64 = 1; + +crate::user_safe! { + /// Arguments for `SYS_MESSAGE_SEND`, passed as a single pointer. + #[derive(Clone, Copy)] + pub struct MessageSend { + pub bytes: u64, + pub len: u64, + pub handles: u64, + pub count: u64, + /// [`MESSAGE_NONBLOCK`] or none; any other bit is `InvalidArgument`. + pub flags: u64, + } +} + +/// Write `bytes` to `conn` and move `handles` to its peer, as one message. /// -/// **Send the handles before the frame that announces them.** They travel in a -/// queue of their own rather than interleaved with the connection's bytes, so a -/// peer that has read the frame is guaranteed to find them only in that order. +/// **All or nothing, and in order.** The bytes go into the connection's +/// stream whole, and the handles are queued at the byte just past them: the +/// peer reads the bytes, then takes the batch ([`handle_recv`]), and no read +/// of its crosses the batch before it is taken. Nothing is written and no +/// handle moves on a refusal: +/// - every handle must carry [`Rights::TRANSFER`], and so must `conn` when +/// there are any; `conn` itself and a handle named twice are refused; +/// - handles need bytes (`InvalidArgument`), and past [`MAX_MESSAGE_LEN`] +/// bytes or [`MAX_TRANSFER_HANDLES`] handles is `InvalidArgument`; +/// - a peer already holding [`MAX_QUEUED_BATCHES`] is `ResourceExhausted`, and +/// a peer that is gone is `Gone`; +/// - a stream without room for the whole message waits for it, or with +/// `nonblock` answers `WouldBlock`. /// /// [`Rights::TRANSFER`]: crate::handle::Rights::TRANSFER -pub fn handle_send(conn: RawHandle, handles: &[RawHandle]) -> Result<(), SyscallError> { - check_unit(syscall( - SYS_HANDLE_SEND, - conn.0 as u64, - handles.as_ptr() as u64, - handles.len() as u64, - 0, - )) +pub fn message_send(conn: RawHandle, bytes: &[u8], handles: &[RawHandle], nonblock: bool) -> Result<(), SyscallError> { + let args = MessageSend { + bytes: bytes.as_ptr() as u64, + len: bytes.len() as u64, + handles: handles.as_ptr() as u64, + count: handles.len() as u64, + flags: if nonblock { MESSAGE_NONBLOCK } else { 0 }, + }; + check_unit(syscall(SYS_MESSAGE_SEND, conn.0 as u64, &args as *const MessageSend as u64, 0, 0)) } -/// Take the oldest batch the peer sent, into `out`. Answers how many it wrote. +/// Take the batch the peer sent at this reader's place in the connection's +/// bytes, into `out`. Answers how many it wrote. /// -/// Never blocks: zero means nothing is queued right now. A batch larger than -/// `out` is `InvalidArgument` and stays queued — `out` should be -/// [`MAX_TRANSFER_HANDLES`] long. +/// Never blocks: zero means no batch sits here, which for a reader that has +/// just read a message carrying handles is a peer that broke its protocol. A +/// batch larger than `out` is `InvalidArgument` and stays queued — `out` +/// should be [`MAX_TRANSFER_HANDLES`] long. pub fn handle_recv(conn: RawHandle, out: &mut [RawHandle]) -> Result { check(syscall( SYS_HANDLE_RECV, diff --git a/sdk/libc/src/errcode.rs b/sdk/libc/src/errcode.rs index 4741026c6b3..63909fbbacb 100644 --- a/sdk/libc/src/errcode.rs +++ b/sdk/libc/src/errcode.rs @@ -34,6 +34,7 @@ pub(crate) const ENOPROTOOPT: i32 = 92; pub(crate) const EOPNOTSUPP: i32 = 95; pub(crate) const EAFNOSUPPORT: i32 = 97; pub(crate) const EADDRINUSE: i32 = 98; +pub(crate) const EPROTO: i32 = 71; pub(crate) const ECONNABORTED: i32 = 103; pub(crate) const ECONNRESET: i32 = 104; pub(crate) const ENOTCONN: i32 = 107; @@ -52,6 +53,8 @@ pub(crate) fn of(e: SyscallError) -> i32 { // A write whose reader is gone, and not `ENOENT`, which says the file does not exist. SyscallError::Gone => EPIPE, SyscallError::Io => EIO, + // A read of a connection met handles no frame announced: the peer broke its protocol. + SyscallError::HandlesPending => EPROTO, _ => EINVAL, } } diff --git a/sdk/std/sys/pal/mod.rs b/sdk/std/sys/pal/mod.rs index d06fc276644..17d54b95896 100644 --- a/sdk/std/sys/pal/mod.rs +++ b/sdk/std/sys/pal/mod.rs @@ -31,6 +31,8 @@ pub fn to_io_error(e: toyos_abi::syscall::SyscallError) -> crate::io::Error { SyscallError::NotSupported => ErrorKind::Unsupported, SyscallError::Io => ErrorKind::Other, SyscallError::Gone => ErrorKind::BrokenPipe, + // A read of a connection met handles the peer sent with no frame announcing them. + SyscallError::HandlesPending => ErrorKind::InvalidData, }; crate::io::Error::from(kind) } diff --git a/sdk/toyos/src/audio.rs b/sdk/toyos/src/audio.rs index cb2cd8d61ac..df93ad6bc45 100644 --- a/sdk/toyos/src/audio.rs +++ b/sdk/toyos/src/audio.rs @@ -223,8 +223,8 @@ pub enum AudioError { /// soundserver closed the signal pipe (daemon exit or client removal). Disconnected, /// soundserver announced the stream and did not send the ring and the signal - /// pipe with it. Handles cross before the frame that names them, so a - /// short batch is a protocol violation and never something to wait for. + /// pipe with it. Handles travel in the frame that names them, so a short + /// batch is a protocol violation and never something to wait for. MissingHandles, /// The slot ring soundserver sent is refused ([`SharedMemory::lent`]). Lent(crate::shm::Lent), @@ -269,7 +269,7 @@ impl AudioStream { }; let batch = control - .recv_handles_exact::() + .recv_handles_exact::(&header) .ok_or(AudioError::MissingHandles)?; let signal = crate::Pipe(crate::OwnedHandle(batch[STREAM_OPENED_SIGNAL])); diff --git a/sdk/toyos/src/fs.rs b/sdk/toyos/src/fs.rs index aeeca64acd5..6a85cf00b7d 100644 --- a/sdk/toyos/src/fs.rs +++ b/sdk/toyos/src/fs.rs @@ -31,7 +31,7 @@ //! here keeps a write the server acknowledged and never made durable: that is //! what `Fsync` is for. -use toyos_abi::syscall::{SyscallError, MAX_BADGE, MAX_SERVICE_NAME}; +use toyos_abi::syscall::{SyscallError, MAX_BADGE, MAX_SERVICE_NAME, MAX_TRANSFER_HANDLES}; use crate::ipc::{Connection, IpcError}; use crate::ipc_payload; @@ -519,6 +519,24 @@ impl Dir { /// One request and its reply on the current connection. A connection that /// has ended answers `Gone` and is dropped, so the next call reconnects. fn call(&mut self, op: u32, request: &Request) -> Result<(u32, Reply), Error> { + let mut batch = [toyos_abi::HANDLE_INVALID; MAX_TRANSFER_HANDLES]; + let (msg_type, reply, carried) = self.call_with(op, request, &mut batch)?; + if carried != 0 { + // A reply that brings handles nobody asked for speaks another protocol. + close_all(&batch[..carried]); + self.conn = None; + return Err(Error::Malformed); + } + Ok((msg_type, reply)) + } + + /// [`Self::call`], and the handles the reply frame carries into `batch`. + fn call_with( + &mut self, + op: u32, + request: &Request, + batch: &mut [RawHandle; MAX_TRANSFER_HANDLES], + ) -> Result<(u32, Reply, usize), Error> { if self.conn.is_none() { self.reconnect()?; } @@ -526,7 +544,8 @@ impl Dir { let answered = conn.send(op, request).map_err(transport).and_then(|()| { let header = conn.recv_header().map_err(transport)?; let reply: Reply = conn.recv_payload(&header).map_err(transport)?; - Ok((header.msg_type, reply)) + let carried = conn.recv_handles(&header, batch).map_err(transport)?; + Ok((header.msg_type, reply, carried)) }); if answered.is_err() { self.conn = None; @@ -627,24 +646,20 @@ impl Dir { /// A pipe whose bytes the server appends to this file from `offset` on, /// for a program's standard output: the write end. pub fn stream(&mut self, fid: u64, generation: u64, offset: u64) -> Result { - self.fid_call(STREAM, generation, &Request { fid, offset, ..Request::new() })?; - let conn = self.conn.as_ref().expect("a call answered on it"); - let mut batch = [toyos_abi::HANDLE_INVALID; toyos_abi::syscall::MAX_TRANSFER_HANDLES]; - match conn.recv_handles(&mut batch) { - Ok(1) => Ok(Pipe(crate::OwnedHandle(batch[0]))), - Ok(n) => { - for &h in &batch[..n] { - toyos_abi::syscall::close(h); - } + if generation != self.generation || self.conn.is_none() { + return Err(Error::Stale); + } + let mut batch = [toyos_abi::HANDLE_INVALID; MAX_TRANSFER_HANDLES]; + let (msg_type, reply, carried) = + self.call_with(STREAM, &Request { fid, offset, ..Request::new() }, &mut batch)?; + match (answer(msg_type, reply), carried) { + (Ok(_), 1) => Ok(Pipe(crate::OwnedHandle(batch[0]))), + (Err(e), 0) => Err(e), + (_, n) => { + close_all(&batch[..n]); self.conn = None; Err(Error::Malformed) } - // A table with no room leaves the pipe queued, where the next - // stream's answer would find it: the connection goes with it. - Err(e) => { - self.conn = None; - Err(Error::kernel("a stream's pipe", e)) - } } } @@ -765,6 +780,12 @@ pub fn encode_entry(out: &mut [u8], kind: u64, size: u64, name: &str) -> Option< Some(need) } +fn close_all(handles: &[RawHandle]) { + for h in handles { + toyos_abi::syscall::close(*h); + } +} + fn stat_of(reply: &Reply, volume: u64) -> Stat { Stat { kind: reply.kind, size: reply.value2, mtime: reply.mtime, volume, node: reply.node } } @@ -775,11 +796,11 @@ fn stat_of(reply: &Reply, volume: u64) -> Stat { /// A server that refuses a connection as it takes it answers before it lets /// go, so a hello that finds it gone still reads why. fn hello(conn: &Connection, window: &SharedMemory) -> Result { + // The window travels in the HELLO frame itself, as one message. let sent = window .share() - .and_then(|lent| conn.send_handles([lent])) .map_err(|e| Error::kernel("a window's loan", e)) - .and_then(|()| conn.send(HELLO, &Request::new()).map_err(transport)); + .and_then(|lent| conn.send_with(HELLO, &Request::new(), [lent]).map_err(transport)); match sent { Ok(()) | Err(Error::Gone) => {} Err(e) => return Err(e), diff --git a/sdk/toyos/src/ipc.rs b/sdk/toyos/src/ipc.rs index 73fd9e05cd3..1723fc9c021 100644 --- a/sdk/toyos/src/ipc.rs +++ b/sdk/toyos/src/ipc.rs @@ -1,6 +1,13 @@ -//! Framed IPC over sockets (bidirectional pipe pairs). +//! Framed IPC over connections (bidirectional pipe pairs). //! -//! Wire format: `[u32 msg_type][u32 len][len bytes payload]`. +//! Wire format: `[u32 msg_type][u32 len][u32 handles][len bytes payload]`. +//! +//! **A frame and the handles it announces travel as one message** +//! (`SYS_MESSAGE_SEND`): the kernel writes the frame whole and pins its batch +//! at the byte just past it, so the receiver reads the frame and takes exactly +//! the batch it announces, there. A read that reaches a batch no frame +//! announced is answered `HandlesPending`, which is the peer breaking the +//! protocol; the connection is dropped and the batch goes with it. use toyos_abi::RawHandle; use toyos_abi::syscall::{self, SyscallError}; @@ -85,25 +92,33 @@ macro_rules! ipc_payload { /// Bounded, that frame is an error at the header and the peer can be dropped. pub const MAX_FRAME_LEN: u32 = 8192; -/// A frame header. `len` is private because it carries an invariant a peer -/// does not: every `IpcHeader` in existence has a length within -/// [`MAX_FRAME_LEN`], because [`IpcHeader::from_wire`] is the only way to make -/// one out of bytes somebody else chose. +/// A frame header. `len` and `handles` are private because they carry +/// invariants a peer does not: every `IpcHeader` in existence has a length +/// within [`MAX_FRAME_LEN`] and a count within +/// [`MAX_TRANSFER_HANDLES`](syscall::MAX_TRANSFER_HANDLES), because +/// [`IpcHeader::from_wire`] is the only way to make one out of bytes somebody +/// else chose. #[derive(Clone, Copy)] pub struct IpcHeader { pub msg_type: u32, len: u32, + handles: u32, } impl IpcHeader { - pub const WIRE_SIZE: usize = 8; + pub const WIRE_SIZE: usize = 12; - /// Build a header from a length that has not been trusted yet. - pub fn from_wire(msg_type: u32, len: u32) -> Result { - if len > MAX_FRAME_LEN { + /// Build a header from words that have not been trusted yet. + pub fn from_wire(msg_type: u32, len: u32, handles: u32) -> Result { + if len > MAX_FRAME_LEN || handles as usize > syscall::MAX_TRANSFER_HANDLES { return Err(IpcError::Malformed); } - Ok(Self { msg_type, len }) + Ok(Self { msg_type, len, handles }) + } + + fn decode(bytes: &[u8; Self::WIRE_SIZE]) -> Result { + let word = |at: usize| u32::from_ne_bytes(bytes[at..at + 4].try_into().expect("four bytes")); + Self::from_wire(word(0), word(4), word(8)) } pub fn len(&self) -> u32 { @@ -114,30 +129,34 @@ impl IpcHeader { self.len == 0 } + /// How many handles travel with this frame. + pub fn handles(&self) -> usize { + self.handles as usize + } + fn to_wire(self) -> [u8; Self::WIRE_SIZE] { let mut bytes = [0u8; Self::WIRE_SIZE]; bytes[..4].copy_from_slice(&self.msg_type.to_ne_bytes()); - bytes[4..].copy_from_slice(&self.len.to_ne_bytes()); + bytes[4..8].copy_from_slice(&self.len.to_ne_bytes()); + bytes[8..].copy_from_slice(&self.handles.to_ne_bytes()); bytes } - /// The same header, built from a length this endpoint chose itself. - fn frame(msg_type: u32, len: usize) -> Result { - if len > MAX_FRAME_LEN as usize { + /// The same header, built from a length and count this endpoint chose itself. + fn frame(msg_type: u32, len: usize, handles: usize) -> Result { + if len > MAX_FRAME_LEN as usize || handles > syscall::MAX_TRANSFER_HANDLES { return Err(IpcError::TooLarge); } - Ok(Self { msg_type, len: len as u32 }) + Ok(Self { msg_type, len: len as u32, handles: handles as u32 }) } } -/// The largest typed payload [`try_send`] frames without allocating. -/// -/// Every `ipc_payload!` struct in the tree is far below this — the largest is -/// `window::CreateWindowRequest` at 40 bytes. A type past it does not compile, -/// because the alternative would be a payload split across two writes, and a -/// non-blocking send that can be half-refused is the thing [`try_send`] exists -/// to make unrepresentable. -pub const MAX_TYPED_PAYLOAD: usize = 64; +const _: () = assert!(IpcHeader::WIRE_SIZE + MAX_FRAME_LEN as usize <= syscall::MAX_MESSAGE_LEN); + +/// A payload of at most this many bytes, such as a signal or a small typed +/// payload sent per input event, is framed in a buffer this size rather than +/// one of [`MAX_FRAME_LEN`], which would be zeroed on every send. +const SMALL_PAYLOAD: usize = 64; #[derive(Debug)] pub enum IpcError { @@ -155,13 +174,9 @@ pub enum IpcError { /// Why a whole frame could not be handed over without blocking. #[derive(Debug)] pub enum TrySendError { - /// The peer's pipe would not take the whole frame in one write. - /// - /// **The connection is no longer at a message boundary.** The kernel - /// accepts a short write and there is no way to retract the part it took, - /// so there is nothing here to retry: the only correct answer is to drop - /// the peer. That is not a harsh reading of a busy moment — a peer that - /// cannot take one frame has an entire pipe of unread messages behind it. + /// The peer's stream has no room for the whole frame now. **Nothing was + /// sent**: a frame goes in whole or not at all. A peer that cannot take + /// one frame has a stream of unread messages behind it, and is dropped. Full, /// This endpoint will not frame a payload that large. TooLarge, @@ -185,7 +200,7 @@ impl Connection { Self(OwnedHandle(raw)) } - /// This end as a handle, to send it on ([`Self::send_handles`]). + /// This end as a handle, to send it on in a frame ([`Self::send_with`]). pub fn into_owned(self) -> OwnedHandle { self.0 } @@ -214,6 +229,66 @@ impl Connection { try_send_bytes(self.0.raw(), msg_type, data) } + /// A frame and the handles it carries, as one message. + /// + /// **Every handle is consumed, sent or not**: a refused send closes them + /// all, so no caller holds a handle it believes it gave away. More than + /// [`MAX_TRANSFER_HANDLES`](syscall::MAX_TRANSFER_HANDLES) is refused + /// `InvalidArgument`, as the kernel refuses it. + pub fn send_with( + &self, + msg_type: u32, + payload: &T, + handles: impl IntoIterator, + ) -> Result<(), IpcError> { + message(self.0.raw(), msg_type, as_bytes(payload), handles, false).map_err(blocking) + } + + /// [`send_with`](Self::send_with) for a byte payload. + pub fn send_bytes_with( + &self, + msg_type: u32, + data: &[u8], + handles: impl IntoIterator, + ) -> Result<(), IpcError> { + message(self.0.raw(), msg_type, data, handles, false).map_err(blocking) + } + + /// [`send_with`](Self::send_with) for a bare header. + pub fn signal_with(&self, msg_type: u32, handles: impl IntoIterator) -> Result<(), IpcError> { + message(self.0.raw(), msg_type, &[], handles, false).map_err(blocking) + } + + /// [`send_with`](Self::send_with) without blocking: nothing is sent and + /// every handle is closed when the frame does not fit now. + pub fn try_send_with( + &self, + msg_type: u32, + payload: &T, + handles: impl IntoIterator, + ) -> Result<(), TrySendError> { + message(self.0.raw(), msg_type, as_bytes(payload), handles, true).map_err(nonblocking) + } + + /// [`try_send_with`](Self::try_send_with) for a byte payload. + pub fn try_send_bytes_with( + &self, + msg_type: u32, + data: &[u8], + handles: impl IntoIterator, + ) -> Result<(), TrySendError> { + message(self.0.raw(), msg_type, data, handles, true).map_err(nonblocking) + } + + /// [`try_send_with`](Self::try_send_with) for a bare header. + pub fn try_signal_with( + &self, + msg_type: u32, + handles: impl IntoIterator, + ) -> Result<(), TrySendError> { + message(self.0.raw(), msg_type, &[], handles, true).map_err(nonblocking) + } + pub fn recv_header(&self) -> Result { recv_header(self.0.raw()) } @@ -226,53 +301,28 @@ impl Connection { recv(self.0.raw()) } - /// Move `handles` to the peer, as the batch the next frame announces. - /// - /// **Every handle is consumed, sent or not**: a refused move closes them - /// all, so no caller holds a handle it believes it gave away. More than - /// [`MAX_TRANSFER_HANDLES`](syscall::MAX_TRANSFER_HANDLES) is refused - /// `InvalidArgument`, as the kernel refuses it. - /// - /// **The handles go before the frame that announces them.** They travel in - /// a queue of their own rather than interleaved with the bytes, so a peer - /// that has read the frame is guaranteed to find them — and a peer that - /// has not is guaranteed not to act on them early. A frame refused after a - /// move leaves the batch in the peer's queue, which releases it with the - /// connection. - pub fn send_handles( - &self, - handles: impl IntoIterator, - ) -> Result<(), SyscallError> { - move_batch(handles, |batch| syscall::handle_send(self.0.raw(), batch)) - } - - /// Take the batch the peer sent with the frame just received. - /// - /// Never blocks. An empty answer for a frame that promised handles is a - /// peer that sent them out of order, which is a protocol error and not - /// something to wait for. + /// The handles `header`'s frame announced, read after its payload: exactly + /// that many, or [`IpcError::Malformed`] with whatever arrived closed. The + /// kernel pins a frame's batch just past its bytes, so they are there. pub fn recv_handles( &self, + header: &IpcHeader, out: &mut [RawHandle; syscall::MAX_TRANSFER_HANDLES], - ) -> Result { - syscall::handle_recv(self.0.raw(), out) + ) -> Result { + recv_handles(self.0.raw(), header, out) } - /// The `N` handles this frame's message type says travel with it. + /// The `N` handles a frame of this message type carries. /// /// `None` for any other count, with whatever did arrive closed rather than - /// leaked: a short batch is a peer that sent its frame first, a long one is - /// a protocol this endpoint does not speak, and neither is something to - /// wait for. Every protocol here sends a fixed number per message type, so - /// this is the shape every caller wants. - pub fn recv_handles_exact(&self) -> Option<[RawHandle; N]> { + /// leaked: a frame announcing another number speaks a protocol this + /// endpoint does not. + pub fn recv_handles_exact(&self, header: &IpcHeader) -> Option<[RawHandle; N]> { const { assert!(N <= syscall::MAX_TRANSFER_HANDLES) }; let mut batch = [toyos_abi::HANDLE_INVALID; syscall::MAX_TRANSFER_HANDLES]; - let n = self.recv_handles(&mut batch).ok()?; + let n = self.recv_handles(header, &mut batch).ok()?; if n != N { - for h in &batch[..n] { - syscall::close(*h); - } + close_all(&batch[..n]); return None; } Some(core::array::from_fn(|i| batch[i])) @@ -286,6 +336,8 @@ impl Connection { self.0.read_nonblock(buf) } + /// Raw bytes, for a connection a protocol has made a doorbell: never a + /// frame, which [`send`](Self::send) and its kin write whole. pub fn write_nonblock(&self, buf: &[u8]) -> Result { self.0.write_nonblock(buf) } @@ -295,38 +347,94 @@ impl Connection { // Will become pub(crate) once all callers migrate to Connection methods. pub fn send(handle: RawHandle, msg_type: u32, payload: &T) -> Result<(), IpcError> { - let header = IpcHeader::frame(msg_type, core::mem::size_of::())?; - write_all(handle, &header.to_wire())?; - write_all(handle, as_bytes(payload)) + message(handle, msg_type, as_bytes(payload), none(), false).map_err(blocking) } pub fn signal(handle: RawHandle, msg_type: u32) -> Result<(), IpcError> { - write_all(handle, &IpcHeader { msg_type, len: 0 }.to_wire()) + message(handle, msg_type, &[], none(), false).map_err(blocking) } pub fn send_bytes(handle: RawHandle, msg_type: u32, data: &[u8]) -> Result<(), IpcError> { - let header = IpcHeader::frame(msg_type, data.len())?; - write_all(handle, &header.to_wire())?; - if !data.is_empty() { - write_all(handle, data)?; - } - Ok(()) + message(handle, msg_type, data, none(), false).map_err(blocking) } /// Send a framed payload without ever blocking, or refuse the whole frame. /// /// A server writing to a client it does not trust to read cannot use [`send`]: -/// `write_all` parks in the kernel until the peer drains, which is a client -/// deciding when the server runs again. This is the same frame in one -/// `write_nonblock`, so the peer's backlog is an answer rather than a wait. +/// a blocking send parks in the kernel until the peer drains, which is a +/// client deciding when the server runs again. This is the same frame, and the +/// peer's backlog is an answer rather than a wait. pub fn try_send(handle: RawHandle, msg_type: u32, payload: &T) -> Result<(), TrySendError> { - const { assert!(core::mem::size_of::() <= MAX_TYPED_PAYLOAD) }; - let size = core::mem::size_of::(); - let mut frame = [0u8; IpcHeader::WIRE_SIZE + MAX_TYPED_PAYLOAD]; - let header = IpcHeader::frame(msg_type, size).map_err(|_| TrySendError::TooLarge)?; - frame[..IpcHeader::WIRE_SIZE].copy_from_slice(&header.to_wire()); - frame[IpcHeader::WIRE_SIZE..IpcHeader::WIRE_SIZE + size].copy_from_slice(as_bytes(payload)); - write_whole(handle, &frame[..IpcHeader::WIRE_SIZE + size]) + message(handle, msg_type, as_bytes(payload), none(), true).map_err(nonblocking) +} + +/// [`signal`] without blocking. +pub fn try_signal(handle: RawHandle, msg_type: u32) -> Result<(), TrySendError> { + message(handle, msg_type, &[], none(), true).map_err(nonblocking) +} + +/// [`send_bytes`] without blocking. +pub fn try_send_bytes(handle: RawHandle, msg_type: u32, data: &[u8]) -> Result<(), TrySendError> { + message(handle, msg_type, data, none(), true).map_err(nonblocking) +} + +fn none() -> core::iter::Empty { + core::iter::empty() +} + +/// Why [`message`] sent nothing. +enum Unsent { + TooLarge, + Kernel(SyscallError), +} + +fn blocking(e: Unsent) -> IpcError { + match e { + Unsent::TooLarge => IpcError::TooLarge, + Unsent::Kernel(e) => IpcError::Syscall(e), + } +} + +fn nonblocking(e: Unsent) -> TrySendError { + match e { + Unsent::TooLarge => TrySendError::TooLarge, + Unsent::Kernel(SyscallError::WouldBlock) => TrySendError::Full, + Unsent::Kernel(e) => TrySendError::Syscall(e), + } +} + +/// The one way a frame leaves this endpoint: its header and `payload` in one +/// buffer, and `handles`, handed to the kernel as one message. Every handle is +/// consumed whatever the answer. +fn message( + handle: RawHandle, + msg_type: u32, + payload: &[u8], + handles: impl IntoIterator, + nonblock: bool, +) -> Result<(), Unsent> { + let mut too_large = false; + let sent = move_batch(handles, |batch| { + let Ok(header) = IpcHeader::frame(msg_type, payload.len(), batch.len()) else { + too_large = true; + return Err(SyscallError::InvalidArgument); + }; + let send = |frame: &mut [u8]| { + let end = IpcHeader::WIRE_SIZE + payload.len(); + frame[..IpcHeader::WIRE_SIZE].copy_from_slice(&header.to_wire()); + frame[IpcHeader::WIRE_SIZE..end].copy_from_slice(payload); + syscall::message_send(handle, &frame[..end], batch, nonblock) + }; + match payload.len() <= SMALL_PAYLOAD { + true => send(&mut [0u8; IpcHeader::WIRE_SIZE + SMALL_PAYLOAD]), + false => send(&mut [0u8; IpcHeader::WIRE_SIZE + MAX_FRAME_LEN as usize]), + } + }); + match sent { + Ok(()) => Ok(()), + Err(_) if too_large => Err(Unsent::TooLarge), + Err(e) => Err(Unsent::Kernel(e)), + } } /// Hand `handles` to `send` as one batch, consuming every one: what `send` @@ -351,30 +459,10 @@ fn move_batch( Ok(()) } -/// [`signal`] without blocking. A bare header always fits in one write. -pub fn try_signal(handle: RawHandle, msg_type: u32) -> Result<(), TrySendError> { - write_whole(handle, &IpcHeader { msg_type, len: 0 }.to_wire()) -} - -/// [`send_bytes`] without blocking. -/// -/// The frame buffer is [`MAX_FRAME_LEN`] on the stack, so this is for the -/// occasional large message — a clipboard paste — not for a per-frame path. -pub fn try_send_bytes(handle: RawHandle, msg_type: u32, data: &[u8]) -> Result<(), TrySendError> { - let mut frame = [0u8; IpcHeader::WIRE_SIZE + MAX_FRAME_LEN as usize]; - let header = IpcHeader::frame(msg_type, data.len()).map_err(|_| TrySendError::TooLarge)?; - frame[..IpcHeader::WIRE_SIZE].copy_from_slice(&header.to_wire()); - frame[IpcHeader::WIRE_SIZE..IpcHeader::WIRE_SIZE + data.len()].copy_from_slice(data); - write_whole(handle, &frame[..IpcHeader::WIRE_SIZE + data.len()]) -} - pub fn recv_header(handle: RawHandle) -> Result { let mut bytes = [0u8; IpcHeader::WIRE_SIZE]; read_exact(handle, &mut bytes)?; - IpcHeader::from_wire( - u32::from_ne_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]), - u32::from_ne_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]), - ) + IpcHeader::decode(&bytes) } /// The frame's payload as one `T`. A payload of any other length is read @@ -393,10 +481,16 @@ pub fn recv_payload(handle: RawHandle, header: &IpcHeader) -> Res Ok(unsafe { val.assume_init() }) } -/// Receive header + typed payload in one call. +/// Receive header + typed payload in one call, for a frame that carries no +/// handles: one that does is [`IpcError::Malformed`], its batch closed. pub fn recv(handle: RawHandle) -> Result<(u32, T), IpcError> { let header = recv_header(handle)?; let payload = recv_payload(handle, &header)?; + if header.handles() != 0 { + let mut stray = [toyos_abi::HANDLE_INVALID; syscall::MAX_TRANSFER_HANDLES]; + let _ = recv_handles(handle, &header, &mut stray).map(|n| close_all(&stray[..n])); + return Err(IpcError::Malformed); + } Ok((header.msg_type, payload)) } @@ -410,6 +504,32 @@ pub fn recv_bytes(handle: RawHandle, header: &IpcHeader, buf: &mut [u8]) -> Resu Ok(count) } +/// The batch `header` announced, taken where the kernel pinned it: exactly +/// [`IpcHeader::handles`] of them, or [`IpcError::Malformed`] with any that +/// arrived closed. +pub fn recv_handles( + handle: RawHandle, + header: &IpcHeader, + out: &mut [RawHandle; syscall::MAX_TRANSFER_HANDLES], +) -> Result { + let want = header.handles(); + if want == 0 { + return Ok(0); + } + let n = syscall::handle_recv(handle, out).map_err(IpcError::Syscall)?; + if n != want { + close_all(&out[..n]); + return Err(IpcError::Malformed); + } + Ok(n) +} + +fn close_all(handles: &[RawHandle]) { + for h in handles { + syscall::close(*h); + } +} + /// The memory twin of [`recv_payload`], for a reader that already holds the /// bytes. /// @@ -439,13 +559,15 @@ fn exactly(len: usize) -> Result<(), IpcError> { pub enum RxStep { /// The handle has nothing more right now. Idle, - /// A whole frame is buffered; its payload is [`FrameRx::payload`]. - Frame { msg_type: u32, payload_len: usize }, + /// A whole frame is buffered; its payload is [`FrameRx::payload`] and its + /// `handles` are [`FrameRx::take_handles`]'s. + Frame { msg_type: u32, payload_len: usize, handles: usize }, /// The peer hung up, or its handle is gone. Eof, - /// A frame no protocol here can produce: a length past [`MAX_FRAME_LEN`] - /// or past the receiver's `MAX`. Nothing after it can be located, so - /// there is nothing to resynchronise to. + /// The peer broke framing: a length past [`MAX_FRAME_LEN`] or the + /// receiver's `MAX`, a count past the bound, a batch other than its frame + /// announced, or one no frame announced. Nothing after it can be located, + /// so there is nothing to resynchronise to. Malformed, } @@ -461,16 +583,22 @@ pub enum RxStep { /// declaring more is [`RxStep::Malformed`] at its header, so a peer cannot /// make the server hold what it will not look at, and no frame arrives cut /// short. A protocol whose messages are all bare headers takes `MAX = 0`. +/// +/// **A frame's handles come with it.** They are taken when the frame is, and +/// what the caller does not take with [`take_handles`](Self::take_handles) is +/// closed before the next frame is read or when this is dropped. pub struct FrameRx { header: [u8; IpcHeader::WIRE_SIZE], buf: [u8; MAX], len: usize, state: RxState, + held: [RawHandle; syscall::MAX_TRANSFER_HANDLES], + held_len: usize, } enum RxState { Header, - Payload { msg_type: u32, len: usize }, + Payload { msg_type: u32, len: usize, handles: usize }, } impl Default for FrameRx { @@ -479,6 +607,12 @@ impl Default for FrameRx { } } +impl Drop for FrameRx { + fn drop(&mut self) { + self.release(); + } +} + impl FrameRx { pub const fn new() -> Self { Self { @@ -486,6 +620,8 @@ impl FrameRx { buf: [0; MAX], len: 0, state: RxState::Header, + held: [toyos_abi::HANDLE_INVALID; syscall::MAX_TRANSFER_HANDLES], + held_len: 0, } } @@ -494,47 +630,107 @@ impl FrameRx { &self.buf[..len] } + /// The handles the last frame carried, now the caller's to claim. + pub fn take_batch(&mut self) -> Batch { + let batch = Batch { handles: self.held, len: self.held_len }; + self.held_len = 0; + batch + } + + /// The `N` handles the last frame carried, now the caller's; `None`, with + /// them closed, when it carried another number. + pub fn take_handles(&mut self) -> Option<[RawHandle; N]> { + self.take_batch().exact::() + } + + fn release(&mut self) { + close_all(&self.held[..self.held_len]); + self.held_len = 0; + } + /// Read what is there, and report the first thing that happened. /// /// At most one frame per call: the caller has other peers to serve, and a /// peer that always has another frame ready must not be able to keep the /// rest from being served. pub fn pump(&mut self, conn: &Connection) -> RxStep { + self.release(); loop { match self.state { RxState::Header => { match fill(conn, &mut self.header, &mut self.len) { Fill::Idle => return RxStep::Idle, Fill::Eof => return RxStep::Eof, + Fill::Malformed => return RxStep::Malformed, Fill::Filled => {} } - let Some((msg_type, len)) = Self::announced(&self.header) else { + let Some((msg_type, len, handles)) = Self::announced(&self.header) else { return RxStep::Malformed; }; self.len = 0; - self.state = RxState::Payload { msg_type, len }; + self.state = RxState::Payload { msg_type, len, handles }; } - RxState::Payload { msg_type, len } => { + RxState::Payload { msg_type, len, handles } => { match fill(conn, &mut self.buf[..len], &mut self.len) { Fill::Idle => return RxStep::Idle, Fill::Eof => return RxStep::Eof, + Fill::Malformed => return RxStep::Malformed, Fill::Filled => {} } self.len = 0; self.state = RxState::Header; - return RxStep::Frame { msg_type, payload_len: len }; + if handles > 0 { + // Pinned just past the bytes now read, so it is there or never sent. + let n = syscall::handle_recv(conn.as_handle(), &mut self.held).unwrap_or(0); + self.held_len = n; + if n != handles { + return RxStep::Malformed; + } + } + return RxStep::Frame { msg_type, payload_len: len, handles }; } } } } - /// The message type and payload length `header` announces, or `None` for - /// a frame this receiver cannot hold whole. - fn announced(header: &[u8; IpcHeader::WIRE_SIZE]) -> Option<(u32, usize)> { - let msg_type = u32::from_ne_bytes(header[0..4].try_into().unwrap()); - let wire_len = u32::from_ne_bytes(header[4..8].try_into().unwrap()); - let len = IpcHeader::from_wire(msg_type, wire_len).ok()?.len() as usize; - (len <= MAX).then_some((msg_type, len)) + /// The message type, payload length and handle count `header` announces, + /// or `None` for a frame this receiver cannot hold whole. + fn announced(header: &[u8; IpcHeader::WIRE_SIZE]) -> Option<(u32, usize, usize)> { + let header = IpcHeader::decode(header).ok()?; + let len = header.len() as usize; + (len <= MAX).then_some((header.msg_type, len, header.handles())) + } +} + +/// The handles one frame carried, owned: what nobody claims is closed when +/// this drops. +pub struct Batch { + handles: [RawHandle; syscall::MAX_TRANSFER_HANDLES], + len: usize, +} + +impl Batch { + /// Exactly `N` handles, now the caller's; `None`, with them closed, for + /// any other number. + pub fn exact(mut self) -> Option<[RawHandle; N]> { + const { assert!(N <= syscall::MAX_TRANSFER_HANDLES) }; + if self.len != N { + return None; + } + self.len = 0; + Some(core::array::from_fn(|i| self.handles[i])) + } + + /// Every handle, as numbers now the caller's to own and close. + pub fn into_raw(mut self) -> ([RawHandle; syscall::MAX_TRANSFER_HANDLES], usize) { + let len = core::mem::take(&mut self.len); + (self.handles, len) + } +} + +impl Drop for Batch { + fn drop(&mut self) { + close_all(&self.handles[..self.len]); } } @@ -542,6 +738,8 @@ enum Fill { Filled, Idle, Eof, + /// A batch no frame announced sits where the next byte would be. + Malformed, } /// Fill `buf` from `*len` onwards without blocking, carrying `*len` forward so @@ -552,6 +750,7 @@ fn fill(conn: &Connection, buf: &mut [u8], len: &mut usize) -> Fill { Ok(0) => return Fill::Eof, Ok(n) => *len += n, Err(SyscallError::WouldBlock) => return Fill::Idle, + Err(SyscallError::HandlesPending) => return Fill::Malformed, // The handle itself is gone, which is the same hang-up seen from the // other side of the same race. Err(_) => return Fill::Eof, @@ -574,10 +773,15 @@ fn skip(handle: RawHandle, mut remaining: usize) -> Result<(), IpcError> { Ok(()) } +/// Fill `buf` with blocking reads. A batch no frame announced where the next +/// byte would be is the peer breaking framing. fn read_exact(handle: RawHandle, buf: &mut [u8]) -> Result<(), IpcError> { let mut offset = 0; while offset < buf.len() { - let n = syscall::read(handle, &mut buf[offset..]).map_err(IpcError::Syscall)?; + let n = syscall::read(handle, &mut buf[offset..]).map_err(|e| match e { + SyscallError::HandlesPending => IpcError::Malformed, + e => IpcError::Syscall(e), + })?; if n == 0 { return Err(IpcError::Disconnected); } @@ -586,28 +790,6 @@ fn read_exact(handle: RawHandle, buf: &mut [u8]) -> Result<(), IpcError> { Ok(()) } -/// All of `buf` in one non-blocking write, or [`TrySendError::Full`]. -/// -/// A short write is `Full`, not a partial success: the caller asked for a -/// frame, and half a frame is a stream the peer can no longer parse. -fn write_whole(handle: RawHandle, buf: &[u8]) -> Result<(), TrySendError> { - match syscall::write_nonblock(handle, buf) { - Ok(n) if n == buf.len() => Ok(()), - Ok(_) => Err(TrySendError::Full), - Err(SyscallError::WouldBlock) => Err(TrySendError::Full), - Err(e) => Err(TrySendError::Syscall(e)), - } -} - -fn write_all(handle: RawHandle, buf: &[u8]) -> Result<(), IpcError> { - let mut offset = 0; - while offset < buf.len() { - let n = syscall::write(handle, &buf[offset..]).map_err(IpcError::Syscall)?; - offset += n; - } - Ok(()) -} - #[cfg(test)] mod tests { use std::cell::RefCell; @@ -647,10 +829,11 @@ mod tests { assert!(matches!(decode_payload::<[u32; 2]>(&[]), Err(IpcError::Malformed))); } - fn header(len: u32) -> [u8; IpcHeader::WIRE_SIZE] { + fn header(len: u32, handles: u32) -> [u8; IpcHeader::WIRE_SIZE] { let mut bytes = [0u8; IpcHeader::WIRE_SIZE]; bytes[..4].copy_from_slice(&7u32.to_ne_bytes()); - bytes[4..].copy_from_slice(&len.to_ne_bytes()); + bytes[4..8].copy_from_slice(&len.to_ne_bytes()); + bytes[8..].copy_from_slice(&handles.to_ne_bytes()); bytes } @@ -658,11 +841,24 @@ mod tests { /// header past it, never cut down to what fits. #[test] fn a_frame_past_the_receivers_max_is_refused_at_its_header() { - assert_eq!(FrameRx::<8>::announced(&header(0)), Some((7, 0))); - assert_eq!(FrameRx::<8>::announced(&header(8)), Some((7, 8))); - assert_eq!(FrameRx::<8>::announced(&header(9)), None); - assert_eq!(FrameRx::<0>::announced(&header(1)), None); - assert_eq!(FrameRx::<{ MAX_FRAME_LEN as usize + 1 }>::announced(&header(MAX_FRAME_LEN + 1)), None); + assert_eq!(FrameRx::<8>::announced(&header(0, 0)), Some((7, 0, 0))); + assert_eq!(FrameRx::<8>::announced(&header(8, 0)), Some((7, 8, 0))); + assert_eq!(FrameRx::<8>::announced(&header(9, 0)), None); + assert_eq!(FrameRx::<0>::announced(&header(1, 0)), None); + assert_eq!(FrameRx::<{ MAX_FRAME_LEN as usize + 1 }>::announced(&header(MAX_FRAME_LEN + 1, 0)), None); + } + + /// A frame announces at most the batch one message can carry: one more + /// is refused at its header, as the kernel would refuse the send. + #[test] + fn a_frame_announcing_more_handles_than_a_message_carries_is_refused() { + let most = syscall::MAX_TRANSFER_HANDLES as u32; + assert_eq!(FrameRx::<8>::announced(&header(4, most)), Some((7, 4, most as usize))); + assert_eq!(FrameRx::<8>::announced(&header(4, most + 1)), None); + assert!(matches!(IpcHeader::frame(7, 0, most as usize + 1), Err(IpcError::TooLarge))); + let built = IpcHeader::frame(7, 3, 2).unwrap().to_wire(); + let back = IpcHeader::decode(&built).unwrap(); + assert_eq!((back.msg_type, back.len(), back.handles()), (7, 3, 2)); } /// The kernel's refusal leaves every handle at its own number, and the diff --git a/sdk/toyos/src/launch.rs b/sdk/toyos/src/launch.rs index d614941831c..5eaaa0b9b2c 100644 --- a/sdk/toyos/src/launch.rs +++ b/sdk/toyos/src/launch.rs @@ -317,10 +317,10 @@ impl<'a> Request<'a> { /// Why a launch did not answer, and — the part that matters — whether the /// handles it was going to carry are still the caller's. /// -/// **A send consumes them.** `SYS_HANDLE_SEND` takes them out of the sender's -/// table, and a refused one closes them ([`Connection::send_handles`]), so a -/// caller that closed them after either would be closing handles it no longer -/// holds — which, under the bad-handle policy, is the caller exiting. Which +/// **A send consumes them.** `SYS_MESSAGE_SEND` takes them out of the sender's +/// table, and a refused one closes them ([`Connection::send_bytes_with`]), so +/// a caller that closed them after either would be closing handles it no +/// longer holds — which, under the bad-handle policy, is the caller exiting. Which /// side owns them is therefore not a detail of the error but the whole of what /// the caller needs from it. pub enum LaunchError { @@ -346,9 +346,8 @@ pub enum Outcome<'a> { /// Send one launch and read its answer, a `HOME` in `answer`. /// -/// The handles go before the frame that announces them, which is -/// [`Connection::send_handles`]'s whole rule — and the `Process` handle comes -/// back the same way. A `HOME` longer than `answer` or not UTF-8 is a +/// The handles travel in the launch frame itself, and the `Process` handle +/// comes back in the answer's. A `HOME` longer than `answer` or not UTF-8 is a /// malformed answer, never a shortened one. pub fn launch<'a>( conn: &Connection, @@ -362,18 +361,17 @@ pub fn launch<'a>( // Each is the caller's, which is `Launch`'s contract, and `batch` named it once. let owned = |(handles, count): Batch| (0..count).map(move |i| OwnedHandle(handles[i])); let sent = match request.batch(conn.as_handle()) { - Ok(batch) => conn.send_handles(owned(batch)), + Ok(batch) => conn.send_bytes_with(MSG_LAUNCH, &buf[..len], owned(batch)), // Refused here as the kernel would refuse it, and closed as any refused move is. Err(named) => { owned(named).for_each(drop); - Err(SyscallError::InvalidArgument) + Err(IpcError::Syscall(SyscallError::InvalidArgument)) } }; - sent.map_err(|e| LaunchError::Sent(IpcError::Syscall(e)))?; - conn.send_bytes(MSG_LAUNCH, &buf[..len]).map_err(LaunchError::Sent)?; + sent.map_err(LaunchError::Sent)?; let header = conn.recv_header().map_err(LaunchError::Sent)?; match header.msg_type { - MSG_LAUNCHED => match conn.recv_handles_exact::<1>() { + MSG_LAUNCHED => match conn.recv_handles_exact::<1>(&header) { Some([process]) => Ok(Outcome::Started(process)), None => Err(LaunchError::Sent(IpcError::Malformed)), }, diff --git a/sdk/toyos/src/lib.rs b/sdk/toyos/src/lib.rs index 3166a3a3d0b..109dcd52e26 100644 --- a/sdk/toyos/src/lib.rs +++ b/sdk/toyos/src/lib.rs @@ -80,7 +80,7 @@ pub fn null(rights: toyos_abi::handle::Rights) -> Result( self, handles: impl IntoIterator, @@ -333,8 +333,7 @@ impl NetstackConn { payload: &Req, ) -> Result { const { assert!(core::mem::size_of::() <= MAX_REQUEST) }; - self.0.send_handles(handles).map_err(|e| hangup(IpcError::Syscall(e)))?; - self.0.send(msg_type as u32, payload).map_err(hangup)?; + self.0.send_with(msg_type as u32, payload, handles).map_err(hangup)?; Ok(PendingResponse(self)) } @@ -357,21 +356,20 @@ impl NetstackConn { /// rather than exiting with the line it has for exactly this. /// /// **`Disconnected` is only the word for a hang-up this endpoint *read*, and a -/// request writes twice before it reads at all.** `IpcError::Disconnected` is +/// request is sent before anything is read.** `IpcError::Disconnected` is /// raised in one place — `ipc::read_exact`, on a `read` that answered zero — so /// it is what a peer that left while this endpoint was waiting for the response /// looks like. A peer that left *before* the request went out is refused by the -/// kernel at one of the two writes instead, and both answer `Gone` on a -/// connection whose handle is still live and still this process's: when the -/// server end's last handle goes, `port::Acceptor::on_zero_handles` closes -/// every queued connection's inbox — this end's *outbox*, so `HandleQueue::push` -/// refuses `SYS_HANDLE_SEND` rather than filling a queue nobody will drain — -/// and drops the connection, which drops the server's read end, so `SYS_WRITE` -/// is a pipe with no readers. +/// kernel at the send instead, which answers `Gone` on a connection whose +/// handle is still live and still this process's: when the server end's last +/// handle goes, `port::Acceptor::on_zero_handles` closes every queued +/// connection's inbox — this end's *outbox*, so `SYS_MESSAGE_SEND` refuses a +/// message rather than filling a queue nobody will drain — and drops the +/// connection, which drops the server's read end, so the stream has no reader. /// -/// `Gone` cannot mean anything else here. This is applied only to `read`, -/// `write` and `handle_send` on a connection this process holds, and a handle a -/// process does not hold ends it at the kernel rather than answering a word. +/// `Gone` cannot mean anything else here. This is applied only to `read` and +/// `message_send` on a connection this process holds, and a handle a process +/// does not hold ends it at the kernel rather than answering a word. fn hangup(e: IpcError) -> NetError { match e { IpcError::Disconnected @@ -412,7 +410,7 @@ impl PendingResponse { pub fn answer_with_handle(self) -> Result<(Resp, RawHandle, Connection), NetError> { let header = self.recv_checked_header()?; let answer = self.conn().recv_payload(&header).map_err(hangup)?; - let [handle] = self.conn().recv_handles_exact::<1>().ok_or(NetError::Protocol(header.msg_type))?; + let [handle] = self.conn().recv_handles_exact::<1>(&header).ok_or(NetError::Protocol(header.msg_type))?; Ok((answer, handle, (self.0).0)) } @@ -574,16 +572,14 @@ mod tests { use super::*; use toyos_abi::syscall::SyscallError; - /// `SYS_HANDLE_SEND` into a connection whose server end has gone. The first - /// refusal a `tcp_bind` can meet, because a request that hands netstack pipe - /// ends moves the handles before it writes the frame. + /// `SYS_MESSAGE_SEND` into a connection whose server end has gone: the + /// first refusal any request can meet, a `tcp_bind`'s included. #[test] fn a_gone_handle_transfer_is_a_netstack_that_is_not_there() { assert_eq!(hangup(IpcError::Syscall(SyscallError::Gone)), NetError::NetstackNotFound); } - /// `SYS_WRITE` into the same connection a moment later reaches the same - /// word, so `NotFound` is not this fact and no longer reaches it. + /// `NotFound` is not this fact and does not reach it. #[test] fn a_not_found_is_not_a_netstack_that_is_not_there() { assert_eq!(hangup(IpcError::Syscall(SyscallError::NotFound)), NetError::Io); diff --git a/sdk/toyos/src/shm.rs b/sdk/toyos/src/shm.rs index f9edddaa6c5..8a69f130834 100644 --- a/sdk/toyos/src/shm.rs +++ b/sdk/toyos/src/shm.rs @@ -3,7 +3,7 @@ //! A region is a **handle**, not a token. Holding one is the whole of being //! allowed to map it, so there is no grant and no list of pids to keep: giving //! a peer access is [`SharedMemory::share`] plus -//! [`Connection::send_handles`](crate::ipc::Connection::send_handles), and a +//! a frame that carries it ([`Connection::send_with`](crate::ipc::Connection::send_with)), and a //! peer that never receives it can never name it. //! //! The mapping goes away with the last handle, so dropping this is all the diff --git a/sdk/window/src/lib.rs b/sdk/window/src/lib.rs index d1dbe352c63..d70c68f27b3 100644 --- a/sdk/window/src/lib.rs +++ b/sdk/window/src/lib.rs @@ -86,7 +86,7 @@ pub const MSG_COPY_BEGIN: u32 = 13; /// copies it once and closes the connection. pub const MSG_COPY_COMMIT: u32 = 14; /// Compositor → client, bare: a region the compositor made, as long as the copy -/// [`MSG_COPY_BEGIN`] announced, sent ahead of the frame. +/// [`MSG_COPY_BEGIN`] announced, carried in the frame. pub const MSG_COPY_REGION: u32 = 13; /// Watcher → compositor, bare: the first frame on the clipboard port, and the /// ask for the next copy ([`watcher`]). Answered, once a client has copied, @@ -249,7 +249,7 @@ toyos::ipc_payload! { } /// How many bytes of clipboard text a copy region holds. Where a region - /// travels with the message, it is one handle sent ahead of the frame. + /// travels with the message, it is one handle carried in the frame. pub struct ClipboardShmMsg { pub len: u32, } @@ -291,7 +291,7 @@ toyos::ipc_payload! { /// The shape of the window's buffer, in physical pixels, the scale to /// draw into it at, as [`Scale::wire`], and its generation, which every /// present drawn into it carries. **The buffer is one transferred - /// handle**, sent ahead of the frame that carries this. + /// handle**, carried in the frame that carries this. pub struct WindowInfo { pub width: u32, pub height: u32, @@ -525,7 +525,7 @@ fn copy(bytes: &[u8]) -> Result<(), CreateError> { if header.msg_type != MSG_COPY_REGION || !header.is_empty() { return Err(CreateError::Protocol(header.msg_type)); } - let [region] = conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let [region] = conn.recv_handles_exact::<1>(&header).ok_or(CreateError::NoHandle)?; let mut region = SharedMemory::lent(region, bytes.len()).map_err(CreateError::Lent)?; region.as_mut_slice().copy_from_slice(bytes); Ok(conn.signal(MSG_COPY_COMMIT)?) @@ -607,10 +607,10 @@ impl Window { let scale = Scale::from_wire(info.scale).ok_or(CreateError::Protocol(MSG_WINDOW_CREATED))?; let buf_size = info.stride as usize * info.height as usize * 4; - // The buffer crossed ahead of the frame. A compositor that announced a + // The buffer travels in the frame. A compositor that announced a // window and sent nothing with it is not serving this client, whatever // else it is doing. - let [buffer] = conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let [buffer] = conn.recv_handles_exact::<1>(&header).ok_or(CreateError::NoHandle)?; let shm = SharedMemory::lent(buffer, buf_size).map_err(CreateError::Lent)?; let poller = Poller::new(1); @@ -718,7 +718,7 @@ impl Window { // The compositor shows the old buffer only until this // window's next present, which it reads from the new one, so a // window that cannot map the new one has nothing to present. - let Some([buffer]) = self.conn.recv_handles_exact::<1>() else { + let Some([buffer]) = self.conn.recv_handles_exact::<1>(header) else { return Event::Close; }; let Ok(shm) = SharedMemory::lent(buffer, buf_size) else { @@ -743,7 +743,7 @@ impl Window { let Ok(info) = self.conn.recv_payload::(header) else { return Event::Close; }; - let Some([buffer]) = self.conn.recv_handles_exact::<1>() else { + let Some([buffer]) = self.conn.recv_handles_exact::<1>(header) else { return Event::ClipboardPaste(Vec::new()); }; let Ok(shm) = SharedMemory::lent(buffer, info.len as usize) else { diff --git a/sdk/window/src/watcher.rs b/sdk/window/src/watcher.rs index c6184ff4c8c..717f6b540a4 100644 --- a/sdk/window/src/watcher.rs +++ b/sdk/window/src/watcher.rs @@ -113,7 +113,7 @@ impl Watcher { RxStep::Idle => return Ok(Watched::Idle), RxStep::Eof => return Err(CreateError::CompositorGone.into()), RxStep::Malformed => return Err(CreateError::BrokenOff.into()), - RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), }; let payload = self.rx.payload(len); match msg_type { @@ -129,7 +129,7 @@ impl Watcher { if !copy_fits(len) { return Err(WatchError::TooLong(len)); } - let [region] = self.conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let [region] = self.rx.take_handles::<1>().ok_or(CreateError::NoHandle)?; let region = SharedMemory::lent(region, len).map_err(CreateError::Lent)?; let text = String::from_utf8(region.as_slice().to_vec()).map_err(|_| WatchError::NotText)?; self.conn.try_signal(MSG_CLIPBOARD_WATCH)?; @@ -137,7 +137,7 @@ impl Watcher { } MSG_COPY_REGION if len == 0 => { let text = self.region_owed.take().ok_or(CreateError::Protocol(msg_type))?; - let [region] = self.conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let [region] = self.rx.take_handles::<1>().ok_or(CreateError::NoHandle)?; let mut region = SharedMemory::lent(region, text.len()).map_err(CreateError::Lent)?; region.as_mut_slice().copy_from_slice(text.as_bytes()); self.conn.try_signal(MSG_COPY_COMMIT)?; diff --git a/system/compositor/src/client.rs b/system/compositor/src/client.rs index 273150c769d..a7e50960a9f 100644 --- a/system/compositor/src/client.rs +++ b/system/compositor/src/client.rs @@ -294,9 +294,8 @@ pub fn note_opened(client: RawHandle, content: toyos_desktop::Rect, live: usize) /// Hand a window a typed frame, or mark it for removal. /// -/// A failure is never retried and never ignored: `TrySendError::Full` can have -/// left part of the frame in the pipe, so the peer's stream is past saving — -/// which is the price of never blocking on it. +/// A failure is never retried and never ignored: the window is dropped, which +/// is the price of never blocking on a client. pub fn deliver(dead: &mut Vec, win: &Win, msg_type: u32, payload: &T) { if let Err(e) = win.client.conn.try_send(msg_type, payload) { mark_dead(dead, win.client.conn.as_handle(), e.into()); @@ -312,9 +311,8 @@ pub fn deliver_with_handle( msg_type: u32, payload: &T, ) { - match win.client.conn.send_handles([handle]) { - Ok(()) => deliver(dead, win, msg_type, payload), - Err(e) => mark_dead(dead, win.client.conn.as_handle(), e.into()), + if let Err(e) = win.client.conn.try_send_with(msg_type, payload, [handle]) { + mark_dead(dead, win.client.conn.as_handle(), e.into()); } } diff --git a/system/compositor/src/session.rs b/system/compositor/src/session.rs index fc5865c5862..03206851fb5 100644 --- a/system/compositor/src/session.rs +++ b/system/compositor/src/session.rs @@ -626,7 +626,7 @@ impl Session { let Some((conn, rx)) = self.volume.as_mut() else { return }; let shown = match rx.pump(conn) { RxStep::Idle => return, - RxStep::Frame { msg_type: MSG_VOLUME, payload_len } => { + RxStep::Frame { msg_type: MSG_VOLUME, payload_len, .. } => { match ipc::decode_payload::(rx.payload(payload_len)) { Ok(v) if v.muted != 0 => Shown::Muted, Ok(v) => Shown::Level { position: v.position, bottom: v.bottom }, @@ -1031,7 +1031,7 @@ impl Session { ended = Some("a frame no protocol here can make".to_string()); break; } - RxStep::Frame { msg_type, payload_len } => wire::decode(msg_type, source.rx.payload(payload_len)), + RxStep::Frame { msg_type, payload_len, .. } => wire::decode(msg_type, source.rx.payload(payload_len)), }; match record { Ok(Record::Key { source, key }) => { @@ -1142,7 +1142,7 @@ impl Session { RxStep::Malformed => { mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol) } - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let mut frame = ClientFrame::new(handle, msg_type); frame.set_payload(self.pending[i].rx.payload(payload_len)); // A connection is identified by its first frame and by @@ -1177,7 +1177,7 @@ impl Session { RxStep::Malformed => { mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol) } - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let mut frame = ClientFrame::new(handle, msg_type); frame.set_payload(win.client.rx.payload(payload_len)); out.push(frame); @@ -1501,11 +1501,7 @@ impl Session { return; } }; - if let Err(e) = conn.send_handles([theirs]) { - mark_dead(&mut self.dead, handle, e.into()); - return; - } - if let Err(e) = conn.try_signal(window::MSG_COPY_REGION) { + if let Err(e) = conn.try_signal_with(window::MSG_COPY_REGION, [theirs]) { mark_dead(&mut self.dead, handle, e.into()); return; } @@ -1561,7 +1557,7 @@ impl Session { RxStep::Idle => return, RxStep::Eof => return mark_dead(&mut self.dead, handle, DropReason::Gone), RxStep::Malformed => return mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol), - RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), }; let payload = w.rx.payload(len).to_vec(); match msg_type { @@ -1627,10 +1623,7 @@ impl Session { return mark_dead(&mut self.dead, from, DropReason::OutOfProtocol); } let w = self.watcher.as_ref().expect("a region is opened only off the watcher"); - if let Err(e) = w.conn.send_handles([theirs]) { - return mark_dead(&mut self.dead, from, e.into()); - } - if let Err(e) = w.conn.try_signal(window::MSG_COPY_REGION) { + if let Err(e) = w.conn.try_signal_with(window::MSG_COPY_REGION, [theirs]) { mark_dead(&mut self.dead, from, e.into()); } } @@ -1650,13 +1643,8 @@ impl Session { }) { Ok(theirs) => w .conn - .send_handles([theirs]) - .map_err(DropReason::from) - .and_then(|()| { - w.conn - .try_send(window::MSG_CLIPBOARD_PASTE_SHM, &window::ClipboardShmMsg { len: len as u32 }) - .map_err(DropReason::from) - }), + .try_send_with(window::MSG_CLIPBOARD_PASTE_SHM, &window::ClipboardShmMsg { len: len as u32 }, [theirs]) + .map_err(DropReason::from), Err(e) => Err(DropReason::Refused(e)), } }; diff --git a/system/console/src/main.rs b/system/console/src/main.rs index 1c796d91ede..531b0dce8f7 100644 --- a/system/console/src/main.rs +++ b/system/console/src/main.rs @@ -364,7 +364,7 @@ fn main() { ended = Some("a frame no protocol here can make".to_string()); break; } - RxStep::Frame { msg_type, payload_len } => wire::decode(msg_type, rx.payload(payload_len)), + RxStep::Frame { msg_type, payload_len, .. } => wire::decode(msg_type, rx.payload(payload_len)), }; match record { Ok(Record::Key { source, key }) => match merge.key(slot, source, key) { diff --git a/system/diskserver/src/main.rs b/system/diskserver/src/main.rs index 63dc04f70ef..323b7859fbc 100644 --- a/system/diskserver/src/main.rs +++ b/system/diskserver/src/main.rs @@ -331,7 +331,7 @@ impl Service { RxStep::Eof | RxStep::Malformed => { pending.remove(i); } - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let p = pending.remove(i); self.asked(clients, p, msg_type, payload_len); } @@ -355,7 +355,7 @@ impl Service { RxStep::Idle => return, RxStep::Eof => return self.drop_driver(id, "its connection ended"), RxStep::Malformed => return self.drop_driver(id, "a frame this protocol does not make"), - RxStep::Frame { msg_type, payload_len } => (msg_type, d.rx.payload(payload_len).to_vec()), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, d.rx.payload(payload_len).to_vec()), }; if let Err(why) = self.heard(id, msg_type, &payload) { return self.drop_driver(id, why); @@ -446,13 +446,13 @@ impl Service { } /// One client's first frame: a listing, or an open. - fn asked(&mut self, clients: &toyos::port::Acceptor, p: Pending, msg_type: u32, payload_len: usize) { + fn asked(&mut self, clients: &toyos::port::Acceptor, mut p: Pending, msg_type: u32, payload_len: usize) { let grant = grant(clients, &p.conn); if msg_type == wire::MSG_LIST && payload_len == 0 { return self.waiting.push(Waiting { conn: p.conn, grant, ask: Ask::List }); } let guid = wire::guid(p.rx.payload(payload_len)); - let handles = p.conn.recv_handles_exact::<1>(); + let handles = p.rx.take_handles::<1>(); let (Some(guid), Some([region]), wire::MSG_OPEN) = (guid, handles, msg_type) else { if let Some([region]) = handles { toyos_abi::syscall::close(region); @@ -506,8 +506,8 @@ impl Service { ); } - /// The session goes to the driver whose disk `offered` is: the client's - /// connection and region, then the frame naming them. + /// The session goes to the driver whose disk `offered` is: the frame + /// naming it, carrying the client's connection and region. fn hand_over(&mut self, offered: Offered, guid: [u8; 16], writes: bool, conn: Connection, region: OwnedHandle) { let Some((&id, d)) = self.drivers.iter().find(|(_, d)| d.disks.values().any(|&ours| ours == offered.disk)) else { unreachable!("diskserver: disk {} is offered and no driver has it", offered.disk) @@ -515,14 +515,11 @@ impl Service { let theirs = *d.disks.iter().find(|(_, ours)| **ours == offered.disk).expect("just found").0; let session = disk::Session { disk: theirs, unique: guid, writes }; // The client hears nothing from here: its open is the driver's to - // answer, and a refused move closes the client's connection. - if let Err(why) = d.conn.send_handles([conn.into_owned(), region]) { - return println!("diskserver: the kernel would not move a session to disk driver {}: {why:?}", d.badge); - } - // Its handles are in the driver's queue ahead of any later frame's, - // so a frame the driver will not take ends the driver. - if d.conn.try_send_bytes(disk::MSG_SESSION, &session.encode()).is_err() { - return self.drop_driver(id, "it would not take a session"); + // answer, and a refused send closes the client's connection. + match d.conn.try_send_bytes_with(disk::MSG_SESSION, &session.encode(), [conn.into_owned(), region]) { + Ok(()) => {} + Err(ipc::TrySendError::Full) => self.drop_driver(id, "it would not take a session"), + Err(why) => println!("diskserver: the kernel would not move a session to disk driver {}: {why:?}", d.badge), } } } diff --git a/system/diskserver/src/session.rs b/system/diskserver/src/session.rs index ea30d0b016c..4c2c6845440 100644 --- a/system/diskserver/src/session.rs +++ b/system/diskserver/src/session.rs @@ -462,8 +462,7 @@ fn handshake( region: &Region, ) -> Result<(Connection, Opened), Error> { let conn = connect(names, service)?; - conn.send_handles([region.share().map_err(Error::Kernel)?]).map_err(|_| Error::Ended)?; - conn.send_bytes(wire::MSG_OPEN, &guid).map_err(|_| Error::Ended)?; + conn.send_bytes_with(wire::MSG_OPEN, &guid, [region.share().map_err(Error::Kernel)?]).map_err(|_| Error::Ended)?; let header = answer(&conn)?; let mut payload = [0u8; Opened::BYTES]; let len = conn.recv_bytes(&header, &mut payload).map_err(|_| Error::Ended)?; diff --git a/system/filepicker/src/main.rs b/system/filepicker/src/main.rs index 6db1bd7adc9..4216ad2d07e 100644 --- a/system/filepicker/src/main.rs +++ b/system/filepicker/src/main.rs @@ -631,7 +631,7 @@ fn main() { ); pending.remove(i); } - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let p = pending.remove(i); if p.consent { match consent::question(p.rx.payload(payload_len), msg_type) { diff --git a/system/fileserver/src/main.rs b/system/fileserver/src/main.rs index 27847c74d22..3b3fc5a5440 100644 --- a/system/fileserver/src/main.rs +++ b/system/fileserver/src/main.rs @@ -51,7 +51,7 @@ use fileserver::volume::{Kind, Meta, Node, OpenHow, Out, Volume}; use fileserver::writeback::WriteBack; use toyos::endow::{self, Endowments}; use toyos::fs::*; -use toyos::ipc::{self, Connection, RxStep, TrySendError}; +use toyos::ipc::{self, Connection, RxStep}; use toyos::poller::{Poller, READABLE}; use toyos::port::Acceptor; use toyos::shm::SharedMemory; @@ -475,7 +475,7 @@ impl Server { RxStep::Idle => {} RxStep::Eof => self.drop_client(id, ""), RxStep::Malformed => self.drop_client(id, "it sent a frame this protocol cannot describe"), - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let Ok(request) = ipc::decode_payload::(client.rx.payload(payload_len)) else { return self.drop_client(id, "its request was not one `Request` wide"); }; @@ -491,11 +491,7 @@ impl Server { let sent = match answer { Answer::Reply(reply) => client.conn.try_send(REPLY, &reply), Answer::Link(len) => client.conn.try_send(LINK, &Reply { value: len as u64, ..Reply::ok() }), - Answer::WithHandle(reply, handle) => client - .conn - .send_handles([handle]) - .map_err(TrySendError::Syscall) - .and_then(|()| client.conn.try_send(REPLY, &reply)), + Answer::WithHandle(reply, handle) => client.conn.try_send_with(REPLY, &reply, [handle]), Answer::Drop(why) => return self.drop_client(id, why), Answer::Refuse(e, why) => { // Let go whether or not the refusal went: either way it is said. @@ -550,7 +546,8 @@ impl Server { if client.window.is_some() { return Answer::Drop("it lent a second window"); } - let lent = || client.conn.recv_handles_exact::<1>().map(|[lent]| SharedMemory::lent(lent, WINDOW_BYTES)); + // The window travels in the HELLO frame: its reader took it with the frame. + let lent = || client.rx.take_handles::<1>().map(|[lent]| SharedMemory::lent(lent, WINDOW_BYTES)); match bounds::hello(all, mine, lent) { Hello::Served(window) => client.window = Some(window), Hello::Refused(why) => return Answer::Refuse(Error::TooManyConnections, why), diff --git a/system/hda/src/main.rs b/system/hda/src/main.rs index 173d0d2ec78..82451eb89fc 100644 --- a/system/hda/src/main.rs +++ b/system/hda/src/main.rs @@ -454,7 +454,7 @@ fn offered(control: &Connection, offer: &Offer) -> Result match ToDriver::decode(header.msg_type, &payload[..n]) { Some(ToDriver::Accepted) => { let [ring] = control - .recv_handles_exact::<{ output::ACCEPTED_HANDLES }>() + .recv_handles_exact::<{ output::ACCEPTED_HANDLES }>(&header) .ok_or_else(|| refused("its acceptance carried no ring".into()))?; // SAFETY: the kernel moved this handle into this table with the // frame that names it, and nothing else answers for it. @@ -469,7 +469,7 @@ fn offered(control: &Connection, offer: &Offer) -> Result fn frame(rx: &mut FrameRx, conn: &Connection, at: &str, which: &str) -> Option<(u32, Vec)> { match rx.pump(conn) { RxStep::Idle => None, - RxStep::Frame { msg_type, payload_len } => Some((msg_type, rx.payload(payload_len).to_vec())), + RxStep::Frame { msg_type, payload_len, .. } => Some((msg_type, rx.payload(payload_len).to_vec())), RxStep::Eof => { say!("{at}: soundserver hung up the {which} connection; the stream stops with it"); std::process::exit(0); diff --git a/system/inspect/src/lib.rs b/system/inspect/src/lib.rs index 1bd10be66a1..634c1eb85ba 100644 --- a/system/inspect/src/lib.rs +++ b/system/inspect/src/lib.rs @@ -39,7 +39,7 @@ pub fn ask(owner: Owner) -> Result, String> { let deadline = Instant::now() + ANSWER_BOUND; loop { match rx.pump(&conn) { - RxStep::Frame { msg_type: MSG_SNAPSHOT, payload_len } => { + RxStep::Frame { msg_type: MSG_SNAPSHOT, payload_len, .. } => { return toyos_inspect::decode(rx.payload(payload_len), owner) .map_err(|why| format!("`{}` answered something that is not a snapshot: {why}", owner.port)); } diff --git a/system/logkeeper/src/main.rs b/system/logkeeper/src/main.rs index 2dedc6fcd07..3631ae31c1a 100644 --- a/system/logkeeper/src/main.rs +++ b/system/logkeeper/src/main.rs @@ -312,13 +312,13 @@ impl Log { RxStep::Malformed => { panic!("logkeeper: the supervisor sent a frame the origins protocol cannot carry") } - RxStep::Frame { msg_type: REGISTER, payload_len } => { + RxStep::Frame { msg_type: REGISTER, payload_len, .. } => { let payload = self.supervisor_rx.payload(payload_len).to_vec(); let Some(registration) = Registration::decode(&payload) else { panic!("logkeeper: the supervisor registered a ring under no name"); }; let name = registration.tag.as_str(); - let Some([ring, alive]) = self.from_supervisor.recv_handles_exact::<2>() else { + let Some([ring, alive]) = self.supervisor_rx.take_handles::<2>() else { panic!("logkeeper: the supervisor registered {name:?} with no ring"); }; // SAFETY: the kernel moved this handle into this table with diff --git a/system/logkeeper/src/serve.rs b/system/logkeeper/src/serve.rs index bf1c55aface..ec4348cc5d2 100644 --- a/system/logkeeper/src/serve.rs +++ b/system/logkeeper/src/serve.rs @@ -18,7 +18,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Condvar, Mutex}; use std::time::Duration; -use toyos::ipc::{Connection, TrySendError}; +use toyos::ipc::Connection; use toyos::poller::{Poller, WRITABLE}; use toyos::{AsHandle, Pipe}; use toyos_abi::syscall::SyscallError; @@ -88,8 +88,7 @@ fn hand_over(conn: &Connection, end: u64) -> Option { return None; } }; - let sent = conn.send_handles([read.into()]).map_err(TrySendError::Syscall); - match sent.and_then(|()| conn.try_send(SERVED, &end)) { + match conn.try_send_with(SERVED, &end, [read.into()]) { Ok(()) => Some(write), Err(e) => { say!("logkeeper: a reader went before it was answered: {e:?}"); diff --git a/system/netstack/src/client.rs b/system/netstack/src/client.rs index 414f9c0fec6..9d6f87cd4f3 100644 --- a/system/netstack/src/client.rs +++ b/system/netstack/src/client.rs @@ -57,10 +57,9 @@ impl Client { } /// **The answer goes out in one non-blocking write, and a refusal is not - /// retried.** `ipc::send` parks in `sys_write` until the client drains, - /// which is a client deciding when the network stack runs again; and - /// `TrySendError::Full` can have left part of the frame in the pipe, so - /// there is nothing here to retry either. The connection closes either way. + /// retried.** `ipc::send` parks in `SYS_MESSAGE_SEND` until the client + /// drains, which is a client deciding when the network stack runs again. + /// The connection closes either way. /// The log is the only place the machine this runs on gets told that a /// client asked something and was never answered. fn answered(&self, sent: Result<(), ipc::TrySendError>) { @@ -121,6 +120,9 @@ pub struct Request { pub msg_type: u32, pub payload: [u8; MAX_REQUEST], pub payload_len: usize, + /// What the frame carried: claimed by the request that takes handles, and + /// closed with any other. + pub handles: toyos::ipc::Batch, } impl Request { diff --git a/system/netstack/src/main.rs b/system/netstack/src/main.rs index 7253946a4f6..f73f265d04c 100644 --- a/system/netstack/src/main.rs +++ b/system/netstack/src/main.rs @@ -530,15 +530,17 @@ fn main() { ); pending.remove(i); } - RxStep::Frame { msg_type, payload_len } => { + RxStep::Frame { msg_type, payload_len, .. } => { let mut payload = [0u8; toyos::net::MAX_REQUEST]; payload[..payload_len].copy_from_slice(pending[i].rx.payload(payload_len)); + let handles = pending[i].rx.take_batch(); let p = pending.remove(i); requests.push(Request { client: Client { conn: p.conn }, msg_type, payload, payload_len, + handles, }); } } diff --git a/system/netstack/src/serve.rs b/system/netstack/src/serve.rs index 036855e1a43..3a2e28314e6 100644 --- a/system/netstack/src/serve.rs +++ b/system/netstack/src/serve.rs @@ -134,8 +134,8 @@ pub struct Sockets { /// A stream's region off a request's connection, judged, and its bell made: /// the session the node is lent and the bell's read end the answer hands /// over. Refused by name, nothing is mapped. -fn lend(client: Client) -> Result<(Region, Pipe, Rc), (Client, u32)> { - let Some([region]) = client.conn.recv_handles_exact::<1>() else { return Err((client, ERR_INVALID_INPUT)) }; +fn lend(client: Client, handles: ipc::Batch) -> Result<(Region, Pipe, Rc), (Client, u32)> { + let Some([region]) = handles.exact::<1>() else { return Err((client, ERR_INVALID_INPUT)) }; let Ok((bell, ring)) = toyos::pipe_pair() else { // The handle is consumed either way. toyos_abi::syscall::close(region); @@ -155,8 +155,7 @@ fn lend(client: Client) -> Result<(Region, Pipe, Rc), (Client, u32)> /// Answers a stream's request with its bell and `answer`, on the connection /// that is the stream's from here on. fn hand_over(connection: &Connection, bell: Pipe, answer: &T) { - let sent = connection.send_handles([OwnedHandle::from(bell)]).map_err(ipc::TrySendError::Syscall); - if let Err(why) = sent.and_then(|()| connection.try_send(RespType::Result as u32, answer)) { + if let Err(why) = connection.try_send_with(RespType::Result as u32, answer, [OwnedHandle::from(bell)]) { say!("netstack: a stream's client left before its answer: {why:?}"); } } @@ -316,7 +315,7 @@ impl Sockets { pub fn request(&mut self, node: &mut Node, now: Instant, req: Request, acceptor: &Acceptor, draw: fn() -> u32) { match MsgType::from_u32(req.msg_type) { Some(MsgType::TcpClose) => self.close(node, now, &req), - Some(MsgType::UdpBind) => self.udp_bind(node, &req, draw), + Some(MsgType::UdpBind) => self.udp_bind(node, req, draw), Some(MsgType::UdpSendTo) => self.udp_send_to(node, now, &req), Some(MsgType::UdpRecvFrom) => self.udp_recv_from(node, now, req), Some(MsgType::UdpClose) => self.udp_close(node, now, &req), @@ -324,7 +323,7 @@ impl Sockets { Some(MsgType::TcpListenerSetOption) => self.listener_set_option(node, &req), Some(MsgType::UdpSetOption) => self.udp_set_option(node, &req), Some(MsgType::TcpConnect) => self.connect(node, now, req), - Some(MsgType::TcpBindPiped) => self.listen(node, &req, draw), + Some(MsgType::TcpBindPiped) => self.listen(node, req, draw), Some(MsgType::TcpAccept) => self.accept(node, now, req), None => { say!("netstack: unknown message type {}", req.msg_type); @@ -395,7 +394,7 @@ impl Sockets { msg.client.error(ERR_INVALID_INPUT); return; }; - let (session, bell, connection) = match lend(msg.client) { + let (session, bell, connection) = match lend(msg.client, msg.handles) { Ok(lent) => lent, Err((client, code)) => return client.error(code), }; @@ -413,12 +412,12 @@ impl Sockets { } } - fn listen(&mut self, node: &mut Node, msg: &Request, draw: fn() -> u32) { + fn listen(&mut self, node: &mut Node, msg: Request, draw: fn() -> u32) { let Ok(req) = ipc::decode_payload::(msg.payload()) else { msg.client.error(ERR_INVALID_INPUT); return; }; - let Some([wakes]) = msg.client.conn.recv_handles_exact::<{ NOTIFY_HANDLES }>() else { + let Some([wakes]) = msg.handles.exact::<{ NOTIFY_HANDLES }>() else { msg.client.error(ERR_INVALID_INPUT); return; }; @@ -446,7 +445,7 @@ impl Sockets { msg.client.error(accept_refused(AcceptRefused::NoListener)); return; }; - let (session, bell, connection) = match lend(msg.client) { + let (session, bell, connection) = match lend(msg.client, msg.handles) { Ok(lent) => lent, Err((client, code)) => { // Refused, the accept still spends its wake. @@ -474,12 +473,12 @@ impl Sockets { } } - fn udp_bind(&mut self, node: &mut Node, msg: &Request, draw: fn() -> u32) { + fn udp_bind(&mut self, node: &mut Node, msg: Request, draw: fn() -> u32) { let Ok(req) = ipc::decode_payload::(msg.payload()) else { msg.client.error(ERR_INVALID_INPUT); return; }; - let Some([to_client, from_client]) = msg.client.conn.recv_handles_exact::<{ DATA_HANDLES }>() else { + let Some([to_client, from_client]) = msg.handles.exact::<{ DATA_HANDLES }>() else { msg.client.error(ERR_INVALID_INPUT); return; }; diff --git a/system/soundserver/src/client.rs b/system/soundserver/src/client.rs index 5529aa8f728..e6b2364fd18 100644 --- a/system/soundserver/src/client.rs +++ b/system/soundserver/src/client.rs @@ -205,8 +205,7 @@ pub(crate) fn open_stream( let slot_reader = AudioSlotReader::new(shm, client_period_bytes, slot_count); - // Handles first, then the frame that announces them, so a client reading - // the frame is guaranteed to find them; both are consumed either way. + // The handles travel in the frame that announces them, consumed either way. let opened = StreamOpenResponse { client_period_frames, client_period_bytes, @@ -215,8 +214,7 @@ pub(crate) fn open_stream( slot_count: slot_count as u16, fade_frames: ramp_frames, }; - let sent = control.send_handles([client_shm, signal_read.into()]); - if sent.is_err() || control.send(MSG_STREAM_OPENED, &opened).is_err() { + if control.send_with(MSG_STREAM_OPENED, &opened, [client_shm, signal_read.into()]).is_err() { // Client died mid-open; the dropped control connection removes it. say!("soundserver: client {client_id} vanished during stream open"); } diff --git a/system/soundserver/src/control.rs b/system/soundserver/src/control.rs index 5a71f0d2280..f730c016aa8 100644 --- a/system/soundserver/src/control.rs +++ b/system/soundserver/src/control.rs @@ -297,7 +297,7 @@ pub(crate) fn control_thread( disconnected = true; break 'msgs; } - RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), }; // The payload travels with the frame instead of being read off // the connection during dispatch: the read side is finished before diff --git a/system/soundserver/src/output.rs b/system/soundserver/src/output.rs index 5891fd26ed4..23b7ec5fb17 100644 --- a/system/soundserver/src/output.rs +++ b/system/soundserver/src/output.rs @@ -186,10 +186,8 @@ impl Outputs { return; } }; - if let Err(why) = driver.conn.send_handles([handle]).map_err(|e| format!("{e:?}")).and_then(|()| { - driver.conn.try_send_bytes(MSG_ACCEPTED, &[]).map_err(|e| format!("{e:?}")) - }) { - say!("soundserver: an output offer's driver took no acceptance ({why})"); + if let Err(why) = driver.conn.try_signal_with(MSG_ACCEPTED, [handle]) { + say!("soundserver: an output offer's driver took no acceptance ({why:?})"); return; } say!( @@ -238,7 +236,7 @@ impl Outputs { loop { let (msg_type, len) = match control.rx.pump(&control.conn) { RxStep::Idle => return, - RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), RxStep::Eof => return self.drop_driver("it hung up".into(), events), RxStep::Malformed => { return self.drop_driver("it sent a frame this protocol cannot describe".into(), events); @@ -300,7 +298,7 @@ fn first_frame(driver: &mut Driver) -> Option> RxStep::Idle => None, RxStep::Eof => Some(Err("it hung up before its first frame")), RxStep::Malformed => Some(Err("its first frame is one this protocol cannot describe")), - RxStep::Frame { msg_type, payload_len } => Some( + RxStep::Frame { msg_type, payload_len, .. } => Some( FromDriver::decode(msg_type, driver.rx.payload(payload_len)).ok_or("its first frame is no message"), ), } @@ -397,7 +395,7 @@ impl Backend for DriverBackend { while self.ended.is_none() { let len = match self.rx.pump(&self.stream) { RxStep::Idle => break, - RxStep::Frame { msg_type: MSG_PLAYED, payload_len } => payload_len, + RxStep::Frame { msg_type: MSG_PLAYED, payload_len, .. } => payload_len, RxStep::Frame { msg_type, .. } => { self.end(format!("the output driver sent stream frame {msg_type}, which this protocol has not")); break; diff --git a/system/soundserver/src/volume.rs b/system/soundserver/src/volume.rs index cd2fc892924..a8502e412bc 100644 --- a/system/soundserver/src/volume.rs +++ b/system/soundserver/src/volume.rs @@ -91,7 +91,7 @@ pub(crate) fn serve(client: &mut VolumeClient, outputs: &mut Outputs, relay: &mu say!("soundserver: a volume frame this protocol cannot describe, disconnecting"); return false; } - RxStep::Frame { msg_type, payload_len } => (msg_type, payload_len), + RxStep::Frame { msg_type, payload_len, .. } => (msg_type, payload_len), }; let asked = match classify(msg_type, payload_len) { Request::Step => { diff --git a/system/supervisor/src/main.rs b/system/supervisor/src/main.rs index 8db2b5c66d2..b9ac44fc7a0 100644 --- a/system/supervisor/src/main.rs +++ b/system/supervisor/src/main.rs @@ -314,8 +314,7 @@ impl Log { let ring = ring.share().expect("supervisor: a log ring would not duplicate"); let mut payload = [0u8; 4 + MAX_TAG]; let len = Registration { pid, tag }.encode(&mut payload); - let sent = self.conn.send_handles([ring, alive.into()]).map_err(ipc::IpcError::Syscall); - if let Err(e) = sent.and_then(|()| self.conn.send_bytes(REGISTER, &payload[..len])) { + if let Err(e) = self.conn.send_bytes_with(REGISTER, &payload[..len], [ring, alive.into()]) { panic!("supervisor: logkeeper's origins connection refused {name}'s ring: {e:?}"); } } @@ -1356,12 +1355,13 @@ impl<'a> Supervisor<'a> { ); pending.remove(i); } - RxStep::Frame { msg_type, payload_len } => { - let p = pending.remove(i); + RxStep::Frame { msg_type, payload_len, .. } => { + let mut p = pending.remove(i); match p.port { Port::Launcher(caller) => { let payload = p.rx.payload(payload_len).to_vec(); - self.serve_launch(p.conn, &caller, msg_type, &payload, &mut parked) + let carried = p.rx.take_batch(); + self.serve_launch(p.conn, &caller, msg_type, &payload, carried, &mut parked) } Port::Grants(caller) => self.serve_grants(&p.conn, &caller, msg_type, p.rx.payload(payload_len)), Port::Power => self.stop(&p.conn, msg_type), @@ -1867,13 +1867,13 @@ impl Supervisor<'_> { caller: &Caller, msg_type: u32, payload: &[u8], + carried: ipc::Batch, parked: &mut Option, ) { if msg_type != launch::MSG_LAUNCH { return; } - let mut batch = [toyos::RawHandle(0); toyos_abi::syscall::MAX_TRANSFER_HANDLES]; - let received = conn.recv_handles(&mut batch).unwrap_or(0); + let (batch, received) = carried.into_raw(); // **Owned on the statement after they arrive, and before anything can // refuse.** The send moved them into the supervisor's table, so every path out of @@ -2209,11 +2209,8 @@ impl Supervisor<'_> { let handle = unsafe { toyos::OwnedHandle::from_raw(toyos::RawHandle(child.into_raw_handle())) }; // Consumed either way, so the supervisor keeps no `Process` handle from a // launch, or a client launching `/system/bin/true` in a loop exhausts the - // one table the machine cannot do without. A frame refused after the move - // leaves it queued on a connection this call is about to drop. - if conn.send_handles([handle]).is_ok() { - let _ = conn.try_signal(launch::MSG_LAUNCHED); - } + // one table the machine cannot do without. + let _ = conn.try_signal_with(launch::MSG_LAUNCHED, [handle]); } // std's word for the kernel's `Gone` for the place, and nothing else // here answers it: the command is prepared, so its spawn calls no @@ -2275,7 +2272,7 @@ struct Parked { fn heard<'a, const MAX: usize>(step: RxStep, rx: &'a ipc::FrameRx, ended: &'static str) -> consent::Heard<'a> { match step { RxStep::Idle => consent::Heard::Nothing, - RxStep::Frame { msg_type, payload_len } => consent::Heard::Frame(msg_type, rx.payload(payload_len)), + RxStep::Frame { msg_type, payload_len, .. } => consent::Heard::Frame(msg_type, rx.payload(payload_len)), RxStep::Eof => consent::Heard::Ended(ended), RxStep::Malformed => consent::Heard::Ended("it sent a frame the protocol cannot describe"), } diff --git a/tests/guest/rust-tests/src/bin/abuse_shared_grant.rs b/tests/guest/rust-tests/src/bin/abuse_shared_grant.rs index 66a2df3420c..2b61ae9eb4a 100644 --- a/tests/guest/rust-tests/src/bin/abuse_shared_grant.rs +++ b/tests/guest/rust-tests/src/bin/abuse_shared_grant.rs @@ -6,7 +6,8 @@ //! unreported, and the target pid was unchecked so the list would take pids //! that had never existed. None of that has a spelling any more — a region is a //! handle, holding one is the whole of being allowed to map it, and giving one -//! away is `SYS_HANDLE_SEND` over a connection the giver already holds. +//! away is a frame carrying it (`SYS_MESSAGE_SEND`) over a connection the +//! giver already holds. //! //! So the subject moves to what replaced it. The negative arm is the one the //! ACL could never state: a process that was sent nothing cannot reach the @@ -73,10 +74,8 @@ fn owner() { let conn = acceptor.accept().expect("the invited peer connects"); let shared = region.share().expect("a second handle to our own region"); - conn.send_handles([shared]).expect("send the region to its peer"); - // The frame the handle was sent ahead of. The peer reads the frame first - // and is guaranteed to find the handle already queued. - conn.signal(1).expect("announce the region"); + // In the frame that announces it: the peer takes it with the frame. + conn.signal_with(1, [shared]).expect("send the region to its peer"); let said = report(&mut invited); assert!(invited.wait().expect("wait the invited peer").success(), "the invited peer failed"); @@ -147,9 +146,8 @@ fn marker(role: &str) { fn invited() { let conn = toyos::endow::service(SERVICE).expect("the invited peer holds a connector"); - // The frame first, then the handle it was sent ahead of. - conn.recv_header().expect("the owner's announcement"); - let [handle] = conn.recv_handles_exact::<1>().expect("the region the owner sent"); + let header = conn.recv_header().expect("the owner's announcement"); + let [handle] = conn.recv_handles_exact::<1>(&header).expect("the region the owner sent"); let region = SharedMemory::lent(handle, REGION).expect("take the region"); println!("read {}", String::from_utf8_lossy(®ion.as_slice()[..SECRET.len()])); std::io::stdout().flush().expect("invited: flush the report"); diff --git a/tests/guest/rust-tests/src/bin/cpal_drop_unreleased.rs b/tests/guest/rust-tests/src/bin/cpal_drop_unreleased.rs index 5f1cc6e5d0e..f5355ba21ab 100644 --- a/tests/guest/rust-tests/src/bin/cpal_drop_unreleased.rs +++ b/tests/guest/rust-tests/src/bin/cpal_drop_unreleased.rs @@ -96,9 +96,8 @@ fn serve() { ) .expect("a ring"); let (signal_read, signal_write) = toyos::pipe_pair().expect("a signal pipe"); - conn.send_handles([ring.share().expect("the ring to share"), signal_read.into()]) - .expect("the open's handles"); - conn.send(MSG_STREAM_OPENED, &OPENED).expect("the open answered"); + conn.send_with(MSG_STREAM_OPENED, &OPENED, [ring.share().expect("the ring to share"), signal_read.into()]) + .expect("the open answered with its handles"); let close = conn.recv_header().expect("the client's close"); assert_eq!(close.msg_type, MSG_STREAM_CLOSE, "the dropped stream did not close"); diff --git a/tests/guest/rust-tests/src/bin/ends_sent_over_each_other_both_end.rs b/tests/guest/rust-tests/src/bin/ends_sent_over_each_other_both_end.rs new file mode 100644 index 00000000000..946afa68b14 --- /dev/null +++ b/tests/guest/rust-tests/src/bin/ends_sent_over_each_other_both_end.rs @@ -0,0 +1,60 @@ +//! Two client ends sent over each other cannot keep each other open: a handle +//! travels only inside a frame, every server reads every frame, and a frame's +//! handles its server does not take are closed at its next read. So once both +//! clients are gone, both connections end. + +use toyos::ipc::{Connection, FrameRx, RxStep}; +use toyos::poller::{Poller, READABLE}; +use toyos::{namespace, port, AsHandle, OwnedHandle}; +use toyos_abi::syscall; + +/// A failure ceiling only: an end that never comes is the defect. +const CEILING_NANOS: u64 = 10_000_000_000; + +fn main() { + let (acceptor, connector) = port::create().expect("a port"); + let names = namespace::build().add("conn", &connector).finish().expect("a namespace"); + let pair = || { + let client = names.open("conn").expect("connect"); + (client, acceptor.accept().expect("accept")) + }; + + let ((client_a, server_a), (client_b, server_b)) = (pair(), pair()); + client_b.signal_with(1, [dup(&client_a)]).expect("A's end over B"); + client_a.signal_with(1, [dup(&client_b)]).expect("B's end over A"); + drop((client_a, client_b)); + let mut ended = [false, false]; + let mut rx = [FrameRx::<0>::new(), FrameRx::<0>::new()]; + let servers = [&server_a, &server_b]; + while ended != [true, true] { + let poller = Poller::new(2); + for (i, s) in servers.iter().enumerate().filter(|(i, _)| !ended[*i]) { + poller.watch(*s, READABLE, i as u64); + } + let mut woke = false; + poller.wait(1, CEILING_NANOS, |_| woke = true); + assert!(woke, "two ends sent over each other kept each other open"); + for i in 0..2 { + loop { + match rx[i].pump(servers[i]) { + // Not taken: the next pump closes it, as any server's does. + RxStep::Frame { handles: 1, .. } => {} + RxStep::Frame { .. } | RxStep::Malformed => panic!("a frame the test never sent"), + RxStep::Eof => { + ended[i] = true; + break; + } + RxStep::Idle => break, + } + } + } + } + println!("ends_sent_over_each_other_both_end: ok"); +} + +/// A second handle to `conn`'s end, to send. +fn dup(conn: &Connection) -> OwnedHandle { + let raw = syscall::dup(conn.as_handle()).expect("a second handle to the end"); + // SAFETY: a fresh duplicate, answered for by nothing else. + unsafe { OwnedHandle::from_raw(raw) } +} diff --git a/tests/guest/rust-tests/src/bin/message_carries_its_handles.rs b/tests/guest/rust-tests/src/bin/message_carries_its_handles.rs new file mode 100644 index 00000000000..f23a689077c --- /dev/null +++ b/tests/guest/rust-tests/src/bin/message_carries_its_handles.rs @@ -0,0 +1,89 @@ +//! A connection carries handles only inside messages: the batch is pinned +//! just past its frame's bytes, so it arrives with them and with nothing else. +//! +//! - A frame's handles are taken with the frame. +//! - Handles with no bytes are refused whole: a poll-driven server sees +//! nothing to read, and the sender still holds what it tried to send. +//! - A batch its frame did not announce stops the reader at it, and dies with +//! the connection the reader then drops. + +use toyos::ipc::{Connection, FrameRx, RxStep}; +use toyos::poller::{Poller, OTHER_END_GONE, READABLE}; +use toyos::{namespace, pipe_pair, port, AsHandle, OwnedHandle, Pipe}; +use toyos_abi::syscall::{self, SyscallError}; + +/// A failure ceiling only: an answer that never comes is the defect. +const CEILING_NANOS: u64 = 10_000_000_000; + +fn main() { + let (acceptor, connector) = port::create().expect("a port"); + let names = namespace::build().add("conn", &connector).finish().expect("a namespace"); + let pair = || { + let client = names.open("conn").expect("connect"); + (client, acceptor.accept().expect("accept")) + }; + + // A frame's handles arrive with it. + let (client, server) = pair(); + let (read_end, write_end) = pipe_pair().expect("a pipe"); + client.signal_with(7, [OwnedHandle::from(write_end)]).expect("a frame carrying a handle"); + let header = server.recv_header().expect("the frame"); + assert_eq!((header.msg_type, header.handles()), (7, 1), "the frame does not announce its handle"); + let [received] = server.recv_handles_exact::<1>(&header).expect("the handle, with its frame"); + syscall::close(received); + assert_eq!(gone(&read_end), Ok(OTHER_END_GONE), "the handle taken with its frame is not the one sent"); + println!("message_carries_its_handles: a frame's handle arrived with it"); + + // Handles with no bytes are refused, and the server has nothing to read. + let (stray_read, stray_write) = pipe_pair().expect("a pipe"); + assert_eq!( + syscall::message_send(client.as_handle(), &[], &[stray_write.as_handle()], false), + Err(SyscallError::InvalidArgument), + "handles were sent with no bytes" + ); + assert!(!readable(&server, 0), "a refused send left the server something to read"); + assert_eq!(stray_write.write(b"x"), Ok(1), "a refused handle left its sender"); + drop(stray_write); + assert_eq!(gone(&stray_read), Ok(OTHER_END_GONE), "the refused handle stayed anywhere but here"); + println!("message_carries_its_handles: handles alone were refused, and the server stayed idle"); + + // A batch its frame did not announce stops the reader, and goes with the connection. + let (client, server) = pair(); + let (lied_read, lied_write) = pipe_pair().expect("a pipe"); + let mut frame = [0u8; 12]; + frame[..4].copy_from_slice(&9u32.to_ne_bytes()); + // Given up before the send, which moves it: this process no longer holds the number. + let lied_write = lied_write.into_raw(); + syscall::message_send(client.as_handle(), &frame, &[lied_write], false) + .expect("a message whose frame announces no handle"); + let mut rx = FrameRx::<0>::new(); + assert!(readable(&server, CEILING_NANOS), "the message did not make the server readable"); + assert!( + matches!(rx.pump(&server), RxStep::Frame { msg_type: 9, handles: 0, .. }), + "the frame was not read as it was written" + ); + assert!(matches!(rx.pump(&server), RxStep::Malformed), "the reader went past a batch no frame announced"); + drop(rx); + drop(server); + assert_eq!(gone(&lied_read), Ok(OTHER_END_GONE), "the unannounced batch outlived its connection"); + drop(client); + println!("message_carries_its_handles: ok"); +} + +/// Whether `conn` answers readable within `timeout`. +fn readable(conn: &Connection, timeout: u64) -> bool { + let poller = Poller::new(1); + poller.watch(conn, READABLE, 1); + let mut answered = false; + poller.wait(u32::from(timeout != 0), timeout, |_| answered = true); + answered +} + +/// What a watch for `end`'s other end leaving is answered, within the ceiling. +fn gone(end: &Pipe) -> Result { + let poller = Poller::new(1); + poller.watch_raw(end.as_handle(), OTHER_END_GONE, 1); + let mut answer = None; + poller.wait_answers(1, CEILING_NANOS, |_, said| answer = Some(said)); + answer.expect("the other end never left") +} diff --git a/tests/guest/rust-tests/src/bin/mutual_kill.rs b/tests/guest/rust-tests/src/bin/mutual_kill.rs index 63679514a7f..f5d857af8ae 100644 --- a/tests/guest/rust-tests/src/bin/mutual_kill.rs +++ b/tests/guest/rust-tests/src/bin/mutual_kill.rs @@ -95,8 +95,7 @@ fn killer_child() -> (Connection, Child) { fn arm(conn: &Connection, go: &SharedMemory, victim: RawHandle) { // SAFETY: a fresh duplicate, answered for by nothing else. let victim = unsafe { OwnedHandle::from_raw(syscall::dup(victim).expect("a handle to send")) }; - conn.send_handles([go.share().expect("the word to send"), victim]).expect("send the batch"); - conn.signal(ARM).expect("say the batch is queued"); + conn.signal_with(ARM, [go.share().expect("the word to send"), victim]).expect("send the batch"); } /// Wait for the killer's one line: it holds its victim and is spinning. @@ -121,7 +120,7 @@ fn killer() -> ! { let conn = ns.open(SERVICE).expect("connect through the endowed connector"); let header = conn.recv_header().expect("the arming frame"); assert_eq!(header.msg_type, ARM, "an unexpected frame"); - let [word_handle, victim] = conn.recv_handles_exact::<2>().expect("the word and the victim"); + let [word_handle, victim] = conn.recv_handles_exact::<2>(&header).expect("the word and the victim"); let go = SharedMemory::lent(word_handle, WORD_BYTES).expect("map the word"); // SAFETY: the parent sent one handle to the victim, which nothing else here owns. let victim = unsafe { Process::from_raw(victim) }; diff --git a/tests/guest/rust-tests/src/bin/netstack_gone_mid_bind.rs b/tests/guest/rust-tests/src/bin/netstack_gone_mid_bind.rs index 5e0686dd8ed..8cc8de5d0ae 100644 --- a/tests/guest/rust-tests/src/bin/netstack_gone_mid_bind.rs +++ b/tests/guest/rust-tests/src/bin/netstack_gone_mid_bind.rs @@ -23,16 +23,14 @@ //! already worked, and it is asserted through `std::net::TcpListener` as //! well as through the SDK, because `ErrorKind::NotConnected` is the literal //! thing `/system/bin/sshserver` matches on. -//! 2. **Gone between the connect and the handle transfer.** The client is -//! connected and queued when the port closes. `tcp_bind` hands netstack a pipe -//! end, and a request that carries handles moves them *before* it writes the -//! frame — so `SYS_HANDLE_SEND` is the first thing it can be refused at, and -//! it answers `SyscallError::Gone`. The refused send consumed the pipe end -//! and closed it, so the pipe's read end, still the child's, reads end of -//! file: the kernel left the end at its own number and the SDK closed it. -//! 3. **The same, for a request that carries no handles**, which reaches the -//! frame write instead, where a pipe with no reader is the same -//! `SyscallError::Gone`. +//! 2. **Gone between the connect and the request.** The client is connected +//! and queued when the port closes. `tcp_bind` hands netstack a pipe end in +//! its request's frame, and `SYS_MESSAGE_SEND` refuses the whole message with +//! `SyscallError::Gone`. The refused send consumed the pipe end and closed +//! it, so the pipe's read end, still the child's, reads end of file: the +//! kernel left the end at its own number and the SDK closed it. +//! 3. **The same, for a request that carries no handles**, which the same send +//! refuses with the same `SyscallError::Gone`. //! //! Arms 2 and 3 are the ones that produced the panic. They are red on a tree //! where `toyos::net::hangup` maps anything but `IpcError::Disconnected` to @@ -176,10 +174,9 @@ fn already_gone() -> ! { /// Which syscall the child's request is refused at. enum Request { - /// Carries a pipe end, so `SYS_HANDLE_SEND` goes first — `tcp_bind`'s own - /// shape. + /// Carries a pipe end, which a refused send closes — `tcp_bind`'s own shape. Bind, - /// Carries none, so the frame write is the first thing the kernel sees. + /// Carries none. Close, } diff --git a/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs b/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs index 4787208946f..b413c16b16b 100644 --- a/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs +++ b/tests/guest/rust-tests/src/bin/pipe_other_end_gone.rs @@ -98,9 +98,10 @@ fn test() { assert_eq!(at_last(&reader), Ok(OTHER_END_GONE), "the write end's last handle closed"); let (reader, writer) = pipe(""); - client.send_handles([OwnedHandle::from(writer)]).expect("the write end into the connection's queue"); + client.signal_with(1, [OwnedHandle::from(writer)]).expect("the write end into the connection's queue"); held(&reader, "its handle sits unreceived in a connection's queue"); - let [received] = server.recv_handles_exact::<1>().expect("the handle the queue held"); + let header = server.recv_header().expect("the frame that carried it"); + let [received] = server.recv_handles_exact::<1>(&header).expect("the handle the queue held"); held(&reader, "its handle was received"); syscall::close(received); assert_eq!(at_last(&reader), Ok(OTHER_END_GONE), "the received handle closed"); diff --git a/tests/guest/rust-tests/src/bin/process_tree.rs b/tests/guest/rust-tests/src/bin/process_tree.rs index cc1a8b121fa..4e47ec6a494 100644 --- a/tests/guest/rust-tests/src/bin/process_tree.rs +++ b/tests/guest/rust-tests/src/bin/process_tree.rs @@ -140,7 +140,7 @@ fn grow() -> Grown { let conn = acceptor.accept().expect("B connected back"); let header = conn.recv_header().expect("B's word that its subtree is grown"); assert_eq!(header.msg_type, MSG_GROWN, "B said something else"); - let [c, d, b_self, e] = conn.recv_handles_exact::<4>().expect("B sent four handles"); + let [c, d, b_self, e] = conn.recv_handles_exact::<4>(&header).expect("B sent four handles"); // SAFETY: B moved each into this table with the frame, and nothing else // answers for them. let (c, d, e) = unsafe { (Process::from_raw(c), Process::from_raw(d), Process::from_raw(e)) }; @@ -399,8 +399,7 @@ fn b() -> ! { syscall::dup(RawHandle(e.as_raw_handle())).expect("a copy of E"), ]; // SAFETY: four fresh duplicates, answered for by nothing else. - conn.send_handles(handles.map(|h| unsafe { OwnedHandle::from_raw(h) })).expect("send A the subtree"); - conn.send_bytes(MSG_GROWN, &[]).expect("announce the subtree"); + conn.signal_with(MSG_GROWN, handles.map(|h| unsafe { OwnedHandle::from_raw(h) })).expect("send A the subtree"); let header = conn.recv_header().expect("A's word to fault"); assert_eq!(header.msg_type, MSG_FAULT, "A said something else"); diff --git a/tests/guest/rust-tests/src/bin/shm_release_reclaims.rs b/tests/guest/rust-tests/src/bin/shm_release_reclaims.rs index 11fd6b3b3c6..91ce3511764 100644 --- a/tests/guest/rust-tests/src/bin/shm_release_reclaims.rs +++ b/tests/guest/rust-tests/src/bin/shm_release_reclaims.rs @@ -83,10 +83,8 @@ fn main() { .expect("spawn donor"); let conn = acceptor.accept().expect("the donor connects"); - conn.recv_header().expect("the donor announces its region"); - let [sent] = conn - .recv_handles_exact::<1>() - .expect("the donor sent the region ahead of the frame"); + let header = conn.recv_header().expect("the donor announces its region"); + let [sent] = conn.recv_handles_exact::<1>(&header).expect("the donor sent the region with the frame"); // Only now, after the donor has let go — its line says so. let mut out = BufReader::new(donor.stdout.take().expect("donor stdout")); @@ -120,8 +118,7 @@ fn donor() { region.as_mut_slice()[..PAYLOAD.len()].copy_from_slice(PAYLOAD); let shared = region.share().expect("donor: a second handle"); - conn.send_handles([shared]).expect("donor: send the region"); - conn.signal(1).expect("donor: announce it"); + conn.signal_with(1, [shared]).expect("donor: send the region"); // The maker lets go while the peer has not mapped yet: no process has this // region mapped, and the in-flight handle is the only thing keeping it — diff --git a/tests/guest/rust-tests/src/bin/spawn_lands_claimed.rs b/tests/guest/rust-tests/src/bin/spawn_lands_claimed.rs index f22c494797d..e2dd02be44a 100644 --- a/tests/guest/rust-tests/src/bin/spawn_lands_claimed.rs +++ b/tests/guest/rust-tests/src/bin/spawn_lands_claimed.rs @@ -52,7 +52,7 @@ fn test() { let conn = acceptor.accept().expect("the place connected back"); let header = conn.recv_header().expect("the place's word"); assert_eq!(header.msg_type, MSG_SELF, "the place said something else"); - let [place_self] = conn.recv_handles_exact::<1>().expect("the place sent its self"); + let [place_self] = conn.recv_handles_exact::<1>(&header).expect("the place sent its self"); assert!(place.try_wait().expect("ask after the place").is_none(), "the place ended before the marked spawn"); assert_eq!( @@ -105,8 +105,7 @@ fn place() -> ! { let conn = endow::service(BACK).expect("the test endowed a port back"); let own = syscall::dup(endow::this_process().as_handle()).expect("a copy of this process's self"); // SAFETY: a fresh duplicate, answered for by nothing else. - conn.send_handles([unsafe { OwnedHandle::from_raw(own) }]).expect("send the test this process's self"); - conn.send_bytes(MSG_SELF, &[]).expect("announce this process's self"); + conn.signal_with(MSG_SELF, [unsafe { OwnedHandle::from_raw(own) }]).expect("send the test this process's self"); park() } diff --git a/tests/guest/rust-tests/src/bin/stream_handles.rs b/tests/guest/rust-tests/src/bin/stream_handles.rs index 9fc578ab9ff..ca663771fe3 100644 --- a/tests/guest/rust-tests/src/bin/stream_handles.rs +++ b/tests/guest/rust-tests/src/bin/stream_handles.rs @@ -158,7 +158,7 @@ fn run(role: &str) -> (Child, Netstack) { let header = conn.recv_header().expect("a request"); assert_eq!(header.msg_type, MsgType::TcpConnect as u32, "a stream's connect"); let _: TcpConnectRequest = conn.recv_payload(&header).expect("the connect's request"); - let [region] = conn.recv_handles_exact::<1>().expect("the stream's region"); + let [region] = conn.recv_handles_exact::<1>(&header).expect("the stream's region"); let region = SharedMemory::lent(region, REGION_BYTES).expect("a region netstack takes"); // SAFETY: the mapping is `REGION_BYTES`, 2 MiB aligned, and its header is reached only as atomics. let words = unsafe { &*(region.as_ptr() as *const Words) }; @@ -167,8 +167,8 @@ fn run(role: &str) -> (Child, Netstack) { unsafe { std::ptr::copy_nonoverlapping(PEER.as_ptr(), region.as_ptr().add(RECEIVE_RING), PEER.len()) }; let _ = server.publish(words, PEER.len() as u32).expect("the client keeps the protocol"); let (bell, ring) = toyos::pipe_pair().expect("the bell"); - conn.send_handles([OwnedHandle::from(bell)]).expect("hand over the bell"); - conn.send(RespType::Result as u32, &TcpConnectResponse { local_port: 49_152, _pad: 0 }).expect("answer the connect"); + conn.send_with(RespType::Result as u32, &TcpConnectResponse { local_port: 49_152, _pad: 0 }, [OwnedHandle::from(bell)]) + .expect("answer the connect with the bell"); (child, Netstack { conn, _region: region, _bell: ring, _server: server }) }