An IPC setup no longer takes back a handle it installed: connect, inbox setup, pipe and port

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(())

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::<InboxSetup>() 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)
         }

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(

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<RawHandle>) {
+    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<RawHandle, SyscallError> {