SYS_LOG_READ reads no loss count from its caller, and refuses a cursor ahead of a shard

issues/build/kernel-loom-without-loom-is-linted-by-no-clippy-run.md

@@ -7,15 +7,14 @@ opened: 2026-09-29
 # `kernel-loom` without `loom` is linted by no clippy run
 
 Both host shapes in `src/clippy.rs` build `kernel-loom` with its default
-`loom`, so its other arm — the three `cfg(not(feature = "loom"))` sites in
-`kernel-loom/src/lib.rs`, `tests/log_body_words.rs` and
-`tests/log_zeroed_init.rs` — is compiled by no shape. A `mem::forget` planted in
-the non-`loom` `percpu_fetch_add` leaves `cargo run -- --clippy` green.
+`loom`, so its other arm — the `cfg(not(feature = "loom"))` sites in
+`kernel-loom/src/lib.rs` and the test files gated that way — is compiled by no
+shape. A `mem::forget` planted in the non-`loom` `percpu_fetch_add` leaves
+`cargo run -- --clippy` green.
 
 `cargo clippy -p kernel-loom --no-default-features --all-targets -- -D warnings`
-exits 101 with twelve findings: `missing_safety_doc` on that `percpu_fetch_add`,
-and `new_without_default` on eleven kernel types' non-`loom` `const fn new`,
-which `kernel-loom` exports `pub` and the kernel binary does not. Their `loom`
-arms carry `#[allow(clippy::new_without_default)]`; these do not.
+exits 101: `missing_safety_doc` on that `percpu_fetch_add`, and
+`new_without_default` on each kernel type's non-`loom` `const fn new` that
+`kernel-loom` exports `pub` and the kernel binary does not.
 
 Exit: a shape builds `kernel-loom` without `loom`, and it is clean.

kernel-loom/src/lib.rs

@@ -186,6 +186,33 @@ pub use log_shard as shard;
 #[path = "../../kernel/src/log/registry.rs"]
 pub mod log_registry;
 
