diff --git a/kernel/src/object/port.rs b/kernel/src/object/port.rs index a0bd6b47e30..add11bdc67c 100644 --- a/kernel/src/object/port.rs +++ b/kernel/src/object/port.rs @@ -139,13 +139,15 @@ impl Connector { } /// One lock acquisition for the check and the insert; see [`PortQueue`]. - pub fn push(&self, connection: PendingConnection) -> Result<(), PushError> { + /// A refusal hands the connection back: its caller holds a handle table's + /// lock, and the pipe ends are not dropped under one. + pub fn push(&self, connection: PendingConnection) -> Result<(), (PendingConnection, PushError)> { let mut queue = self.shared.queue.lock(); if queue.closed { - return Err(PushError::Closed); + return Err((connection, PushError::Closed)); } if queue.pending.len() >= MAX_PENDING_CONNECTIONS { - return Err(PushError::QueueFull); + return Err((connection, PushError::QueueFull)); } queue.pending.push_back(connection); Ok(()) diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index c3dacf6690c..bd932c05bac 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -533,8 +533,12 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> sys_namespace_open(RawHandle(a1 as u32), &name) } SYS_TLS_ALLOC_BLOCK => sys_tls_alloc_block(a1), - // ctx carries the copy-out: sys_inbox_setup writes its answer only once setup succeeds. - SYS_INBOX_SETUP => sys_inbox_setup(&ctx, a1 as u32, a2), + SYS_INBOX_SETUP => { + // Taken before the inbox exists: a bad address must not leave a handle the caller was never told. + let len = core::mem::size_of::() as u64; + let Some(mut out) = ctx.user_bytes_mut(UserAddr::new(a2), len) else { return bad_addr }; + sys_inbox_setup(a1 as u32, &mut out) + } SYS_INBOX_SUBMIT => { sys_inbox_submit(RawHandle(a1 as u32), a2 as u32, a3 as u32, a4) } diff --git a/kernel/src/syscall/ipc.rs b/kernel/src/syscall/ipc.rs index 6b62d9b6d42..b16b2760c70 100644 --- a/kernel/src/syscall/ipc.rs +++ b/kernel/src/syscall/ipc.rs @@ -2,8 +2,10 @@ //! that name them, connections, shared memory, and inboxes. //! No peer is addressed by name or pid: ports, namespaces and connections are //! built only from connectors and handles the caller already holds. -//! Every all-or-nothing claim here is structural: names and connectors are -//! resolved and checked before anything is installed or removed. +//! Every all-or-nothing claim here is structural: whatever can refuse is +//! resolved and checked before anything is installed or removed, or under the +//! hold that installs it. No handle is taken back across a released hold: a +//! sibling thread can close or move one the moment the table's lock is given up. use alloc::vec::Vec; @@ -26,17 +28,27 @@ pub(super) fn sys_pipe() -> u64 { let (reader, writer) = pipe::create(); let read_end = KObjectRef::PipeRead(crate::object::pipe::PipeReadEnd::new(reader)); let write_end = KObjectRef::PipeWrite(crate::object::pipe::PipeWriteEnd::new(writer)); - process::with_process_data(|data| { - let Ok(read_h) = ops::install(&mut data.handles, read_end) else { - return SyscallError::ResourceExhausted.to_u64(); - }; - let Ok(write_h) = ops::install(&mut data.handles, write_end) else { - ops::close(&mut data.handles, read_h, &mut data.pipe_maps) - .expect("the read end this call installed a moment ago"); - return SyscallError::ResourceExhausted.to_u64(); - }; - ((read_h.0 as u64) << 32) | write_h.0 as u64 - }) + install_pair(read_end, write_end) +} + +/// Install two objects or neither; the two handles come back packed into one word, which cannot be read as an error. +fn install_pair(first: KObjectRef, second: KObjectRef) -> u64 { + let installed = process::with_process_data(|data| { + // Room for both before either: a refused pair installs nothing, so nothing is taken back. + if !data.handles.has_room(2) { + return Err((first, second)); + } + let mut install = + |object| ops::install(&mut data.handles, object).expect("room was asked for first"); + let first = install(first); + let second = install(second); + Ok(((first.0 as u64) << 32) | second.0 as u64) + }); + match installed { + Ok(word) => word, + // A refused pair drops here, with the hold given up. + Err(_pair) => SyscallError::ResourceExhausted.to_u64(), + } } /// Map a pipe's ring page into the caller, tracked against the pipe so @@ -87,22 +99,10 @@ pub(super) fn sys_connection_join(rx_h: RawHandle, tx_h: RawHandle) -> u64 { process::with_process_data(|data| handle_result(ops::install(&mut data.handles, object))) } -/// Make a port and install both ends; needs no right and grants none, since a port with no clients is not authority. The two handles come back packed into one word, which cannot be read as an error. +/// Make a port and install both ends; needs no right and grants none, since a port with no clients is not authority. pub(super) fn sys_port_create() -> u64 { let (acceptor, connector) = port::create(); - process::with_process_data(|data| { - let Ok(a) = ops::install(&mut data.handles, KObjectRef::Acceptor(acceptor)) else { - return SyscallError::ResourceExhausted.to_u64(); - }; - let install_c = - ops::install(&mut data.handles, KObjectRef::Connector(connector)); - let Ok(c) = install_c else { - // The acceptor goes back so a refused pair leaves no orphaned port half. - drop(data.handles.remove(a)); - return SyscallError::ResourceExhausted.to_u64(); - }; - ((a.0 as u64) << 32) | c.0 as u64 - }) + install_pair(KObjectRef::Acceptor(acceptor), KObjectRef::Connector(connector)) } /// Build a namespace from a base's kept names plus new bindings; a refusal leaves the caller's table unchanged (every name resolved and connector checked first). @@ -240,35 +240,37 @@ fn connect_through(connector: &port::Connector) -> u64 { // Cross-wired here only: the server's end is built from the same two queues when it accepts. let (to_server, to_client) = crate::object::service::ConnectionEnd::pair_queues(); - // Client's end installed first: queuing before that would let a server accept a connection whose client has no handle. let object = KObjectRef::Connection(crate::object::service::ConnectionEnd::new( sc_reader, // client reads from server→client cs_writer, // client writes to client→server to_client.clone(), // and receives what the server sent to_server.clone(), )); - let h = match process::with_process_data(|data| ops::install(&mut data.handles, object)) { - Ok(h) => h, - Err(e) => return e.to_u64(), - }; - - let queued = connector.push(port::PendingConnection { + let pending = port::PendingConnection { rx: cs_reader, // server reads from client→server tx: sc_writer, // server writes to server→client inbox: to_server, outbox: to_client, stamp: connector.stamp(), + }; + // One hold from the room to the install: the port can still refuse, and the client's end is in the table only once it has not. + let installed = process::with_process_data(|data| { + if !data.handles.has_room(1) { + return Err((SyscallError::ResourceExhausted, object, pending)); + } + match connector.push(pending) { + Ok(()) => Ok(ops::install(&mut data.handles, object).expect("room was asked for first")), + Err((pending, port::PushError::Closed)) => Err((SyscallError::Gone, object, pending)), + Err((pending, port::PushError::QueueFull)) => { + Err((SyscallError::ResourceExhausted, object, pending)) + } + } }); - if let Err(e) = queued { - process::with_process_data(|data| { - ops::close(&mut data.handles, h, &mut data.pipe_maps) - .expect("the connection this call installed a moment ago"); - }); - return match e { - port::PushError::Closed => SyscallError::Gone.to_u64(), - port::PushError::QueueFull => SyscallError::ResourceExhausted.to_u64(), - }; - } + let h = match installed { + Ok(h) => h, + // Both halves of a refused connection drop here, with the hold given up: their pipes go with them. + Err((e, _client, _server)) => return e.to_u64(), + }; // The port's one watch carries both: the server blocked in `accept` and every ring polling it. connector.port().watch().post(); h.0 as u64 @@ -440,12 +442,8 @@ pub(super) fn sys_handle_recv( } } -/// Make an inbox and tell the caller where it is; the inbox owns its page and only this mapping may name it. -pub(super) fn sys_inbox_setup(ctx: &SyscallContext, depth: u32, out: u64) -> u64 { - let out = match UserAddr::checked(out) { - Some(addr) => addr, - None => return SyscallError::InvalidArgument.to_u64(), - }; +/// Make an inbox and tell the caller where it is; the inbox owns its page and only this mapping may name it. `out` is the window the dispatch took for the answer. +pub(super) fn sys_inbox_setup(depth: u32, out: &mut crate::user_ptr::UserBytesMut) -> u64 { let (inbox, vaddr) = match crate::inbox::create(depth) { Ok(v) => v, Err(e) => return e.to_u64(), @@ -457,17 +455,8 @@ pub(super) fn sys_inbox_setup(ctx: &SyscallContext, depth: u32, out: u64) -> u64 Ok(h) => h, Err(e) => return e.to_u64(), }; - let answer = toyos_abi::syscall::InboxSetup { handle, _pad: 0, vaddr }; - match ctx.copy_out(out, &answer) { - Ok(()) => 0, - Err(e) => { - process::with_process_data(|data| { - ops::close(&mut data.handles, handle, &mut data.pipe_maps) - .expect("the inbox this call installed a moment ago"); - }); - e.to_u64() - } - } + out.write_at(0, toyos_abi::usersafe::bytes(&InboxSetup { handle, _pad: 0, vaddr })); + 0 } pub(super) fn sys_inbox_submit( diff --git a/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs b/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs index 997807ac353..375560b152a 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs @@ -25,6 +25,9 @@ //! the kernel's own `self` fills. So a caller's entry labelled `self` is //! refused before anything moves, and so is a vector of `MAX_ENDOWMENTS` //! entries, while one entry fewer starts. +//! +//! **A pipe and a port are two inserts answered as one**: at the cap, and one +//! slot short of fitting, each is refused with the table as it was. use toyos_abi::syscall::{ self, EndowEntry, MmapFlags, MmapProt, SpawnArgs, SyscallError, MAX_ENDOWMENTS, MAX_SLOT_MAP, SELF_LABEL, @@ -160,6 +163,8 @@ fn main() { "handle table reached {n} slots, past the {MAX_HANDLES} cap" ); + a_pair_is_refused_whole(&mut filled); + let last = filled.pop().expect("the fill installed a handle"); let entry = EndowEntry { label_off: 0, label_len: LABELS.len() as u32, handle: last, _pad: 0 }; let spawned = spawn_endowed(&[entry], LABELS); @@ -180,6 +185,33 @@ fn main() { println!("handle table capped at {MAX_HANDLES} on every insert path (refused at {n})"); } +/// A pipe and a port are two handles each: refused at the cap, and refused +/// with one slot free, which is then still free. Takes a full table and +/// leaves it full. +fn a_pair_is_refused_whole(filled: &mut Vec) { + let refused = |room: &str| { + assert_eq!( + syscall::pipe().err(), + Some(SyscallError::ResourceExhausted), + "a pipe with {room}" + ); + assert_eq!( + syscall::port_create().err(), + Some(SyscallError::ResourceExhausted), + "a port with {room}" + ); + }; + refused("no slot free"); + syscall::close(filled.pop().expect("the fill installed a handle")); + refused("one slot free"); + filled.push(syscall::dup(RawHandle(1)).expect("the one free slot survived two refused pairs")); + assert_eq!( + syscall::dup(RawHandle(1)), + Err(SyscallError::ResourceExhausted), + "a refused pair left a second slot free" + ); +} + /// `SYS_SPAWN` of this binary as a child that exits at once, carrying /// `entries` into the blob `labels`. fn spawn_endowed(entries: &[EndowEntry], labels: &[u8]) -> Result {