diff --git a/issues/build/a-key-being-built-is-waited-for-and-another-key-is-not-reds-under-host-load.md b/issues/build/a-key-being-built-is-waited-for-and-another-key-is-not-reds-under-host-load.md new file mode 100644 index 00000000000..4874c8968a0 --- /dev/null +++ b/issues/build/a-key-being-built-is-waited-for-and-another-key-is-not-reds-under-host-load.md @@ -0,0 +1,24 @@ +--- +status: open +kind: tooling +opened: 2026-09-28 +--- + +# `a_key_being_built_is_waited_for_and_another_key_is_not` reds under host load + +`src/buildlock.rs`'s last assertion, `keyed_idle(&root, Keyed::Sysroot, +"k1").is_some()` after the `using` guard drops, failed once under `cargo test +--lib` on a host running several other agents' builds concurrently +(`wt/toyos-desk1` at `e039fe6b`). Run alone straight after, the same test +passed in 0.59 s. The test spawns real child processes and times state +transitions against wall-clock sleeps and deadlines (`appeared`, +`keyed_idle`'s own polling), so host contention can move an event past a +window the test assumed was empty. + +**Exit**: reproduce under synthetic host load (parallel `cargo build`s pinned +to the same cores) to find which wait the contention defeats, then replace +the polled read with the event itself. + +## Owner + +Held by the orchestrator. diff --git a/issues/design-debt/a-client-waits-on-the-compositors-answer-with-no-bound.md b/issues/design-debt/a-client-waits-on-the-compositors-answer-with-no-bound.md new file mode 100644 index 00000000000..559be9f6410 --- /dev/null +++ b/issues/design-debt/a-client-waits-on-the-compositors-answer-with-no-bound.md @@ -0,0 +1,17 @@ +--- +status: open +kind: defect +opened: 2026-09-28 +--- + +# A client waits on the compositor's answer with no bound + +`window::clipboard_set` (`userland/toyos-window/src/lib.rs`) blocks in +`recv_header` for `MSG_COPY_REGION` on a copy past `MAX_INLINE_PAYLOAD`, and +`Window::create` blocks the same way for `MSG_WINDOW_CREATED`. Neither wait has +a deadline, so a compositor that is alive and not answering wedges every +terminal or editor that copies, and every program that asks for a window. + +Owner: `toyos-window`. + +**Exit**: each wait has a bound, and a missed one is a `CreateError` of its own. diff --git a/issues/design-debt/decode-payload-accepts-bytes-past-its-type.md b/issues/design-debt/decode-payload-accepts-bytes-past-its-type.md new file mode 100644 index 00000000000..ff918aabc7d --- /dev/null +++ b/issues/design-debt/decode-payload-accepts-bytes-past-its-type.md @@ -0,0 +1,19 @@ +--- +status: open +kind: defect +opened: 2026-09-28 +--- + +# `ipc::decode_payload` accepts bytes past the type it decodes + +`toyos::ipc::decode_payload` refuses a payload shorter than its `T` and ignores +whatever follows one. Each caller decides for itself whether trailing bytes are +out of protocol, and the compositor's client frames +(`userland/compositor/src/session.rs`) disagree: `copy_begin` refuses them with +a length check of its own, and `MSG_PRESENT`'s `Rect`, `MSG_SET_CURSOR`'s +style, `ResolutionRequest` and `CreateWindowRequest` accept them. + +Owner: `toyos::ipc`. + +**Exit**: one trailing-bytes rule in `toyos::ipc` that every `decode_payload` +caller gets, and `copy_begin`'s own check deleted. diff --git a/issues/isolation/a-received-handle-has-no-knowable-type.md b/issues/isolation/a-received-handle-has-no-knowable-type.md index 9e4dd02c6b9..ebbb2f4c887 100644 --- a/issues/isolation/a-received-handle-has-no-knowable-type.md +++ b/issues/isolation/a-received-handle-has-no-knowable-type.md @@ -31,6 +31,11 @@ typed can be ended by whoever sent it.** The sender needs nothing but - An audio client receives two handles from soundd and calls `SYS_SHM_MAP` on the first (`toyos/src/audio.rs`). A hostile *server* ends every client. - A window client receives its buffer the same way (`userland/toyos-window/src/lib.rs`). +- blockd maps the region a client sends with its open (`Region::adopt` in + `userland/blockd/src/region.rs`), so a *client* holding its connector ends it. +- netd adopts as pipes the two handles a client sends with a piped socket + (`DataPipes::take`) and the one with a piped bind (`handle_tcp_bind_piped`), + both in `userland/netd/src/main.rs`: a *client* ends it the same way. Nothing in the tree is hostile today, so nothing fails. The property the architecture claims — that a process cannot be harmed by what it was not given — @@ -57,18 +62,6 @@ change, and a new syscall is the owner's to approve. Judged while clearing PR #22's blockers, with the reasoning written down because the next reader will ask why a class this wide was left open. -- **The one instance a hostile *client* can reach is closed.** `/system/bin/init`'s - launcher takes `extra` connectors from anybody holding a `launcher` connector - and hands them to `SYS_NAMESPACE_BUILD`, and that call answers - `InvalidArgument` for a wrong type rather than ending the caller. It is the - one handle argument in the ABI that routinely crosses a trust boundary, - `kernel/CLAUDE.md` says so where the policy is stated, and `launcher_refusals` - gates it. -- **Every other instance needs a hostile *server*** — soundd sending an audio - client its region, the compositor sending a window its buffer. A client whose - server is hostile has already lost: that server chooses what the client sees, - when it is answered, and whether it is answered at all. Ending it with a - `WrongType` is not a new capability. - **The fix is an ABI shape change and the ABI was the owner's to approve.** It widens `SYS_HANDLE_RECV`'s answer from `n` to `n` pairs, which is a syscall the owner approved changing shape after the fact. diff --git a/issues/isolation/a-refused-handle-move-leaves-the-compositor-holding-it.md b/issues/isolation/a-refused-handle-move-leaves-the-compositor-holding-it.md new file mode 100644 index 00000000000..441297e51f3 --- /dev/null +++ b/issues/isolation/a-refused-handle-move-leaves-the-compositor-holding-it.md @@ -0,0 +1,35 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# A refused handle move leaves the compositor holding what it meant to send + +`deliver_with_handles` (`userland/compositor/src/client.rs`) sends through +`Connection::try_send_with_handles`, which calls `syscall::handle_send` and +then the frame. When `handle_send` itself is refused, the kernel restores every +handle at its own number (`sys_handle_send` in `kernel/src/syscall/ipc.rs`). +The compositor drops the client, but it still holds the handle and never +closes it. + +Each refusal keeps one handle slot and one region. Three sites are affected: + +- `create_window`'s `MSG_WINDOW_CREATED`: a client that closes before the + answer arrives. +- `paste`'s `MSG_CLIPBOARD_PASTE_SHM`. +- `rebuffer`'s `MSG_WINDOW_RESIZED`, reached by any app through + `MSG_SET_RESOLUTION`, which reallocates every window's buffer. A window that + never takes its handles fills its `MAX_QUEUED_BATCHES` queue, and the next + move is refused. + +A client can repeat this, and nothing bounds it before the handle table or +memory runs out. + +Owner: the compositor's client delivery, `deliver_with_handles` in +`userland/compositor/src/client.rs`. + +**Exit**: `deliver_with_handles` calls `syscall::handle_send` on its own and +closes the handles if that is refused, then sends the frame. It never closes a +handle after a successful move. `copy_begin` in +`userland/compositor/src/session.rs` already has this shape. diff --git a/issues/isolation/a-window-buffer-is-read-while-its-client-writes-it.md b/issues/isolation/a-window-buffer-is-read-while-its-client-writes-it.md new file mode 100644 index 00000000000..d12454c2b12 --- /dev/null +++ b/issues/isolation/a-window-buffer-is-read-while-its-client-writes-it.md @@ -0,0 +1,20 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# A window's buffer is read while its client writes it + +Each window has one region, which the client draws into and the compositor +blits from (`render::draw_window` in `userland/compositor/src/render.rs`). The +compositor reads it as a `&[u8]` while the client may be writing it: a data +race in Rust's model, and on the panel a frame whose pixels come from two of +the client's frames. + +Owner: the compositor's window blit, `render::draw_window` in +`userland/compositor/src/render.rs`. + +**Exit**: the compositor makes two buffers per window, a client hands one over +with its present and gets it back on release, and the compositor never reads a +buffer the client holds. diff --git a/issues/isolation/sharedmemory-slices-rest-on-a-size-nobody-checks.md b/issues/isolation/sharedmemory-slices-rest-on-a-size-nobody-checks.md new file mode 100644 index 00000000000..25ca04aaca3 --- /dev/null +++ b/issues/isolation/sharedmemory-slices-rest-on-a-size-nobody-checks.md @@ -0,0 +1,29 @@ +--- +status: open +kind: defect +opened: 2026-09-28 +--- + +# `SharedMemory`'s safe slices rest on a size nobody checks + +`SharedMemory::as_slice`, `as_mut_slice` and `as_atomic` (`toyos/src/shm.rs`) +are safe functions that make a slice of `size` bytes over the mapping. The type +enforces neither premise that makes this sound: + +- `adopt` is safe and takes `size` from its caller without checking it against + the region the kernel mapped. A server that adopts a peer's region at the + peer's declared length reads and writes past it in safe code. +- `shm_map` is idempotent, so `share()` followed by `adopt` gives two + `SharedMemory` values over one mapping in one process. `as_mut_slice` on one + then aliases `as_slice` or `as_atomic` on the other. + +`#![forbid(unsafe_code)]` on the compositor (`userland/compositor/src/main.rs`) +leans on both. The compositor meets them, since it adopts only regions it +created and the kernel's framebuffer and cursor, but the compiler does not know +that. + +Owner: `toyos::shm`. + +**Exit**: `adopt` is an `unsafe fn` whose contract is both premises, or it +checks `size` against the kernel's region and refuses a second mapping of one +region. diff --git a/issues/isolation/the-compositor-ignores-a-message-it-does-not-know.md b/issues/isolation/the-compositor-ignores-a-message-it-does-not-know.md new file mode 100644 index 00000000000..4e02e63b686 --- /dev/null +++ b/issues/isolation/the-compositor-ignores-a-message-it-does-not-know.md @@ -0,0 +1,23 @@ +--- +status: open +kind: defect +opened: 2026-09-27 +--- + +# The compositor ignores a message type it does not know + +`Session::dispatch` (`userland/compositor/src/session.rs`) ends its match with +`_ => {}`: a frame of a type no protocol here defines is read, framed, and +dropped without a word, on a window's connection and on a fresh one alike. The +one retired type, `window::MSG_RETIRED_CLIPBOARD_SET_SHM`, is refused by name; +every other unknown type is accepted and silently discarded. + +`compositor_stall`'s streaming case depends on it: its window sends +`UNKNOWN_MSG` on every pass as load with nothing to draw +(`tests/toyos-rust-tests/src/bin/compositor_stall.rs`). A refusal there would +drop the window after its first frame and leave the case passing with no load. + +Owner: `Session::dispatch` in `userland/compositor/src/session.rs`. + +**Exit**: an unknown type drops its client with `DropReason::OutOfProtocol`, +and the stall's stream is a type the compositor serves without drawing. diff --git a/issues/isolation/the-compositor-keeps-a-committed-regions-mapping-for-as-long-as-the-client-does.md b/issues/isolation/the-compositor-keeps-a-committed-regions-mapping-for-as-long-as-the-client-does.md new file mode 100644 index 00000000000..b63546278f1 --- /dev/null +++ b/issues/isolation/the-compositor-keeps-a-committed-regions-mapping-for-as-long-as-the-client-does.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-09-28 +--- + +# The compositor keeps a committed region's mapping for as long as the client does + +`CopyRegion::take` (`userland/compositor/src/client.rs`) drops the +compositor's own `SharedMemory`, closing its handle to the region. That does +not unmap it: `SharedMemObject::on_zero_handles` (`kernel/src/object/shm.rs`) +tears down every process's mapping together, only once every handle to the +object is gone anywhere — `unmap_from` exists to drop one process's own +mapping on its own, but nothing outside `kernel/src/inbox/mod.rs` calls it. A +client that keeps the handle its copy answered with keeps the compositor's own +2 MiB mapping alive too, for as long as it likes, and a client that copies +repeatedly and keeps every handle leaves one such mapping per copy — bounded +only by its own handle table, never by the compositor's need for the memory. + +Owner: `kernel::object::shm`'s handle-driven mapping teardown. + +**Exit**: closing a process's last handle to a shared-memory object unmaps +that process's own view at once, through `unmap_from`, independent of whether +another process still holds a handle to the same object. diff --git a/tests/toyos-rust-tests/src/bin/compositor_client_death.rs b/tests/toyos-rust-tests/src/bin/compositor_client_death.rs index 04eeffb4038..018b96be915 100644 --- a/tests/toyos-rust-tests/src/bin/compositor_client_death.rs +++ b/tests/toyos-rust-tests/src/bin/compositor_client_death.rs @@ -20,9 +20,9 @@ //! each one used to take a process with it. //! //! Each case leaves its damage standing and then asks the compositor a -//! question **with a deadline**, exactly as `compositor_stall` does — the host -//! asserts the other half, that the desktop is still painting and that every -//! client dropped on the way was named with its pid. +//! question it answers from its dispatch — the host asserts the other half, +//! that the desktop is still painting and that every client dropped on the way +//! was named with its pid. use std::io::{BufRead, BufReader}; use std::os::toyos::process::CommandExt; @@ -30,9 +30,8 @@ use std::process::{exit, Command, Stdio}; use toyos::endow; use toyos::AsHandle; -use toyos::shm::SharedMemory; use toyos::{ipc, Connection}; -use toyos_abi::syscall::{self, SyscallError}; +use toyos_abi::syscall; use toyos_abi::RawHandle; use window::Window; @@ -45,21 +44,8 @@ const RELAY_SOCKET: RawHandle = RawHandle(3); /// reaped. Nothing is ever read off it but the hang-up. const RELAY_GO: RawHandle = RawHandle(4); -/// `MSG_GET_RESOLUTION` is answered from the compositor's dispatch, so a reply -/// proves the event loop reached the end of a pass rather than merely that the -/// process still exists. -const PROBE_POLLS: u32 = 500; -const PROBE_POLL_NS: u64 = 10_000_000; - -/// How many events a closed window is asked for. -/// -/// Two would do — `Close`, then `None` — and this is a handful more so the -/// failure prints a stream rather than a single wrong answer. Each poll past -/// the close costs nothing: the handle is ready, so none of them waits. -const POLLS_AFTER_CLOSE: usize = 8; -/// Long enough that a compositor still on its way to closing the connection is -/// waited for rather than raced. -const POLL_TIMEOUT_NS: u64 = 2_000_000_000; +/// `timeout_nanos` for a wait with no clock (`syscall::inbox_submit`). +const FOREVER: u64 = u64::MAX; fn main() { match std::env::args().nth(1).as_deref() { @@ -106,23 +92,25 @@ fn run() { // A window is a connection promoted by its first frame, so a second // `MSG_CREATE_WINDOW` on one arrives with nothing to promote. The // compositor read that as its own bug. + waiting("a second create on a live window", "its window"); let doubled = Window::create(64, 64).expect("a window to send a second create on"); write_handle(doubled.handle(), &create_frame(), "a second create"); probe("a second create on a live window"); - // A clipboard frame with no region sent ahead of it. The receive is not a - // poll — a short batch is a peer that sent its frame first — so this must - // cost the client its connection and nothing else. - clipboard_shm(None, 64, "a clipboard frame with no region"); - probe("a clipboard frame with no region"); + // A commit with no copy begun: there is no region for it to name, so this + // must cost the client its connection and nothing else. + let commit = endow::service("compositor").expect("a connection to commit on"); + ipc::signal(commit.as_handle(), window::MSG_COPY_COMMIT).expect("send the commit"); + probe("a commit with no copy begun"); - // A region really sent, with a length no region can satisfy. The length - // decides how much of the region is read as clipboard text, so it is the - // compositor's to bound rather than the client's to choose. - let region = SharedMemory::create(4096).expect("a region of our own"); - let shared = region.share().expect("a second handle to it"); - clipboard_shm(Some(shared), u32::MAX, "a clipboard longer than any region"); - probe("a clipboard longer than any region"); + // A copy no region is made for. The length decides how large a region the + // compositor makes, so it is the compositor's to bound rather than the + // client's to choose. + let begin = endow::service("compositor").expect("a connection to begin on"); + begin + .send(window::MSG_COPY_BEGIN, &window::ClipboardShmMsg { len: u32::MAX }) + .expect("send the begin"); + probe("a copy longer than any clipboard"); // An inline clipboard one byte past what any client may inline. The // compositor keeps that one byte, so the frame is refusable here instead of @@ -139,41 +127,33 @@ fn run() { // `MSG_DESTROY_WINDOW` makes the compositor drop the connection, after // which the handle is permanently read-ready at EOF — so a `poll_event` that // did not latch answered `Close` for as long as anybody kept asking, and a - // client draining until `None` never got out. Two calls decide it. + // client draining until `None` never got out. Two calls decide it, and + // the second waits for nothing: a latched window answers `None` at once, + // and an unlatched one reads the end of the stream at once. + let what = "a window closed from the inside"; + waiting(what, "its window"); let mut ending = Window::create(64, 64).expect("a window to close from the inside"); ipc::signal(ending.handle(), window::MSG_DESTROY_WINDOW) .expect("ask the compositor to destroy this window"); - // Named rather than kept, because `Event` is not `Debug` and a failure - // here has to print the whole sequence it saw. - let mut seen: Vec<&'static str> = Vec::new(); - for _ in 0..POLLS_AFTER_CLOSE { - let name = match ending.poll_event(POLL_TIMEOUT_NS) { - None => "none", - Some(window::Event::Close) => "close", + waiting(what, "the window's Close"); + loop { + match ending.poll_event(FOREVER) { + Some(window::Event::Close) => break, // A frame the compositor had already sent can arrive first. It is // not what this case is about, and skipping it is not a weakening: // what follows still has to be close and then nothing. - Some(_) => "other", - }; - seen.push(name); - if name == "none" { - break; + Some(_) => {} + None => fail(&format!("[{what}] the connection went and the window never said so")), } } - let sequence = seen.join(","); - let Some(closed_at) = seen.iter().position(|n| *n == "close") else { + waiting(what, "the latched poll answering None"); + if ending.poll_event(FOREVER).is_some() { fail(&format!( - "[a window closed from the inside] the connection went and the window never said \ - so: {sequence}" - )); - }; - if seen.get(closed_at + 1) != Some(&"none") { - fail(&format!( - "[a window closed from the inside] the poll after Close answered again: {sequence} \ - — a client that drains until None cannot leave" + "[{what}] the poll after Close answered again — a client that drains until None \ + cannot leave" )); } - probe("a window closed from the inside"); + probe(what); println!("compositor client death: 6 deaths survived, compositor still serving"); } @@ -234,17 +214,6 @@ fn create_frame() -> Vec { frame } -fn clipboard_shm(region: Option, len: u32, what: &str) { - let conn = endow::service("compositor") - .unwrap_or_else(|e| fail(&format!("[{what}] the compositor is not serving: {e:?}"))); - let msg = window::ClipboardShmMsg { len }; - let sent = match region { - Some(h) => conn.send_with_handles(&[h], window::MSG_CLIPBOARD_SET_SHM, &msg), - None => conn.send(window::MSG_CLIPBOARD_SET_SHM, &msg), - }; - sent.unwrap_or_else(|e| fail(&format!("[{what}] could not send: {e:?}"))); -} - /// Every write here fits in the pipe it goes into, so a blocking `write` can /// only be the compositor's problem, never this binary's. fn write_handle(handle: toyos_abi::RawHandle, bytes: &[u8], what: &str) { @@ -257,32 +226,28 @@ fn write_handle(handle: toyos_abi::RawHandle, bytes: &[u8], what: &str) { } } -/// Ask the compositor something it always answers, and give it a deadline. +/// Ask the compositor something it always answers from its dispatch, so an +/// answer proves the event loop reached the end of a pass rather than merely +/// that the process still exists. fn probe(what: &str) { let conn: Connection = endow::service("compositor") .unwrap_or_else(|e| fail(&format!("[{what}] the compositor is not serving: {e:?}"))); if let Err(e) = ipc::signal(conn.as_handle(), window::MSG_GET_RESOLUTION) { fail(&format!("[{what}] could not ask the compositor for its resolution: {e:?}")); } - let mut buf = [0u8; 16]; - let mut got = 0; - for _ in 0..PROBE_POLLS { - match conn.read_nonblock(&mut buf[got..]) { - Ok(0) => fail(&format!("[{what}] the compositor closed the probe unanswered")), - Ok(n) => { - got += n; - if got == buf.len() { - return; - } - } - Err(SyscallError::WouldBlock) => syscall::nanosleep(PROBE_POLL_NS), - Err(e) => fail(&format!("[{what}] the probe could not be read: {e:?}")), - } + waiting(what, "the compositor's answer to a probe"); + let header = conn + .recv_header() + .unwrap_or_else(|e| fail(&format!("[{what}] the probe went unanswered: {e:?}"))); + if header.msg_type != window::MSG_RESOLUTION_CHANGED { + fail(&format!("[{what}] the probe was answered with message type {}", header.msg_type)); } - fail(&format!( - "[{what}] the compositor did not answer in {} ms — it is gone or its loop is parked", - PROBE_POLLS as u64 * PROBE_POLL_NS / 1_000_000, - )); +} + +/// Said before each wait on the compositor: the line a missing event leaves +/// last. +fn waiting(what: &str, awaited: &str) { + println!("compositor client death: [{what}] waiting for {awaited}"); } fn fail(msg: &str) -> ! { diff --git a/tests/toyos-rust-tests/src/bin/compositor_hostile_clipboard.rs b/tests/toyos-rust-tests/src/bin/compositor_hostile_clipboard.rs new file mode 100644 index 00000000000..2f8f453dcba --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/compositor_hostile_clipboard.rs @@ -0,0 +1,269 @@ +//! Needs a live compositor and a host that types GUI+V, which the shared boot +//! does not have — it is in `RUST_SKIP` and `metal_sim_hostile_clipboard` runs +//! it on the metal-sim profile. +//! +//! 1. **A wrong-typed handle.** A pipe end rides the retired region message. +//! The kernel ends whoever maps a pipe as shared memory, so the compositor +//! must refuse the client without ever receiving the handle — and the pipe's +//! writer, queued on the refused connection, must go back to the kernel +//! unused. +//! 2. **A copy that is not UTF-8.** The client commits a region of `0xFF`, and +//! a paste has to be the clipboard from before it. +//! 3. **A region rewritten after its commit.** Once the compositor has closed +//! the connection the region is the client's own again, and a paste has to +//! be what was committed, not what the region holds now. +//! 4. **A copy never committed.** The client holds its region and says nothing; +//! the compositor has to drop it by name. +//! 5. **A second begin.** A connection holding a region may send its commit and +//! nothing else, so a second `MSG_COPY_BEGIN` on it is refused rather than +//! answered with another region. +//! 6. **A begin with bytes past its length.** Refused rather than answered. +//! 7. **A commit with a payload.** Refused, so a paste has to be the clipboard +//! from before it and not the region's text. +//! 8. **A commit on a window.** A commit names a region held, so a window +//! sending one loses its connection. +//! +//! Each case ends with a probe the compositor answers from its dispatch. The +//! host asserts what this side cannot see: no handle fault and no compositor +//! exit in the kernel's records, and the refusals named. + +use std::sync::atomic::Ordering; + +use toyos::endow; +use toyos::ipc; +use toyos::shm::SharedMemory; +use toyos::{AsHandle, Connection}; +use toyos_abi::syscall; +use toyos_abi::RawHandle; +use window::{Event, Window}; + +// The wire as this client speaks it, spelled here rather than imported: the +// negative control builds this binary against a `window` that predates all +// four. +const RETIRED_CLIPBOARD_SET_SHM: u32 = 10; +const COPY_BEGIN: u32 = 13; +const COPY_COMMIT: u32 = 14; +const COPY_REGION: u32 = 13; +/// The longest copy the compositor makes a region for. +const COPY_LEN: usize = 2 * 1024 * 1024; + +/// The line the host answers with GUI+V, once. +const PASTE_MARKER: &str = "===HOSTILE_CLIPBOARD_PASTE==="; + +const BEFORE: &str = "hostile clipboard: the text before"; +const AFTER: &str = "hostile clipboard: the text after"; + +fn main() { + // First, so it has the focus: GUI+V pastes into the focused window. + waiting("the paste target", "its window"); + let mut target = Window::create_with_title(160, 120, "paste") + .unwrap_or_else(|e| fail("the paste target", &format!("no window: {e}"))); + target.present(); + waiting("the clipboard to start from", "the compositor taking it"); + window::clipboard_set(BEFORE) + .unwrap_or_else(|e| fail("the clipboard to start from", &e.to_string())); + probe("the clipboard to start from"); + + wrong_typed_handle(); + probe("a wrong-typed handle"); + + let what = "a copy that is not UTF-8"; + commit_filled(what, 0xFF); + probe(what); + let first = paste(&mut target, what); + if first != BEFORE.as_bytes() { + let len = first.len(); + fail(what, &format!("the paste was {len} bytes, not the clipboard from before")); + } + + let what = "a region rewritten after its commit"; + let region = commit_filled(what, b'C'); + // Text too, so a read of the region at the paste is pasted rather than + // refused. + fill(®ion, b'D'); + probe(what); + let second = paste(&mut target, what); + if second.len() != COPY_LEN || second.iter().any(|&b| b != b'C') { + fail(what, &describe(&second, b'C')); + } + + let what = "a copy never committed"; + let (conn, _region) = begin_copy(what); + await_hangup(conn.as_handle(), what, "the compositor giving up on the commit"); + probe(what); + + let what = "a second begin on a copy"; + let (conn, _region) = begin_copy(what); + conn.send(COPY_BEGIN, &window::ClipboardShmMsg { len: COPY_LEN as u32 }) + .unwrap_or_else(|e| fail(what, &format!("could not begin again: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor's refusal"); + probe(what); + + let what = "a begin with bytes past its length"; + let conn = connect(what); + let mut begin = (COPY_LEN as u32).to_ne_bytes().to_vec(); + begin.extend_from_slice(&[0; 4]); + conn.send_bytes(COPY_BEGIN, &begin) + .unwrap_or_else(|e| fail(what, &format!("could not begin: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor's refusal"); + probe(what); + + let what = "a commit with a payload"; + set_inline(what, AFTER); + let (conn, region) = begin_copy(what); + fill(®ion, b'E'); + conn.send_bytes(COPY_COMMIT, &[0; 4]) + .unwrap_or_else(|e| fail(what, &format!("no commit: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor's refusal"); + probe(what); + let third = paste(&mut target, what); + if third != AFTER.as_bytes() { + let len = third.len(); + fail(what, &format!("the paste was {len} bytes, not the clipboard from before")); + } + + // Last: the new window takes the focus the pastes went to. + let what = "a commit on a window"; + waiting(what, "its window"); + let committing = Window::create_with_title(64, 64, "commit") + .unwrap_or_else(|e| fail(what, &format!("no window: {e}"))); + ipc::signal(committing.handle(), COPY_COMMIT) + .unwrap_or_else(|e| fail(what, &format!("no commit: {e:?}"))); + ipc::signal(committing.handle(), window::MSG_GET_RESOLUTION) + .unwrap_or_else(|e| fail(what, &format!("no probe: {e:?}"))); + await_hangup(committing.handle(), what, "the compositor closing the window"); + probe(what); + + println!("hostile clipboard: every case survived, compositor still serving"); +} + +/// Put `text` on the clipboard inline, and wait for the compositor to be done +/// with it. +fn set_inline(what: &str, text: &str) { + let conn = connect(what); + conn.send_bytes(window::MSG_CLIPBOARD_SET, text.as_bytes()) + .unwrap_or_else(|e| fail(what, &format!("could not set the clipboard: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor closing the clipboard"); +} + +/// A pipe end where the retired message carried a region. +fn wrong_typed_handle() { + let what = "a wrong-typed handle"; + let ends = syscall::pipe().unwrap_or_else(|e| fail(what, &format!("no pipe: {e:?}"))); + let conn = connect(what); + conn.send_with_handles( + &[ends.write], + RETIRED_CLIPBOARD_SET_SHM, + &window::ClipboardShmMsg { len: 64 }, + ) + .unwrap_or_else(|e| fail(what, &format!("could not send: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor's refusal"); + // The send moved the only writer, so the reader hangs up exactly when the + // queue holding it is gone — and not while anything holds it. + await_hangup(ends.read, what, "the writer's return to the kernel"); + syscall::close(ends.read); +} + +/// Commit a whole copy of `byte`, and wait for the compositor to be done with +/// it. +fn commit_filled(what: &str, byte: u8) -> SharedMemory { + let (conn, region) = begin_copy(what); + fill(®ion, byte); + conn.signal(COPY_COMMIT).unwrap_or_else(|e| fail(what, &format!("no commit: {e:?}"))); + await_hangup(conn.as_handle(), what, "the compositor closing the copy"); + region +} + +/// A connection holding the region the compositor made for a whole copy. +fn begin_copy(what: &str) -> (Connection, SharedMemory) { + let conn = connect(what); + conn.send(COPY_BEGIN, &window::ClipboardShmMsg { len: COPY_LEN as u32 }) + .unwrap_or_else(|e| fail(what, &format!("could not begin: {e:?}"))); + waiting(what, "the compositor's region"); + let header = conn.recv_header().unwrap_or_else(|e| fail(what, &format!("no answer: {e:?}"))); + if header.msg_type != COPY_REGION || header.len() != 0 { + fail( + what, + &format!( + "the compositor answered with message type {} and {} bytes", + header.msg_type, + header.len() + ), + ); + } + let [region] = + conn.recv_handles_exact::<1>().unwrap_or_else(|| fail(what, "the answer had no region")); + let region = SharedMemory::adopt(region, COPY_LEN) + .unwrap_or_else(|e| fail(what, &format!("the region would not map: {e:?}"))); + (conn, region) +} + +fn fill(region: &SharedMemory, byte: u8) { + for b in region.as_atomic() { + b.store(byte, Ordering::Relaxed); + } +} + +/// Ask the host for GUI+V and return what the target is pasted. +fn paste(target: &mut Window, what: &str) -> Vec { + waiting(what, "the paste"); + println!("{PASTE_MARKER}"); + loop { + match target.recv_event() { + Event::ClipboardPaste(text) => return text, + Event::Close => fail(what, "the paste target's window was closed"), + _ => {} + } + } +} + +/// A paste, summarised — never printed whole. +fn describe(text: &[u8], fill: u8) -> String { + let stray = text.iter().position(|&b| b != fill); + format!( + "the paste was {} bytes, UTF-8: {}, first byte that is not {:?} at {stray:?}", + text.len(), + std::str::from_utf8(text).is_ok(), + fill as char + ) +} + +fn connect(what: &str) -> Connection { + endow::service("compositor") + .unwrap_or_else(|e| fail(what, &format!("the compositor is not serving: {e:?}"))) +} + +/// Said before every blocking wait: the line a missing event leaves last. +fn waiting(what: &str, awaited: &str) { + println!("hostile clipboard: [{what}] waiting for {awaited}"); +} + +/// Wait for the peer of `handle` to hang up, failing on anything it sends. +fn await_hangup(handle: RawHandle, what: &str, awaited: &str) { + waiting(what, awaited); + let mut byte = [0u8; 1]; + match syscall::read(handle, &mut byte) { + Ok(0) => {} + Ok(_) => fail(what, &format!("the peer answered where {awaited} was due")), + Err(e) => fail(what, &format!("waiting for {awaited}: {e:?}")), + } +} + +/// Ask the compositor something it always answers. +fn probe(what: &str) { + let conn = connect(what); + conn.signal(window::MSG_GET_RESOLUTION) + .unwrap_or_else(|e| fail(what, &format!("could not ask for the resolution: {e:?}"))); + waiting(what, "the compositor's answer to a probe"); + let header = conn + .recv_header() + .unwrap_or_else(|e| fail(what, &format!("the probe went unanswered: {e:?}"))); + if header.msg_type != window::MSG_RESOLUTION_CHANGED { + fail(what, &format!("the probe was answered with message type {}", header.msg_type)); + } +} + +fn fail(what: &str, msg: &str) -> ! { + eprintln!("hostile clipboard: [{what}] {msg}"); + std::process::exit(1); +} diff --git a/tests/toyos-rust-tests/src/bin/window_refusal.rs b/tests/toyos-rust-tests/src/bin/window_refusal.rs index 28bbaa3a021..59695620142 100644 --- a/tests/toyos-rust-tests/src/bin/window_refusal.rs +++ b/tests/toyos-rust-tests/src/bin/window_refusal.rs @@ -12,15 +12,20 @@ //! to take now. Instead it creates a port, builds a namespace mapping //! `"compositor"` to that port's connector, and spawns a child holding it: the //! child's `Window::create` reaches this process and nothing else, no other -//! process can see the service, and the same four answers are reachable from -//! one binary. That is the pattern every hostile-server test uses from here. +//! process can see the service. That is the pattern every hostile-server test +//! uses from here. //! //! Roles: no argument is the server; `client` is the child that asks for a //! window and decodes what comes back. +//! +//! No wait here has a clock: each side prints what it waits for and blocks on +//! it, so an answer that never comes is the harness's ceiling with its name on +//! the last line. use std::os::toyos::process::CommandExt; use std::process::{Command, Stdio}; +use toyos::endow::{self, EndowError}; use toyos::port::Acceptor; use toyos::AsHandle; use toyos::{ipc, namespace, port}; @@ -29,16 +34,29 @@ use window::{CreateError, Window}; const SELF_PATH: &str = "/system/bin/test_rs_window_refusal"; -/// The reply, and the `CreateError` the client must turn it into. `None` is -/// the "not an answer to this request at all" case. -const CASES: &[(Option, CreateError)] = &[ - (Some(window::REFUSED_AT_CAPACITY), CreateError::AtCapacity), - (Some(window::REFUSED_TOO_LARGE), CreateError::TooLarge), +/// What the stand-in compositor does with one `MSG_CREATE_WINDOW`. +#[derive(Clone, Copy, Debug)] +enum Reply { + Refuse(u32), + /// A frame that answers nothing this request asked. + Unrelated, + /// No answer: the connection closes. + Close, + /// `MSG_WINDOW_CREATED` with no buffer sent ahead of it. + NoBuffer, +} + +/// The reply, and the `CreateError` the client must turn it into. +const CASES: &[(Reply, CreateError)] = &[ + (Reply::Refuse(window::REFUSED_AT_CAPACITY), CreateError::AtCapacity), + (Reply::Refuse(window::REFUSED_TOO_LARGE), CreateError::TooLarge), // A reason from a newer compositor than this client. It must arrive as a // refusal carrying the raw value, not as a protocol error and not as a // window. - (Some(4242), CreateError::Refused(4242)), - (None, CreateError::Protocol(window::MSG_FRAME)), + (Reply::Refuse(4242), CreateError::Refused(4242)), + (Reply::Unrelated, CreateError::Protocol(window::MSG_FRAME)), + (Reply::Close, CreateError::BrokenOff), + (Reply::NoBuffer, CreateError::NoHandle), ]; fn main() { @@ -66,9 +84,11 @@ fn server() { .expect("window_refusal: spawn the client"); for (reply, _) in CASES { + println!("window refusal: [{reply:?}] waiting for the client's request"); serve_one(&acceptor, *reply); } + println!("window refusal: waiting for the client to exit"); let status = child.wait().expect("window_refusal: reap the client"); assert_eq!(status.code(), Some(0), "the client did not survive the refusals"); println!("{} refusal outcomes decoded, none panicked the client", CASES.len()); @@ -77,7 +97,7 @@ fn server() { /// Answer one `MSG_CREATE_WINDOW`, then drop the connection — which is what the /// compositor does after a refusal, and the reason the reply has to still be /// readable once the writer is gone. -fn serve_one(acceptor: &Acceptor, reply: Option) { +fn serve_one(acceptor: &Acceptor, reply: Reply) { let accepted = acceptor.accept().expect("accept a client"); let handle = accepted.as_handle(); let header = ipc::recv_header(handle).expect("request header"); @@ -85,19 +105,31 @@ fn serve_one(acceptor: &Acceptor, reply: Option) { let _req: window::CreateWindowRequest = ipc::recv_payload(handle, &header).expect("request payload"); match reply { - Some(reason) => { + Reply::Refuse(reason) => { ipc::send(handle, window::MSG_WINDOW_REFUSED, &window::WindowRefused { reason }) .expect("send the refusal"); } - None => { + Reply::Unrelated => { ipc::signal(handle, window::MSG_FRAME).expect("send a reply that answers nothing"); } + Reply::Close => {} + Reply::NoBuffer => { + let info = window::WindowInfo { width: 100, height: 100, stride: 100, pixel_format: 0 }; + ipc::send(handle, window::MSG_WINDOW_CREATED, &info) + .expect("send a window with no buffer"); + } } } fn client() { - for (reply, expected) in CASES { - let outcome = Window::create(100, 100); + // Every request is made before any is judged: the stand-in serves one per + // case, and a client that stopped early would leave it waiting on the next. + let mut outcomes = Vec::new(); + for (reply, _) in CASES { + println!("window refusal: [{reply:?}] waiting for the answer"); + outcomes.push(Window::create(100, 100)); + } + for ((reply, expected), outcome) in CASES.iter().zip(outcomes) { let got = match outcome { Ok(_) => panic!("reply {reply:?} produced a window"), Err(e) => e, @@ -107,4 +139,17 @@ fn client() { // compositor drops the connection the moment it has answered. assert!(!got.to_string().is_empty(), "{got:?} has no message"); } + + // A port whose queue is full, which the stand-in never drains: the kernel + // refuses the connection itself. + let mut queued = Vec::new(); + let refused = loop { + match endow::service("compositor") { + Ok(conn) => queued.push(conn), + Err(EndowError::Refused(e)) => break e, + Err(other) => panic!("filling the queue ended in {other:?}"), + } + }; + let got = Window::create(100, 100).err(); + assert_eq!(got, Some(CreateError::Kernel(refused)), "a full queue decoded wrongly"); } diff --git a/tests/toyos.rs b/tests/toyos.rs index 4cb11680e3a..29f1ca34a8d 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -309,6 +309,9 @@ const RUST_SKIP: &[&str] = &[ // Same again, and it also needs a host injecting pointer packets: // `metal_sim_window_drag` runs it. "window_drag", + // Same again, and it also needs a host typing GUI+V: + // `metal_sim_hostile_clipboard` runs it. + "compositor_hostile_clipboard", // Needs a compositor, a terminal and a shell: `desktop_window_child` // launches it from that shell. "window_child", @@ -703,6 +706,9 @@ const MACHINE_TESTS: &[(&str, Sched, Tier)] = &[ // slow on purpose. Its own boot too: it leaves the pointer somewhere else // and the window in a different place than it found them. ("metal_sim_window_drag", Sched::Serial, Tier::Nightly), + // Its own boot: the compositor it abuses has to be one nothing else has + // touched. + ("metal_sim_hostile_clipboard", Sched::Parallel, Tier::Fast), // A host-measured drain rate with an 8 s ceiling on a 3.3 s expectation. // Not gate A, but the same instrument: what it measures is how fast a // client's audio leaves the machine. @@ -1676,6 +1682,7 @@ const CARRIES: &[(&str, &[&str])] = &[ ("metal_sim_compositor_stall", METAL_SIM_CLIENTS), ("metal_sim_client_death", METAL_SIM_CLIENTS), ("metal_sim_window_drag", &["test_rs_window_drag"]), + ("metal_sim_hostile_clipboard", &["test_rs_compositor_hostile_clipboard"]), ("desktop_window_child", &["test_rs_window_child"]), ("toolkit_window_wake", &["test_rs_window_wake"]), ("toolkit_winit_loop", &["test_rs_winit_loop"]), @@ -7537,6 +7544,85 @@ fn metal_sim_window_drag(rust_bins: &[(String, Vec)]) -> Result<(), String> Ok(()) } +/// The guest runs the cases and asks for a paste after each copy; this half +/// types GUI+V at every ask and asserts what the guest cannot see. **The +/// kernel's record is the independent half**: a compositor that maps the pipe +/// is ended by the kernel with a handle fault, whatever the compositor believed +/// it was doing, so no such record may appear — and each refusal the guest +/// caused must be named by the compositor. +fn metal_sim_hostile_clipboard(rust_bins: &[(String, Vec)]) -> Result<(), String> { + /// The guest's `PASTE_MARKER`. + const PASTE: &str = "===HOSTILE_CLIPBOARD_PASTE==="; + let bins: Vec<(String, Vec)> = rust_bins + .iter() + .filter(|(name, _)| name == "compositor_hostile_clipboard") + .cloned() + .collect(); + if bins.is_empty() { + return Err("the compositor_hostile_clipboard client was not built".to_string()); + } + let config = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/metalcase"); + let options = + BootOptions { profile: qemu::Profile::Metal, qmp: true, ..Default::default() }; + metal_sim_argv_check(&qemu::profile_argv(&options))?; + let mut qemu = QemuInstance::boot_with_options(&config, &[], &bins, options); + + let result = qemu.run_test_paced( + "test_rs_compositor_hostile_clipboard", + Duration::from_secs(240), + |socket, line| { + if line.contains(PASTE) { + let socket = socket.expect("this boot was made with QMP"); + qemu::QmpInput::open(socket).keys(&[ + ("meta_l", true), + ("v", true), + ("v", false), + ("meta_l", false), + ]); + } + }, + ); + let text = format!("{}\n{}", result.stdout, result.serial); + + for death in ["handle fault:", "exit: compositor"] { + if let Some(line) = text.lines().find(|l| l.contains(death)) { + return Err(format!("the kernel ended the compositor: {}\n{text}", line.trim())); + } + } + if result.error.is_some() || result.exit_code != Some(0) { + let why = match &result.error { + Some(err) => err.to_string(), + None => String::from("it finished and its exit code is the finding"), + }; + return Err(format!( + "compositor_hostile_clipboard exited {:?}: {why}\n{text}", + result.exit_code + )); + } + if !text.contains("hostile clipboard: every case survived") { + return Err(format!("the guest did not report that every case survived:\n{text}")); + } + if !text.contains("it began a copy and never committed it") { + return Err(format!( + "the client that held its region and never committed was not dropped by name:\n{text}" + )); + } + // The compositor's `DropReason::Retired`, which no other case produces. + const RETIRED: &str = "it sent the retired clipboard region"; + if !text.lines().any(|l| l.contains("compositor: dropping client") && l.contains(RETIRED)) { + return Err(format!( + "the client that sent a pipe where a region went was not refused by name:\n{text}" + )); + } + const NOT_UTF8: [&str; 2] = ["compositor: refusing a clipboard from client", "it is not UTF-8"]; + if !text.lines().any(|l| NOT_UTF8.iter().all(|s| l.contains(s))) { + return Err(format!("the copy that is not UTF-8 was not refused by name:\n{text}")); + } + serial::Serial::named("boot console", result.serial.as_str()).must_be_clean()?; + eprintln!(" [metal-sim] a pipe refused unused"); + Ok(()) +} + /// How far above its content the host reaches for a window's title bar. /// /// Not the compositor's title-bar height — this is a probe, and what it needs @@ -11277,9 +11363,6 @@ fn metal_sim_client_death(boot: &mut Boot) -> Result<(), String> { )); } - // The one case whose verdict is a line rather than survival: a payload - // past what any client may inline is refused by name, because storing the - // prefix a frame reader keeps is the silent half of the same event. const OVERSIZE: &str = "compositor: refusing an inline payload past"; if !result.stdout.contains(OVERSIZE) { return Err(format!( @@ -11288,6 +11371,15 @@ fn metal_sim_client_death(boot: &mut Boot) -> Result<(), String> { result.stdout )); } + // A copy's length sizes the region the compositor makes, so a length past + // the clipboard is refused before any region exists. + const LONG_COPY: &str = "compositor: refusing a copy of 4294967295 bytes"; + if !result.stdout.contains(LONG_COPY) { + return Err(format!( + "a copy longer than any clipboard was not refused by name:\n{}", + result.stdout + )); + } // Still painting once every case is behind it, on a capture that starts // empty — so this counts frames produced *after* the last case. The @@ -15184,6 +15276,7 @@ fn run_machine_test( Ok(()) } "metal_sim_window_drag" => metal_sim_window_drag(rust_bins), + "metal_sim_hostile_clipboard" => metal_sim_hostile_clipboard(rust_bins), "metal_sim_pointer_churn" => { // The owner froze his desktop twice by plugging a mouse in and // pulling it out again, and the second freeze landed on the fourth diff --git a/toyos/src/shm.rs b/toyos/src/shm.rs index 03ad9bec45a..f81eec0e62a 100644 --- a/toyos/src/shm.rs +++ b/toyos/src/shm.rs @@ -9,6 +9,8 @@ //! The mapping goes away with the last handle, so dropping this is all the //! cleanup there is. +use core::sync::atomic::AtomicU8; + use toyos_abi::syscall::{self, SyscallError}; use crate::{AsHandle, OwnedHandle, RawHandle}; @@ -72,6 +74,13 @@ impl SharedMemory { pub fn as_mut_slice(&mut self) -> &mut [u8] { unsafe { core::slice::from_raw_parts_mut(self.ptr, self.size) } } + + /// The region as the one type that may alias memory a peer writes. + pub fn as_atomic(&self) -> &[AtomicU8] { + // SAFETY: the mapping is `size` bytes and lives as long as `self`, and + // `AtomicU8` has `u8`'s size and alignment. + unsafe { core::slice::from_raw_parts(self.ptr as *const AtomicU8, self.size) } + } } impl AsHandle for SharedMemory { diff --git a/userland/compositor/src/client.rs b/userland/compositor/src/client.rs index b2326912b5c..b2504f30f7b 100644 --- a/userland/compositor/src/client.rs +++ b/userland/compositor/src/client.rs @@ -7,7 +7,13 @@ //! it the same decision by not reading. Here a peer that stops halfway through //! a frame costs a buffer and a deadline, and one that will not take a message //! costs itself. +//! +//! **The compositor never receives a handle from a client.** A received handle's +//! kind is unknowable, and using one of the wrong kind ends the process that +//! used it — so every region a client writes is one the compositor made, and a +//! handle a client sends stays queued until its connection closes. +use std::sync::atomic::Ordering; use std::time::{Duration, Instant}; use toyos::shm::SharedMemory; @@ -41,17 +47,6 @@ pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(2); /// declare up to `ipc::MAX_FRAME_LEN` and the excess is discarded unread. pub const MAX_KEPT_PAYLOAD: usize = window::MAX_INLINE_PAYLOAD + 1; -/// The largest clipboard the compositor will hold for a client. -/// -/// Two things meet here. `MSG_CLIPBOARD_SET_SHM` carries a length the client -/// chooses and the compositor reads that many bytes out of a region the client -/// sent, so an unbounded length is a read past the mapping — and the kernel -/// rounds every shared region up to one 2 MiB page -/// (`object::shm::SharedMemObject::create`), which makes a page the largest -/// length that cannot leave the smallest region anybody can send. It is also policy: a clipboard is text somebody selected, -/// and a megabyte of it is already generous. -pub const MAX_CLIPBOARD_BYTES: usize = 2 * 1024 * 1024; - /// One client's inbound framing. pub type ClientRx = ipc::FrameRx; @@ -74,20 +69,30 @@ pub type Win = Window; /// A whole client message, off the connection and in memory. /// -/// `conn` is `Some` only for the first frame on a freshly accepted connection: -/// `MSG_CREATE_WINDOW` keeps it, and every other message type answers on it -/// and lets it close. +/// `conn` is `Some` only for a frame off a connection that is not a window: +/// `MSG_CREATE_WINDOW` keeps it, `MSG_COPY_BEGIN` puts it back with the region +/// it answered with, and every other message type answers on it and lets it +/// close. pub struct ClientFrame { pub handle: RawHandle, pub msg_type: u32, payload: [u8; MAX_KEPT_PAYLOAD], payload_len: usize, pub conn: Option, + /// The connection's [`PendingConn::copy`]. + pub copy: Option, } impl ClientFrame { pub fn new(handle: RawHandle, msg_type: u32) -> Self { - Self { handle, msg_type, payload: [0; MAX_KEPT_PAYLOAD], payload_len: 0, conn: None } + Self { + handle, + msg_type, + payload: [0; MAX_KEPT_PAYLOAD], + payload_len: 0, + conn: None, + copy: None, + } } pub fn set_payload(&mut self, bytes: &[u8]) { @@ -109,6 +114,28 @@ pub struct PendingConn { pub conn: Connection, pub rx: ClientRx, pub since: Instant, + /// The region a `MSG_COPY_BEGIN` was answered with. A connection holding + /// one may send `MSG_COPY_COMMIT` and nothing else, within + /// [`HANDSHAKE_TIMEOUT`] of `since`. + pub copy: Option, +} + +/// A region made for one client's copy, which that client maps and may still +/// be writing. +/// +/// **Read once, and only by being taken.** Its one method consumes it, so +/// nothing here can read the region twice or validate it in place. +pub struct CopyRegion(SharedMemory); + +impl CopyRegion { + pub fn new(region: SharedMemory) -> Self { + Self(region) + } + + /// Every byte, each read once, into memory no client can write. + pub fn take(self) -> Vec { + self.0.as_atomic().iter().map(|b| b.load(Ordering::Relaxed)).collect() + } } /// Why a client is going. @@ -126,6 +153,9 @@ pub enum DropReason { /// A frame no protocol here can produce. The next message boundary is /// unlocatable, so there is nothing to resynchronise to. OutOfProtocol, + /// `window::MSG_RETIRED_CLIPBOARD_SET_SHM`, whose handle this compositor + /// never takes. + Retired, /// Its pipe would not take a whole frame — an entire pipe of messages it /// has not read. NotReading, @@ -133,15 +163,19 @@ pub enum DropReason { Gone, /// Accepted, and never completed a first frame. HandshakeTimeout, + /// Given a region to copy into, and never committed it. + CopyTimeout, } impl DropReason { pub fn why(self) -> &'static str { match self { Self::OutOfProtocol => "it sent a frame this protocol cannot describe", + Self::Retired => "it sent the retired clipboard region, whose handle is never taken", Self::NotReading => "its pipe will not take another message and it is not reading", Self::Gone => "its connection is gone", Self::HandshakeTimeout => "it never finished its first message", + Self::CopyTimeout => "it began a copy and never committed it", } } } @@ -215,10 +249,6 @@ pub fn deliver(dead: &mut Vec, win: &Win, msg_type: u3 } /// [`deliver`] for a message whose payload names buffers that travel with it. -/// -/// The handles are moved whether or not the frame lands, so the caller has -/// already given them up — and a client dropped here drops the queue holding -/// them, which is what releases the region. pub fn deliver_with_handles( dead: &mut Vec, win: &Win, diff --git a/userland/compositor/src/main.rs b/userland/compositor/src/main.rs index de29f51c1dd..e0c199c0078 100644 --- a/userland/compositor/src/main.rs +++ b/userland/compositor/src/main.rs @@ -14,6 +14,8 @@ //! What is left in this file is the policy the other three read: the numbers //! that are decisions rather than derivations. +#![forbid(unsafe_code)] + mod client; mod render; mod session; diff --git a/userland/compositor/src/render.rs b/userland/compositor/src/render.rs index ef0696c175a..468977a1067 100644 --- a/userland/compositor/src/render.rs +++ b/userland/compositor/src/render.rs @@ -179,9 +179,7 @@ fn draw_window( } if let Some(b) = content_blit(win, clip) { - let buffer = unsafe { - std::slice::from_raw_parts(win.client.shm.as_ptr(), win.client.shm.len()) - }; + let buffer = win.client.shm.as_slice(); let offset = (b.src_y as usize * win.buf_w as usize + b.src_x as usize) * 4; surface.blit( b.dst.x0 as usize, @@ -333,27 +331,22 @@ pub fn scale_wallpaper( /// Render the cursor sprite (RGBA) into a 64x64 BGRA hardware cursor buffer. pub fn upload_cursor( fb: &toyos::FramebufferDev, - cursor_buf: *mut u8, + cursor_buf: &mut [u8], sprite: &sprite::Sprite, hw_cursor: bool, ) { let data = sprite.data(); let w = sprite.width(); let h = sprite.height(); - unsafe { - core::ptr::write_bytes(cursor_buf, 0, 64 * 64 * 4); - } + cursor_buf[..64 * 64 * 4].fill(0); for y in 0..h.min(64) { for x in 0..w.min(64) { let si = (y * w + x) * 4; let di = (y * 64 + x) * 4; - unsafe { - let dst = cursor_buf.add(di); - *dst = data[si + 2]; - *dst.add(1) = data[si + 1]; - *dst.add(2) = data[si]; - *dst.add(3) = data[si + 3]; - } + cursor_buf[di] = data[si + 2]; + cursor_buf[di + 1] = data[si + 1]; + cursor_buf[di + 2] = data[si]; + cursor_buf[di + 3] = data[si + 3]; } } if hw_cursor { diff --git a/userland/compositor/src/session.rs b/userland/compositor/src/session.rs index fff0fa135e3..bc64d560d9d 100644 --- a/userland/compositor/src/session.rs +++ b/userland/compositor/src/session.rs @@ -16,7 +16,7 @@ use toyos::poller::{Poller, READABLE}; use toyos::port::Acceptor; use toyos::shm::SharedMemory; use toyos::{ipc, system, AsHandle, FramebufferDev, Keyboard, Mouse}; -use toyos_abi::syscall::DeviceType; +use toyos_abi::syscall::{self, DeviceType}; use toyos_abi::RawHandle; use toyos_desktop::{ cursor_from_abs, cursor_style, fold_mouse, hit_test, key_action, set_mode, tab_action, Chrome, @@ -27,8 +27,8 @@ use window::Screen; use crate::client::{ announce, deliver, deliver_signal, deliver_with_handles, mark_dead, note_closed, note_opened, - Client, ClientFrame, ClientRx, Dead, DropReason, PendingConn, Win, HANDSHAKE_TIMEOUT, - MAX_CLIPBOARD_BYTES, MAX_KEPT_PAYLOAD, MAX_PENDING_CONNS, + Client, ClientFrame, ClientRx, CopyRegion, Dead, DropReason, PendingConn, Win, + HANDSHAKE_TIMEOUT, MAX_KEPT_PAYLOAD, MAX_PENDING_CONNS, }; use crate::render::{self, Assets, BackBuffer, SystemStats, TitleBarIcons}; use crate::stats::{FrameStats, FrameTotals}; @@ -96,9 +96,8 @@ pub struct Session { screen: Screen, back: BackBuffer, hw_cursor: bool, - /// Held because `cursor_buf` points into it. - _cursor_shm: SharedMemory, - cursor_buf: *mut u8, + /// The hardware cursor's 64x64 BGRA image. + cursor_shm: SharedMemory, cursors: Cursors, current_cursor: CursorStyle, @@ -172,9 +171,8 @@ impl Session { let back = BackBuffer::new(screen.width(), screen.height(), screen.pixel_format_raw()); let hw_cursor = fb_info.flags & FLAG_HARDWARE_CURSOR != 0; - let cursor_shm = SharedMemory::adopt(fb_info.cursor, 64 * 64 * 4) + let mut cursor_shm = SharedMemory::adopt(fb_info.cursor, 64 * 64 * 4) .expect("the cursor buffer the framebuffer claim just handed over"); - let cursor_buf = cursor_shm.as_ptr(); let cursors = Cursors { default: read_sprite("/system/share/icons/cursor-bold.svg", CURSOR_PX, [255, 255, 255]), resize: read_sprite( @@ -184,7 +182,7 @@ impl Session { ), crosshair: read_sprite("/system/share/icons/crosshair-simple-bold.svg", CURSOR_PX, [0, 0, 0]), }; - render::upload_cursor(&fb_dev, cursor_buf, &cursors.default, hw_cursor); + render::upload_cursor(&fb_dev, cursor_shm.as_mut_slice(), &cursors.default, hw_cursor); let font_data = std::fs::read("/system/share/fonts/JetBrainsMono-Regular-8x16.font") .expect("failed to read font"); @@ -257,8 +255,7 @@ impl Session { screen, back, hw_cursor, - _cursor_shm: cursor_shm, - cursor_buf, + cursor_shm, cursors, current_cursor: CursorStyle::Default, font, @@ -335,11 +332,11 @@ impl Session { // client's traffic. let now = Instant::now(); for p in self.pending.iter().filter(|p| now.duration_since(p.since) >= HANDSHAKE_TIMEOUT) { - eprintln!( - "compositor: dropping client {} — {}", - p.conn.as_handle().0, - DropReason::HandshakeTimeout.why() - ); + let reason = match p.copy { + Some(_) => DropReason::CopyTimeout, + None => DropReason::HandshakeTimeout, + }; + eprintln!("compositor: dropping client {} — {}", p.conn.as_handle().0, reason.why()); } self.pending.retain(|p| now.duration_since(p.since) < HANDSHAKE_TIMEOUT); @@ -373,27 +370,26 @@ impl Session { } fn keys(&mut self) { - let mut events = [window::KeyEvent::EMPTY; 8]; - let buf = unsafe { - std::slice::from_raw_parts_mut( - events.as_mut_ptr() as *mut u8, - std::mem::size_of_val(&events), - ) - }; + const EVENT: usize = std::mem::size_of::(); + let mut buf = [0u8; 8 * EVENT]; // Never read blocking here. The kernel wakes only when a report queued // an event, so readiness and `has_data` agree — but a blocking read on // an empty queue parks the compositor until the next real key, and one // spurious wake anywhere would freeze it. - let n = self.kb.read_nonblock(buf).unwrap_or(0); - for event in &events[..n / std::mem::size_of::()] { + let n = self.kb.read_nonblock(&mut buf).unwrap_or(0); + let (events, torn) = buf[..n].as_chunks::(); + assert!(torn.is_empty(), "compositor: the keyboard read {n} bytes, not whole events"); + for raw in events { + let event = ipc::decode_payload::(raw) + .expect("a chunk is exactly one event long"); let focused = self.stack.focused(); let action = - key_action((*event).into(), focused.map(|i| self.stack[i].mode), self.launcher_open); + key_action(event.into(), focused.map(|i| self.stack[i].mode), self.launcher_open); match action { KeyAction::Ignore => {} KeyAction::Forward => { if let Some(i) = focused { - deliver(&mut self.dead, &self.stack[i], window::MSG_KEY_INPUT, event); + deliver(&mut self.dead, &self.stack[i], window::MSG_KEY_INPUT, &event); } } KeyAction::CloseLauncher => { @@ -466,7 +462,7 @@ impl Session { self.current_cursor = wanted; render::upload_cursor( &self.fb_dev, - self.cursor_buf, + self.cursor_shm.as_mut_slice(), self.cursors.get(wanted), self.hw_cursor, ); @@ -655,7 +651,12 @@ impl Session { } Ok(conn) => { self.poller.watch(&conn, READABLE, conn.as_handle().0 as u64); - self.pending.push(PendingConn { conn, rx: ClientRx::new(), since: Instant::now() }); + self.pending.push(PendingConn { + conn, + rx: ClientRx::new(), + since: Instant::now(), + copy: None, + }); } } } @@ -688,14 +689,16 @@ impl Session { frame.set_payload(self.pending[i].rx.payload(payload_len)); // A connection is identified by its first frame and by // nothing else. `MSG_CREATE_WINDOW` promotes it to a - // window; anything else is a one-shot request, answered and - // closed — which is what an `endow::service` caller like - // `window::clipboard_set` expects. + // window; `MSG_COPY_BEGIN` puts it back to wait for its + // commit; anything else is a one-shot request, answered and + // closed. // // One promotion per pass keeps `i` meaningful across the // `remove`; the rest are re-armed below and served next // pass. - frame.conn = Some(self.pending.remove(i).conn); + let p = self.pending.remove(i); + frame.conn = Some(p.conn); + frame.copy = p.copy; out.push(frame); break; } @@ -726,7 +729,7 @@ impl Session { } fn dispatch(&mut self, frames: Vec) { - for frame in frames { + for mut frame in frames { let handle = frame.handle; // A payload filling the kept buffer declared more than any client // may inline, so what arrived is a prefix of what was sent. @@ -739,8 +742,22 @@ impl Session { mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); continue; } + // A connection holding a region may send its commit and nothing + // else, and a commit is only ever of a region held. + match (frame.copy.take(), frame.msg_type) { + (Some(region), window::MSG_COPY_COMMIT) => { + self.copy_commit(frame, region); + continue; + } + (Some(_), _) | (None, window::MSG_COPY_COMMIT) => { + mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); + continue; + } + (None, _) => {} + } match frame.msg_type { window::MSG_CREATE_WINDOW => self.create_window(frame), + window::MSG_COPY_BEGIN => self.copy_begin(frame), window::MSG_PRESENT => { let Ok(rect) = ipc::decode_payload::(frame.payload()) else { mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); @@ -761,8 +778,9 @@ impl Session { self.damage_all(); } } - window::MSG_CLIPBOARD_SET => { - self.clipboard = String::from_utf8_lossy(frame.payload()).into_owned(); + window::MSG_CLIPBOARD_SET => self.set_clipboard(handle, frame.payload().to_vec()), + window::MSG_RETIRED_CLIPBOARD_SET_SHM => { + mark_dead(&mut self.dead, handle, DropReason::Retired); } window::MSG_LAYOUT_CHANGED => { // The compositor is the root of the surface tree and @@ -776,38 +794,6 @@ impl Session { deliver_signal(&mut self.dead, win, window::MSG_LAYOUT_CHANGED); } } - window::MSG_CLIPBOARD_SET_SHM => { - let Ok(info) = - ipc::decode_payload::(frame.payload()) - else { - mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); - continue; - }; - // The length is the client's claim about how much of the - // region it sent is text, and past the region that is a - // read of somebody else's memory rather than a clipboard. - // The region itself is no longer a claim: it is a handle - // the client moved, so there is nothing left to disbelieve - // about which memory this is. - if info.len as usize > MAX_CLIPBOARD_BYTES { - eprintln!( - "compositor: refusing {} bytes of clipboard from client {}, max \ - {MAX_CLIPBOARD_BYTES}", - info.len, - handle.0 - ); - continue; - } - let Some([buffer]) = ipc::recv_handles_exact::<1>(handle) else { - mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); - continue; - }; - let Ok(shm) = SharedMemory::adopt(buffer, info.len as usize) else { - mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); - continue; - }; - self.clipboard = String::from_utf8_lossy(shm.as_slice()).into_owned(); - } window::MSG_SET_CURSOR => { let Ok(style) = ipc::decode_payload::(frame.payload()) else { mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); @@ -962,6 +948,88 @@ impl Session { self.damage_all(); } + /// `MSG_COPY_BEGIN`: make the region the client's text goes into, send it, + /// and hold the connection for its commit. + fn copy_begin(&mut self, frame: ClientFrame) { + let handle = frame.handle; + // Exactly one `ClipboardShmMsg`: bytes past it are not this protocol. + let exact = frame.payload().len() == std::mem::size_of::(); + let info = ipc::decode_payload::(frame.payload()); + let (Ok(info), Some(conn), true) = (info, frame.conn, exact) else { + mark_dead(&mut self.dead, handle, DropReason::OutOfProtocol); + return; + }; + let len = info.len as usize; + if !window::copy_fits(len) { + eprintln!( + "compositor: refusing a copy of {len} bytes from client {}, max {}", + handle.0, + window::MAX_CLIPBOARD_BYTES + ); + return; + } + let region = match SharedMemory::create(len) { + Ok(region) => region, + Err(e) => { + eprintln!( + "compositor: client {} gets no copy — no memory for {len} bytes ({e:?})", + handle.0 + ); + return; + } + }; + let theirs = match region.share() { + Ok(theirs) => theirs, + Err(e) => { + eprintln!( + "compositor: client {} gets no copy — its region cannot be shared ({e:?})", + handle.0 + ); + return; + } + }; + // Moved on its own, so that a refused move leaves `theirs` here to + // close: once moved, its number is no longer this process's to close. + if syscall::handle_send(conn.as_handle(), &[theirs]).is_err() { + syscall::close(theirs); + mark_dead(&mut self.dead, handle, DropReason::Gone); + return; + } + if let Err(e) = conn.try_signal(window::MSG_COPY_REGION) { + mark_dead(&mut self.dead, handle, e.into()); + return; + } + self.pending.push(PendingConn { + conn, + rx: ClientRx::new(), + since: Instant::now(), + copy: Some(CopyRegion::new(region)), + }); + } + + /// `MSG_COPY_COMMIT` on a connection holding `region`: take the text out + /// once, and let the connection close. + fn copy_commit(&mut self, frame: ClientFrame, region: CopyRegion) { + if !frame.payload().is_empty() { + mark_dead(&mut self.dead, frame.handle, DropReason::OutOfProtocol); + return; + } + self.set_clipboard(frame.handle, region.take()); + } + + /// The clipboard, from bytes that are the compositor's own: validated + /// only once no client can change them. + fn set_clipboard(&mut self, from: RawHandle, bytes: Vec) { + match String::from_utf8(bytes) { + Ok(text) => self.clipboard = text, + Err(e) => eprintln!( + "compositor: refusing a clipboard from client {} — it is not UTF-8 ({})", + from.0, + e.utf8_error() + ), + } + } + fn answer_resolution(&mut self, handle: RawHandle) { let reply = window::ResolutionInfo { width: self.fb_info.width, height: self.fb_info.height }; diff --git a/userland/editor/src/main.rs b/userland/editor/src/main.rs index 34293a33460..d35e80d2dfd 100644 --- a/userland/editor/src/main.rs +++ b/userland/editor/src/main.rs @@ -1610,13 +1610,20 @@ fn handle_key(editor: &mut Editor, key: &KeyPress, fb: &mut Framebuffer) { } Some('c') => { if let Some(text) = editor.selected_text() { - window::clipboard_set(&text).ok(); + if let Err(e) = window::clipboard_set(&text) { + eprintln!("editor: nothing was copied — {e}"); + } } } Some('x') => { if let Some(text) = editor.selected_text() { - window::clipboard_set(&text).ok(); - editor.delete_selection(); + // A cut whose copy was refused keeps its text. + match window::clipboard_set(&text) { + Ok(()) => { + editor.delete_selection(); + } + Err(e) => eprintln!("editor: nothing was cut — {e}"), + } } } Some('v') => {} // Paste handled via ClipboardPaste event diff --git a/userland/terminal/src/main.rs b/userland/terminal/src/main.rs index 793456fbd16..49cdc2f5f0e 100644 --- a/userland/terminal/src/main.rs +++ b/userland/terminal/src/main.rs @@ -38,6 +38,12 @@ fn present(console: &Console, window: &Window) { } } +fn copy(text: &str) { + if let Err(e) = window::clipboard_set(text) { + eprintln!("terminal: nothing was copied — {e}"); + } +} + fn main() { // **This terminal's surface is a port it makes, not a name it registers.** // One per instance: the connector goes into the namespace of the shell it @@ -148,7 +154,7 @@ fn main() { window::Event::KeyInput(key) if key.gui() && key.keycode == 0x06 => { // Cmd+C: copy selection to clipboard if let Some(text) = console.get_selection() { - window::clipboard_set(&text).ok(); + copy(&text); } } window::Event::KeyInput(key) => { @@ -175,7 +181,7 @@ fn main() { } window::MOUSE_RELEASE if ev.changed == 1 => { if let Some(text) = console.mouse_up(col, row) { - window::clipboard_set(&text).ok(); + copy(&text); } present(&console, &window); } diff --git a/userland/toyos-window/src/lib.rs b/userland/toyos-window/src/lib.rs index 6350e5853c6..393343c75d3 100644 --- a/userland/toyos-window/src/lib.rs +++ b/userland/toyos-window/src/lib.rs @@ -6,6 +6,7 @@ pub use framebuffer::{Color, Framebuffer, Screen, Traffic}; pub use wait::{Waiter, Waker, Woke}; /// What [`Window::handle`] answers with, and what a [`Waiter`] waits on. pub use toyos_abi::RawHandle; +use toyos_abi::syscall::SyscallError; use toyos::ipc; use toyos::AsHandle; @@ -61,10 +62,34 @@ pub const MSG_RESOLUTION_CHANGED: u32 = 8; /// another window has no move except to serve it or to drop the connection. pub const MSG_WINDOW_REFUSED: u32 = 9; -// Shared-memory clipboard (for payloads > 116 bytes) -pub const MSG_CLIPBOARD_SET_SHM: u32 = 10; +/// Client → compositor, retired: it carried a region the client chose, and the +/// compositor never uses a handle a client sent. A client sending it is dropped +/// by name. +pub const MSG_RETIRED_CLIPBOARD_SET_SHM: u32 = 10; +/// Compositor → client: a clipboard past [`MAX_INLINE_PAYLOAD`], in a region +/// the compositor made. Payload is a [`ClipboardShmMsg`]. pub const MSG_CLIPBOARD_PASTE_SHM: u32 = 11; +/// Client → compositor, as a connection's first frame whose payload is exactly +/// a [`ClipboardShmMsg`]: a clipboard of that many bytes, which [`copy_fits`]. +/// Answered with [`MSG_COPY_REGION`]. +pub const MSG_COPY_BEGIN: u32 = 13; +/// Client → compositor, bare, and the only frame a connection answered with +/// [`MSG_COPY_REGION`] may send next: the text is in the region. The compositor +/// 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. +pub const MSG_COPY_REGION: u32 = 13; + +/// The longest clipboard. Policy: a clipboard is text somebody selected. +pub const MAX_CLIPBOARD_BYTES: usize = 2 * 1024 * 1024; + +/// Whether a clipboard of `len` bytes is one the compositor makes a region for. +pub fn copy_fits(len: usize) -> bool { + (1..=MAX_CLIPBOARD_BYTES).contains(&len) +} + /// Either direction: [`surface::LAYOUT_CONFIG`] changed, re-read it. /// /// The compositor translates nothing, so it does not act on this — it is the @@ -89,17 +114,28 @@ toyos::ipc_payload! { } } -/// Why creating a window failed. +/// Why a request to the compositor failed: a window, or the exchange under a +/// [`CopyError`]. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CreateError { /// This program was given no `compositor` connector. A statement about /// what the manifest says this program holds, not about the machine — and /// never about timing: every port exists before any server runs. NotEndowed, - /// The compositor exited, or the connection died mid-request. + /// The compositor exited: its port is closed for good. CompositorGone, + /// The kernel refused a call of the exchange: the connection, a frame, or + /// the map of the region the compositor sent. + Kernel(SyscallError), + /// The exchange broke off: the compositor closed the connection, having + /// exited or refused (its log says which), or sent a frame this client + /// cannot read. + BrokenOff, /// The compositor answered, and not with anything this exchange allows. Protocol(u32), + /// The answer came without the one handle it carries: the compositor sent + /// none, or this process had no room to take it. + NoHandle, /// The compositor is already holding as many windows as it can afford. AtCapacity, /// The requested size is bigger than the screen it would be drawn on. @@ -115,12 +151,17 @@ impl std::fmt::Display for CreateError { match self { Self::NotEndowed => write!(f, "this program was given no compositor"), Self::CompositorGone => write!(f, "the compositor is gone"), + Self::Kernel(e) => { + write!(f, "the kernel refused the exchange with the compositor ({e:?})") + } + Self::BrokenOff => write!(f, "the compositor broke off the exchange"), + Self::NoHandle => write!(f, "the compositor's answer arrived without its handle"), Self::AtCapacity => write!(f, "the compositor is at its window limit"), Self::TooLarge => write!(f, "the window is larger than the screen"), Self::NoMemory => write!(f, "there is no memory for a window that size"), Self::Refused(reason) => write!(f, "the compositor refused (reason {reason})"), Self::Protocol(msg_type) => { - write!(f, "the compositor answered with message type {msg_type}") + write!(f, "the compositor's answer (type {msg_type}) is not one this exchange allows") } } } @@ -132,11 +173,26 @@ impl From for CreateError { fn from(e: EndowError) -> Self { match e { EndowError::NotEndowed => Self::NotEndowed, - EndowError::ServerGone | EndowError::Refused(_) => Self::CompositorGone, + EndowError::ServerGone => Self::CompositorGone, + EndowError::Refused(e) => Self::Kernel(e), } } } +impl From for CreateError { + fn from(e: ipc::IpcError) -> Self { + match e { + ipc::IpcError::Disconnected | ipc::IpcError::Malformed => Self::BrokenOff, + ipc::IpcError::Syscall(e) => Self::Kernel(e), + ipc::IpcError::TooLarge => { + unreachable!("every frame this crate sends is within ipc::MAX_FRAME_LEN") + } + } + } +} + +const _: () = assert!(MAX_INLINE_PAYLOAD <= ipc::MAX_FRAME_LEN as usize); + impl CreateError { fn from_wire(reason: u32) -> Self { match reason { @@ -163,8 +219,8 @@ toyos::ipc_payload! { pub h: u32, } - /// How much of the region that travels with this message is text. The - /// region itself is one transferred handle, sent ahead of the frame. + /// 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. pub struct ClipboardShmMsg { pub len: u32, } @@ -353,31 +409,61 @@ pub fn load_layout(translator: &mut Translator) { } } +/// Why [`clipboard_set`] put nothing on the clipboard. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CopyError { + /// Longer than [`MAX_CLIPBOARD_BYTES`]. + TooLong(usize), + /// The exchange with the compositor failed. + Compositor(CreateError), +} + +impl std::fmt::Display for CopyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TooLong(len) => { + write!(f, "{len} bytes is past the clipboard's {MAX_CLIPBOARD_BYTES}") + } + Self::Compositor(e) => e.fmt(f), + } + } +} + +impl std::error::Error for CopyError {} + +impl From for CopyError { + fn from(e: CreateError) -> Self { + Self::Compositor(e) + } +} + /// Put `text` on the system clipboard, over a connection of its own. /// -/// Fallible where it used to `expect`: a program the manifest gives no -/// compositor is a program that cannot copy, which is an answer and not a -/// reason to take the caller down. -pub fn clipboard_set(text: &str) -> Result<(), CreateError> { - use std::sync::Mutex; - static CLIPBOARD_SHM: Mutex> = Mutex::new(None); +/// Past [`MAX_INLINE_PAYLOAD`] the text goes into a region the compositor +/// made for it ([`MSG_COPY_BEGIN`]). +pub fn clipboard_set(text: &str) -> Result<(), CopyError> { + let bytes = text.as_bytes(); + if bytes.len() > MAX_INLINE_PAYLOAD && !copy_fits(bytes.len()) { + return Err(CopyError::TooLong(bytes.len())); + } + Ok(copy(bytes)?) +} +/// [`clipboard_set`]'s exchange, for text the clipboard holds. +fn copy(bytes: &[u8]) -> Result<(), CreateError> { let conn = endow::service("compositor")?; - let bytes = text.as_bytes(); if bytes.len() <= MAX_INLINE_PAYLOAD { - let _ = conn.send_bytes(MSG_CLIPBOARD_SET, bytes); - } else if let Ok(mut shm) = SharedMemory::create(bytes.len()) { - shm.as_mut_slice()[..bytes.len()].copy_from_slice(bytes); - if let Ok(handle) = shm.share() { - let _ = conn.send_with_handles( - &[handle], - MSG_CLIPBOARD_SET_SHM, - &ClipboardShmMsg { len: bytes.len() as u32 }, - ); - } - *CLIPBOARD_SHM.lock().unwrap() = Some(shm); + return Ok(conn.send_bytes(MSG_CLIPBOARD_SET, bytes)?); + } + conn.send(MSG_COPY_BEGIN, &ClipboardShmMsg { len: bytes.len() as u32 })?; + let header = conn.recv_header()?; + if header.msg_type != MSG_COPY_REGION || header.len() != 0 { + return Err(CreateError::Protocol(header.msg_type)); } - Ok(()) + let [region] = conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let mut region = SharedMemory::adopt(region, bytes.len()).map_err(CreateError::Kernel)?; + region.as_mut_slice().copy_from_slice(bytes); + Ok(conn.signal(MSG_COPY_COMMIT)?) } pub struct Window { @@ -432,32 +518,28 @@ impl Window { let len = bytes.len().min(30); req.title[..len].copy_from_slice(&bytes[..len]); req.title_len = len as u8; - conn.send(MSG_CREATE_WINDOW, &req).map_err(|_| CreateError::CompositorGone)?; + conn.send(MSG_CREATE_WINDOW, &req)?; // Header first, then the payload the message type calls for: the two // answers carry different structs, and a payload shorter than the type // it was asked for is a refusal from `recv_payload`, not a window. - let header = conn.recv_header().map_err(|_| CreateError::CompositorGone)?; + let header = conn.recv_header()?; match header.msg_type { MSG_WINDOW_CREATED => {} MSG_WINDOW_REFUSED => { - let refused: WindowRefused = conn - .recv_payload(&header) - .map_err(|_| CreateError::CompositorGone)?; + let refused: WindowRefused = conn.recv_payload(&header)?; return Err(CreateError::from_wire(refused.reason)); } other => return Err(CreateError::Protocol(other)), } - let info: WindowInfo = conn - .recv_payload(&header) - .map_err(|_| CreateError::CompositorGone)?; + let info: WindowInfo = conn.recv_payload(&header)?; let buf_size = info.stride as usize * info.height as usize * 4; // The buffer crossed ahead of 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::CompositorGone)?; - let shm = SharedMemory::adopt(buffer, buf_size).map_err(|_| CreateError::CompositorGone)?; + let [buffer] = conn.recv_handles_exact::<1>().ok_or(CreateError::NoHandle)?; + let shm = SharedMemory::adopt(buffer, buf_size).map_err(CreateError::Kernel)?; let poller = Poller::new(1); Ok(Self {