+/// The walks `SYS_LOG_READ` and the console drain take, driven by
+/// `tests/log_cursor.rs` over real shards; without `loom` alone, as is the
+/// [`shards`] shim it names.
+#[cfg(not(feature = "loom"))]
+#[path = "../../kernel/src/log/read.rs"]
+pub mod log_read;
+
+// The shards `read.rs` walks: in the kernel cpu0's and every AP's published
+// one, here whichever the calling test thread installed.
+#[cfg(not(feature = "loom"))]
+std::thread_local! {
+    static SHARDS: core::cell::Cell<[Option<&'static shard::Shard>; toyos_abi::log::MAX_LOG_SHARDS]> =
+        const { core::cell::Cell::new([None; toyos_abi::log::MAX_LOG_SHARDS]) };
+}
+
+/// What `read.rs` names as `super::shards()`.
+#[cfg(not(feature = "loom"))]
+pub fn shards() -> [Option<&'static shard::Shard>; toyos_abi::log::MAX_LOG_SHARDS] {
+    SHARDS.with(core::cell::Cell::get)
+}
+
+/// Install the shards this thread's walks see. No kernel counterpart.
+#[cfg(not(feature = "loom"))]
+pub fn install_shards(shards: [Option<&'static shard::Shard>; toyos_abi::log::MAX_LOG_SHARDS]) {
+    SHARDS.with(|cell| cell.set(shards));
+}
+
 #[path = "../../kernel/src/sched/reap_gate.rs"]
 pub mod reap_gate;
 

kernel-loom/tests/log_cursor.rs

@@ -0,0 +1,99 @@
+//! Host-fast regression for what `SYS_LOG_READ` takes from a caller's cursor.
+//!
+//! `kernel/src/log/read.rs` is the walk the syscall copies a caller's cursor
+//! into, compiled here over real shards. The cursor crossed the syscall
+//! boundary, so its loss count is never read and a position no shard has
+//! issued is refused.
+//!
+//! `--no-default-features`, for [`log_zeroed_init`]'s reason: the shards are the
+//! real ones, built the way the kernel builds an AP's.
+//!
+//! [`log_zeroed_init`]: ../log_zeroed_init.rs
+
+#![cfg(not(feature = "loom"))]
+
+use std::alloc::{alloc_zeroed, Layout};
+
+use kernel_loom::arch::IrqGuard;
+use kernel_loom::log_read::{drain_ordered, Cursor, RecordSink};
+use kernel_loom::log_shard::{Shard, FIRST_SEQ, SHARD_RECORDS};
+use toyos_abi::log::{LogCursor, LogRecord, MAX_LOG_SHARDS};
+
+/// A shard for the life of the test binary, which a walk names as `'static`.
+fn shard() -> &'static Shard {
+    // SAFETY: a `Shard` is not zero-sized; the block is never freed.
+    let ptr = unsafe { alloc_zeroed(Layout::new::<Shard>()) }.cast::<Shard>();
+    assert!(!ptr.is_null());
+    // SAFETY: fresh, zeroed, aligned and private to this test until it returns.
+    unsafe {
+        Shard::initialize_zeroed(ptr);
+        &*ptr
+    }
+}
+
+/// Commits `count` records to `shard`, each stamped with its own number.
+fn emit(shard: &Shard, count: u64) {
+    let guard = IrqGuard::close();
+    for _ in 0..count {
+        // SAFETY: this thread is the shard's only producer, and each number is
+        // committed once under the guard it was reserved with.
+        unsafe {
+            let seq = shard.reserve(&guard);
+            let mut record = LogRecord::EMPTY;
+            record.seq = seq;
+            record.at_ns = seq;
+            shard.commit(seq, &record, &guard);
+        }
+    }
+}
+
+/// A sink that takes nothing, so a walk only positions its cursor.
+struct Full;
+
+impl RecordSink for Full {
+    fn put(&mut self, _: &LogRecord) -> bool {
+        false
+    }
+}
+
+/// A cursor at every shard's start claiming `u64::MAX` records already lost,
+/// behind a shard that has overwritten some: the answer is the overwritten
+/// count, not the claim plus it.
+#[test]
+fn a_cursors_loss_claim_is_not_read() {
+    const OVERWRITTEN: u64 = 3;
+    let lapped = shard();
+    emit(lapped, SHARD_RECORDS as u64 + OVERWRITTEN);
+    let mut shards = [None; MAX_LOG_SHARDS];
+    shards[0] = Some(lapped);
+    kernel_loom::install_shards(shards);
+
+    let mut cursor = LogCursor { lost: u64::MAX, ..LogCursor::new() };
+    let mut walk = Cursor::from_reader(&cursor).expect("a cursor at every shard's start is taken");
+    drain_ordered(&mut walk, &mut Full);
+    walk.write_into(&mut cursor);
+    assert_eq!(cursor.lost, OVERWRITTEN);
+}
+
+/// A position past the number a shard issues next is refused, and one at it
+/// taken: a published shard's head, and an unpublished shard's first number.
+#[test]
+fn a_cursor_ahead_of_a_shard_is_refused() {
+    const RECORDS: u64 = 5;
+    let published = shard();
+    emit(published, RECORDS);
+    let mut shards = [None; MAX_LOG_SHARDS];
+    shards[0] = Some(published);
+    kernel_loom::install_shards(shards);
+
+    let taken = |shard: usize, next: u64| {
+        let mut cursor = LogCursor::new();
+        cursor.next[shard] = next;
+        Cursor::from_reader(&cursor).is_some()
+    };
+    let head = FIRST_SEQ + RECORDS;
+    assert!(taken(0, head), "a cursor caught up with a published shard");
+    assert!(!taken(0, head + 1), "a cursor past a published shard's head");
+    assert!(taken(1, FIRST_SEQ), "a cursor at an unpublished shard's first number");
+    assert!(!taken(1, FIRST_SEQ + 1), "a cursor past an unpublished shard's first number");
+}

kernel/src/log/read.rs

@@ -65,9 +65,13 @@ impl Cursor {
         Self { next: [FIRST_SEQ; MAX_LOG_SHARDS], lost: 0 }
     }
 
-    /// A caller's raw `LogCursor`, unvalidated: every field is clamped where it is used instead.
-    pub fn from_reader(cursor: &toyos_abi::log::LogCursor) -> Self {
-        Self { next: cursor.next, lost: cursor.lost }
+    /// A caller's `LogCursor` as a walk, or `None` for a position past the number its shard issues
+    /// next; its `lost` is never read, so the walk counts this read's loss alone.
+    pub fn from_reader(cursor: &toyos_abi::log::LogCursor) -> Option<Self> {
+        // An unpublished shard is held to the head it is published with, so no cursor is ahead of one that appears mid-walk.
+        let issued = |shard: Option<&'static Shard>| shard.map_or(FIRST_SEQ, Shard::head);
+        let ahead = cursor.next.iter().zip(super::shards()).any(|(&next, shard)| next > issued(shard));
+        (!ahead).then_some(Self { next: cursor.next, lost: 0 })
     }
 
     /// Writes the walk state back into the caller's `LogCursor`.
@@ -82,6 +86,7 @@ impl Cursor {
         let oldest = shard.oldest_readable();
         // Clamped to `FIRST_SEQ`: a zeroed cursor from the syscall boundary must not read as having missed everything.
         let want = self.next.get(i).copied().unwrap_or(FIRST_SEQ).max(FIRST_SEQ);
+        // Cannot overflow: `lost` starts at zero in every cursor and grows only as far as the clamp below moves `next[i]`, which never passes a head.
         self.lost += oldest.saturating_sub(want);
         let want = want.max(oldest);
         *self.next.get_mut(i)? = want;

kernel/src/log/user.rs

@@ -1,6 +1,6 @@
 //! Kernel side of `SYS_LOG_READ` and its readiness source.
 //!
-//! No per-reader state in a read: a cursor is the caller's own sequence numbers and loss count, copied in, walked, and copied back; readers coexist uncoordinated. Requires [`Rights::LOG`] on a `SysCap`, not ambient.
+//! No per-reader state in a read: a cursor is the caller's own sequence numbers, copied in, refused if ahead of a shard, walked, and copied back with this read's loss; readers coexist uncoordinated. Requires [`Rights::LOG`] on a `SysCap`, not ambient.
 //!
 //! [`Rights::LOG`]: toyos_abi::handle::Rights::LOG
 
@@ -52,7 +52,9 @@ pub fn read(
         return Err(SyscallError::InvalidArgument);
     }
 
-    let mut walk = Cursor::from_reader(cursor);
+    let Some(mut walk) = Cursor::from_reader(cursor) else {
+        return Err(SyscallError::InvalidArgument);
+    };
     let mut sink = UserRecords { out, written: 0, capacity };
     drain_ordered(&mut walk, &mut sink);
     let written = sink.written;

src/ci.rs

@@ -545,8 +545,6 @@ fn host(root: &Path) -> Vec<Step> {
             Err(failed.join("; "))
         }
     }));
-    // `log_zeroed_init` and `log_body_words` are gated `cfg(not(feature =
-    // "loom"))`, so the default invocation runs nothing from either.
     steps.push(step("kernel-loom without loom", || {
         cargo(root, &[
             "test",
@@ -557,6 +555,8 @@ fn host(root: &Path) -> Vec<Step> {
             "log_zeroed_init",
             "--test",
             "log_body_words",
+            "--test",
+            "log_cursor",
         ])
     }));
     for control in CONTROLS {

toyos-abi/src/log.rs

@@ -257,15 +257,17 @@ pub struct LogCursor {
     /// the first time and reads it back.
     pub shards: u32,
     pub _pad: u32,
-    /// In/out: cumulative records this cursor never saw because they were
-    /// overwritten.
+    /// Out: records this read skipped because they were overwritten. The
+    /// kernel never reads it, so a reader's total is the reader's own sum.
     ///
     /// **Derived, never counted by a producer.** The kernel computes it from
     /// `head` and `next`, which both have to be right anyway, so no counter can
     /// drift from the ring. It lives here so a reader that ignores loss has to
     /// actively ignore a field it is already passing.
     pub lost: u64,
-    /// In/out: the next sequence number wanted from each shard.
+    /// In/out: the next sequence number wanted from each shard. A number past
+    /// the one the shard issues next is refused, and a shard not yet published
+    /// issues 1 next.
     pub next: [u64; MAX_LOG_SHARDS],
 }
 

toyos-abi/src/syscall.rs

@@ -914,7 +914,8 @@ pub fn process_kill(proc: RawHandle) -> Result<(), SyscallError> {
 /// indexes by shift and the kernel does no length arithmetic. A buffer that
 /// cannot hold one record, or that cannot hold what the machine's shard count
 /// requires, is `InvalidArgument` — untrusted input that cannot be satisfied is
-/// refused, never truncated to fit.
+/// refused, never truncated to fit. So is a `cursor` ahead of a shard
+/// ([`crate::log::LogCursor::next`]).
 pub fn log_read(
     syscap: RawHandle,
     cursor: &mut crate::log::LogCursor,

toyos/src/log/mod.rs

@@ -69,6 +69,8 @@ use crate::AsHandle;
 /// line.
 pub struct LogTail {
     cursor: LogCursor,
+    /// Every read's `cursor.lost`, summed: the kernel answers each read's alone.
+    lost: u64,
 }
 
 impl Default for LogTail {
@@ -79,16 +81,12 @@ impl Default for LogTail {
 
 impl LogTail {
     pub const fn new() -> Self {
-        Self { cursor: LogCursor::new() }
+        Self { cursor: LogCursor::new(), lost: 0 }
     }
 
     /// Records this cursor never saw because a producer overwrote them.
-    ///
-    /// Cumulative and exact: the kernel derives it from the two numbers that
-    /// have to be right anyway, so it cannot drift from the ring the way a
-    /// producer-side counter would.
     pub fn lost(&self) -> u64 {
-        self.cursor.lost
+        self.lost
     }
 
     /// Shards the machine has, once a read has answered. Zero before that.
@@ -106,6 +104,7 @@ impl LogTail {
         out: &'a mut [LogRecord],
     ) -> Result<&'a [LogRecord], SyscallError> {
         let count = syscall::log_read(cap.as_handle(), &mut self.cursor, out)?;
+        self.lost += self.cursor.lost;
         Ok(&out[..count])
     }
 }

userland/logd/src/main.rs

@@ -479,7 +479,10 @@ impl Log {
                 // The one call this program is built around. A refusal is not
                 // survivable by retrying — the buffer and the rights are the
                 // same every time — so it ends loudly.
-                Err(e) => panic!("logd: SYS_LOG_READ refused a {BATCH}-record buffer ({e:?})"),
+                Err(e) => panic!(
+                    "logd: SYS_LOG_READ refused a {BATCH}-record buffer or a cursor ahead of a \
+                     shard ({e:?})"
+                ),
             };
             let short = batch.len() < BATCH;
             out.extend(