Skip to content

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

Merged
Japabu merged 3 commits into
mainfrom
wt/toyos-logread
Oct 3, 2026
Merged

Japabu merged 3 commits into
mainfrom
wt/toyos-logread

Conversation

@Japabu

@Japabu Japabu commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

What changed, and why

The defect. SYS_LOG_READ copied the caller's LogCursor::lost into its walk (Cursor::from_reader) and added this read's loss to it (Cursor::open), in a kernel built with overflow-checks = true. A holder of Rights::LOG passing lost = u64::MAX with a next behind a shard's oldest record panicked the kernel. Measured before the fix, on the host (table below): attempt to add with overflow at kernel/src/log/read.rs:85:9.

The kernel reads no loss count. from_reader starts every caller's walk at lost = 0, so the reply's lost is this read's loss and the total is the reader's: LogTail sums it, and logd, its one consumer, reads it as before. Not taken: saturating_add, which clamps the lie into the reply; and checked_add with a refusal, which fires only when this read happens to lose something, after the walk has written the caller's buffer. A count the kernel only added to is bookkeeping userland can do, and a field it never reads cannot be a lie it trusts. The kernel's own += carries its invariant at the site: every cursor's lost starts at zero in the kernel and grows only as far as the clamp moves next[i], which never passes a head.

The rest of the cursor's contract, field by field.

  • next behind a shard's oldest record: clamped to it and counted as loss, as before. That is an honest slow reader.
  • next of zero: read as FIRST_SEQ, as before. That is the fresh cursor.
  • next past the number a shard issues next: taken unchecked until now. It reached no arithmetic: every use compares against the shard's own bounds, and the one += 1 follows a successful read below head. But the walk answered "nothing yet", and the reader skipped every record up to its claim with none counted lost. Now refused: from_reader answers None, the syscall answers InvalidArgument, and nothing is copied back. An unpublished shard is held to FIRST_SEQ, the head it is published with, so a shard published between the check and the walk has no position ahead of it. An honest cursor is never refused, because next[i] only ever becomes oldest_readable or a read sequence number plus one. Both are at most head, and head only grows.
  • shards is out only and never read. _pad is never read, and is copied back as the caller sent it.

logd's panic names both refusals. logd panics on any refusal of SYS_LOG_READ, and its text named only the buffer. The cursor refusal arrives as the same InvalidArgument, so the text now names a BATCH-record buffer or a cursor ahead of a shard. logd cannot tell the two apart, and an honest LogTail is never ahead, so a cursor refusal there would be a kernel bug.

The test's tier. kernel-loom/tests/log_cursor.rs compiles kernel/src/log/read.rs on the host over real 512-slot shards, built the way the kernel builds an AP's. It runs in the --no-default-features invocation that src/ci.rs already runs for log_zeroed_init and log_body_words. kernel-loom/src/lib.rs supplies the shards() the file names, per test thread. That reaches the whole defect: the cursor's conversion, the walk and the write-back. So no guest test and no metal row is added. The syscall glue around it is user.rs's let … else, plus machine.rs's copy-in and copy-out, which are unchanged. This test checks what reading cannot: the walk's arithmetic under rustc's overflow check over a real lapped ring, and the refusal's exact boundary against a real shard's head.

The issue. issues/build/kernel-loom-without-loom-is-linted-by-no-clippy-run.md listed the unlinted arm's sites and counted its findings. This branch adds a site (log_read and tests/log_cursor.rs) and a finding (new_without_default on read.rs's Published). The issue now names them by rule rather than by count. It no longer says that each of them has a loom arm carrying the allow: read.rs compiles only without loom, so its Published has none. Measured with the issue's own command (table below): base exits 101 with 10 findings and this branch with 11. With the issue's two lints allowed, the new shim and test add none.

Checks

High-risk: a security boundary and the ABI. Every exit is the command's own, and every log is under the orchestrator's job scratchpad, orch/. The patches are in a comment on this pull request, and each was applied with git apply --check, run, reverted, and the tree checked clean in the same script.

The fix is a1d850afd. b7a4534a7 is the review's named changes: a panic string in logd, and an issue sentence and three comments deleted. The image and the issue's command were run again at b7a4534a7. The host and guest runs are of a1d850afd, and were not run again.

Arm What runs Exit Log
Red on base cargo test --manifest-path kernel-loom/Cargo.toml --no-default-features --test log_cursor, the base-API test on origin/main (12bc37a67) 101: panicked at kernel/src/log/read.rs:85:9: attempt to add with overflow orch/logread/red-first-base.log
Green same, at a1d850afd 0, 2 passed orch/logread/mutations.log, its green section
Negative control: the whole fix reverted onto base git diff HEAD origin/main -- kernel/src/log/read.rs kernel/src/log/user.rs toyos-abi/src toyos/src applied, then the test 101, E0599: base's from_reader returns no Option. With the base-API test in its place: 101, the same overflow at read.rs:85:9 orch/logread/mutations.log
M1 lost: cursor.lost restored the test 101, attempt to add with overflow at read.rs:90:9 orch/logread/mutations.log
M2 the ahead check && false the test 101, "a cursor past a published shard's head" orch/logread/mutations.log
M3 > made >= the test 101, "a cursor caught up with a published shard" orch/logread/mutations.log
M4 an unpublished shard held to u64::MAX the test 101, "a cursor past an unpublished shard's first number" orch/logread/mutations.log
M5 an unpublished shard held to 0 the test 101, "a cursor at an unpublished shard's first number" orch/logread/mutations.log
Image cargo run -- --build-only, at b7a4534a7 0, logd rebuilt orch/logread-named/build-only.log
Host cargo run -- --ci host, at a1d850afd 0, 67 steps. kernel-loom without loom ran log_cursor's 2 tests orch/logread/ci-host.log
Guest cargo test --test toyos-build, at a1d850afd 0, 26 of 26, at host load average 49 to 59 on 14 cores orch/logread/guest-suite.log; load in guest-suite.load
The issue's command, on base cargo clippy -p kernel-loom --no-default-features --all-targets -- -D warnings 101, 10 findings orch/logread/clippy-noloom-base.log
The issue's command, at b7a4534a7 same 101, 11 findings: missing_safety_doc on percpu_fetch_add, new_without_default on ten const fn new, read.rs's Published among them orch/logread-named/clippy-noloom-head.log
The same, its two lints allowed, at b7a4534a7 … -- -D warnings -A clippy::missing_safety_doc -A clippy::new_without_default 101, drop_non_drop in log_body_words and log_zeroed_init only orch/logread-named/clippy-noloom-head-rest.log
The same, the shim and log_cursor alone, at b7a4534a7 cargo clippy -p kernel-loom --no-default-features --lib --test log_cursor -- -D warnings -A clippy::missing_safety_doc -A clippy::new_without_default 0 orch/logread-named/clippy-noloom-head-cursor.log

The guest suite reaches the change through logd. Every round calls SYS_LOG_READ and panics on a refusal, and the runner's ===READY=== reaches the console only through those rounds. So every boot that reached it had an honest cursor answered by this kernel.

Oracle. This ABI has no external specification or differential implementation. The expected values come from the ring's capacity, not from the code under test: a shard that issued SHARD_RECORDS + 3 records has overwritten exactly 3, and a shard that issued 5 has head 6. The base defect was caught by rustc's overflow check, the same check the kernel's toyos profile compiles in, and not by an assertion of mine.

Size. Net +163/−26. Production is +28/−16: kernel +12/−5, two of those lines comments; ABI docs +7/−4; SDK +5/−6; logd +4/−1. Tests and tooling are +135/−10: the kernel-loom shim +27, the test +99, src/ci.rs +2/−2, and the issue +7/−8.

What I am unsure of

  • Nothing tests the SDK's sum, self.lost += self.cursor.lost in LogTail::read. It calls a syscall, and the SDK's host build links none. It is one line, checked by reading only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8

…r ahead of a shard

The kernel copied a caller's `LogCursor::lost` into its walk and added this
read's loss to it, in a kernel built with `overflow-checks = true`: a holder
of `Rights::LOG` passing `lost = u64::MAX` with a `next` behind a shard's
oldest record panicked the kernel at `kernel/src/log/read.rs:85`. Measured
on the host before the fix, with `kernel/src/log/read.rs` compiled into
`kernel-loom` over a real shard lapped by three records:
`attempt to add with overflow` at that line.

The kernel no longer reads `lost` at all. `Cursor::from_reader` starts every
caller's walk at zero, so `lost` comes back as this read's loss, and the
running total is the reader's own: `LogTail` sums it. A count the kernel only
added to was bookkeeping userland can do, and a field the kernel never reads
cannot be a lie it trusts; no refusal is needed for it.

The cursor's other caller-supplied field, `next`, was also taken unchecked
from above: a position past the number a shard issues next walked as "nothing
yet", so that reader silently skipped records it was never counted as having
lost. `from_reader` now refuses it, and the syscall answers
`InvalidArgument`. An unpublished shard is held to `FIRST_SEQ`, the head it
is published with, so a shard that appears between the check and the walk
has no position ahead of it. `shards` is out only and `_pad` is not read.

`kernel-loom/tests/log_cursor.rs` drives the walk with a lying cursor on the
host, in the `--no-default-features` invocation `src/ci.rs` already runs for
the shard's other host-fast regressions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8
@Japabu

Japabu commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

The patches behind the Checks table, at head a1d850afd. Each was applied with git apply --check, run with cargo test --manifest-path kernel-loom/Cargo.toml --no-default-features --test log_cursor, reverted, and the tree checked clean in the same script.

Red on base: applied to origin/main (12bc37a), exit 101
diff --git a/kernel-loom/src/lib.rs b/kernel-loom/src/lib.rs
index 4f2c2cdb1..ffffe00f0 100644
--- a/kernel-loom/src/lib.rs
+++ b/kernel-loom/src/lib.rs
@@ -186,6 +186,34 @@ 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. A `//` comment for
+// the reason `WHO`'s is one.
+#[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;
 
diff --git a/kernel-loom/tests/log_cursor.rs b/kernel-loom/tests/log_cursor.rs
new file mode 100644
index 000000000..f029f26e8
--- /dev/null
+++ b/kernel-loom/tests/log_cursor.rs
@@ -0,0 +1,75 @@
+//! 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.
+//!
+//! `--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, 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);
+    drain_ordered(&mut walk, &mut Full);
+    walk.write_into(&mut cursor);
+    assert_eq!(cursor.lost, OVERWRITTEN);
+}
Mutation M1-lost-read-again, exit 101
--- a/kernel/src/log/read.rs	2026-10-03 17:51:55
+++ b/kernel/src/log/read.rs	2026-10-03 17:51:55
@@ -71,7 +71,7 @@
         // 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 })
+        (!ahead).then_some(Self { next: cursor.next, lost: cursor.lost })
     }
 
     /// Writes the walk state back into the caller's `LogCursor`.
Mutation M2-no-ahead-check, exit 101
--- a/kernel/src/log/read.rs	2026-10-03 17:51:55
+++ b/kernel/src/log/read.rs	2026-10-03 17:51:55
@@ -70,7 +70,7 @@
     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));
+        let ahead = cursor.next.iter().zip(super::shards()).any(|(&next, shard)| next > issued(shard)) && false;
         (!ahead).then_some(Self { next: cursor.next, lost: 0 })
     }
 
Mutation M3-caught-up-refused, exit 101
--- a/kernel/src/log/read.rs	2026-10-03 17:51:55
+++ b/kernel/src/log/read.rs	2026-10-03 17:51:55
@@ -70,7 +70,7 @@
     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));
+        let ahead = cursor.next.iter().zip(super::shards()).any(|(&next, shard)| next >= issued(shard));
         (!ahead).then_some(Self { next: cursor.next, lost: 0 })
     }
 
Mutation M4-unpublished-unbounded, exit 101
--- a/kernel/src/log/read.rs	2026-10-03 17:51:55
+++ b/kernel/src/log/read.rs	2026-10-03 17:51:55
@@ -69,7 +69,7 @@
     /// 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 issued = |shard: Option<&'static Shard>| shard.map_or(u64::MAX, 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 })
     }
Mutation M5-unpublished-zero, exit 101
--- a/kernel/src/log/read.rs	2026-10-03 17:51:55
+++ b/kernel/src/log/read.rs	2026-10-03 17:51:55
@@ -69,7 +69,7 @@
     /// 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 issued = |shard: Option<&'static Shard>| shard.map_or(0, 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 })
     }
Negative control: the whole fix reverted onto base, exit 101 (E0599)
diff --git a/kernel/src/log/read.rs b/kernel/src/log/read.rs
index ad3e65876..ecaaa8831 100644
--- a/kernel/src/log/read.rs
+++ b/kernel/src/log/read.rs
@@ -65,13 +65,9 @@ impl Cursor {
         Self { next: [FIRST_SEQ; MAX_LOG_SHARDS], lost: 0 }
     }
 
-    /// 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 })
+    /// 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 }
     }
 
     /// Writes the walk state back into the caller's `LogCursor`.
@@ -86,7 +82,6 @@ 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;
diff --git a/kernel/src/log/user.rs b/kernel/src/log/user.rs
index dfdd429ec..0d0c0a6ac 100644
--- a/kernel/src/log/user.rs
+++ b/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, 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.
+//! 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.
 //!
 //! [`Rights::LOG`]: toyos_abi::handle::Rights::LOG
 
@@ -52,9 +52,7 @@ pub fn read(
         return Err(SyscallError::InvalidArgument);
     }
 
-    let Some(mut walk) = Cursor::from_reader(cursor) else {
-        return Err(SyscallError::InvalidArgument);
-    };
+    let mut walk = Cursor::from_reader(cursor);
     let mut sink = UserRecords { out, written: 0, capacity };
     drain_ordered(&mut walk, &mut sink);
     let written = sink.written;
diff --git a/toyos-abi/src/log.rs b/toyos-abi/src/log.rs
index e6aed28d4..e7c7326d8 100644
--- a/toyos-abi/src/log.rs
+++ b/toyos-abi/src/log.rs
@@ -257,17 +257,15 @@ pub struct LogCursor {
     /// the first time and reads it back.
     pub shards: u32,
     pub _pad: u32,
-    /// 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.
+    /// In/out: cumulative records this cursor never saw because they were
+    /// overwritten.
     ///
     /// **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. A number past
-    /// the one the shard issues next is refused, and a shard not yet published
-    /// issues 1 next.
+    /// In/out: the next sequence number wanted from each shard.
     pub next: [u64; MAX_LOG_SHARDS],
 }
 
diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs
index 71b559378..e970422f2 100644
--- a/toyos-abi/src/syscall.rs
+++ b/toyos-abi/src/syscall.rs
@@ -914,8 +914,7 @@ 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. So is a `cursor` ahead of a shard
-/// ([`crate::log::LogCursor::next`]).
+/// refused, never truncated to fit.
 pub fn log_read(
     syscap: RawHandle,
     cursor: &mut crate::log::LogCursor,
diff --git a/toyos/src/log/mod.rs b/toyos/src/log/mod.rs
index 694db89b1..3e6ae8904 100644
--- a/toyos/src/log/mod.rs
+++ b/toyos/src/log/mod.rs
@@ -69,8 +69,6 @@ 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 {
@@ -81,16 +79,16 @@ impl Default for LogTail {
 
 impl LogTail {
     pub const fn new() -> Self {
-        Self { cursor: LogCursor::new(), lost: 0 }
+        Self { cursor: LogCursor::new() }
     }
 
     /// Records this cursor never saw because a producer overwrote them.
     ///
-    /// Cumulative and exact: the kernel derives each read's from the two
-    /// numbers that have to be right anyway, so it cannot drift from the ring
-    /// the way a producer-side counter would.
+    /// 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.lost
+        self.cursor.lost
     }
 
     /// Shards the machine has, once a read has answered. Zero before that.
@@ -108,7 +106,6 @@ 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])
     }
 }
Applied after NC-fix-reverted: the test made the base-API one, exit 101 (overflow at read.rs:85:9)
--- a/kernel-loom/tests/log_cursor.rs
+++ b/kernel-loom/tests/log_cursor.rs
@@ -2,8 +2,7 @@
 //!
 //! `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.
+//! boundary, so its loss count is never read.
 //!
 //! `--no-default-features`, for [`log_zeroed_init`]'s reason: the shards are the
 //! real ones, built the way the kernel builds an AP's.
@@ -16,7 +15,7 @@
 
 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 kernel_loom::log_shard::{Shard, 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`.
@@ -69,31 +68,8 @@
     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");
+    let mut walk = Cursor::from_reader(&cursor);
     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");
-}

@Japabu

Japabu commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Review, round 1, head a1d850afd.

Merge: git merge-tree --write-tree is clean against origin/main 12bc37a67 and against the merge queue's head d6daaaa22 (#688). Of the local branches in flight, wt/toyos-folders2 (3bbd89a1c, no pull request yet) moves kernel-loom/ to kernel/loom/. It conflicts in kernel/loom/src/lib.rs and leaves tests/log_cursor.rs at the old path, so whichever branch lands second moves the shim, the test and the src/ci.rs step. wt/toyos-rename (997fffc7d) merges cleanly as text. It renames logd, which this diff's source never names, though the PR body names it.

Net lines: git diff --shortstat origin/main...a1d850afd gives 9 files, +167 −24.

  • Production: +27 −14 (kernel +12 −5, toyos-abi +7 −4, SDK +8 −5).
  • Tests and tooling: +140 −10 (shim +28, test +99, src/ci.rs +5 −2, issue +8 −8).

The production growth is accepted: it is the refusal and its let … else, and the loss total moves out of the kernel into the SDK.

The high-risk claims were read against orch/logread/:

  • red-first-base.log: exit 101, overflow at read.rs:85:9.
  • mutations.log: green exits 0. The control with the whole fix reverted exits 101 with E0599. With the base-API test it exits 101 with the overflow at read.rs:85:9. M1 to M5 each exit 101 on their own assertion, and the tree is clean after each.
  • ci-host.log: EXIT=0, "67 step(s), all green", and log_cursor ran 2 tests and both passed.
  • guest-suite.log: EXIT=0, "26 passed, 26 total".

For an independent check, the ring's capacity sets the expected values and rustc's overflow check is the oracle. Callers of LogCursor, log_read and LogTail were searched across the tree, rust/library and ~/.cargo/git/checkouts. There are no others besides logd and inbox_log_post, and neither changes.

BLOCKER
(none)

NOTE

  • PR body, Checks table: no row names its log. Main's record should name each row's log, as The panic console takes no input, and an isa claim grants a process exact ports #592 does. Also, the cargo run -- --build-only row's run (orch/logread/build-only.log, "Build finished" at 15:50:34 UTC) ran before the head commit was made (15:51:38 UTC). Re-run it at a1d850afd or drop the row, since the guest suite built every kernel at the head.
  • userland/logd/src/main.rs:479-482: the panic text names only the buffer, but this branch adds a second refusal (a cursor ahead of a shard) under the same InvalidArgument. "What I am unsure of" in the PR body is not a record. File it in issues/ with an owner and an exit, or have the orchestrator widen the fence. wt/toyos-rename moves this file.
  • issues/build/kernel-loom-without-loom-is-linted-by-no-clippy-run.md:17-19: "Their loom arms carry #[allow(clippy::new_without_default)]" is false of the finding this branch adds. read.rs's Published has no loom arm: read.rs compiles only without loom, and its count of new_without_default is 0, against 1 in each of the other eight files. The paragraph was rewritten by this branch, and the record must stay true of what it describes.

REMOVE

  • kernel-loom/src/lib.rs:197-198: "A // comment for the reason WHO's is one." This sentence explains the comment's own syntax and is none of the three kinds.
  • src/ci.rs:548-550: this comment was corrected when it should have been deleted. It restates the --test list below it.
  • toyos/src/log/mod.rs:89-91: this paragraph was corrected when it should have been deleted. Line 87 already says what lost() counts, and the doc on LogCursor::lost carries the derivation.

LAND AFTER NAMED CHANGES

…py issue stays true

Review round 1 of #693 named these.

- userland/logd/src/main.rs: the panic on a refusal named only the
  buffer, but SYS_LOG_READ now also refuses a cursor ahead of a shard
  under the same InvalidArgument. The text names both.
- issues/build/kernel-loom-without-loom-is-linted-by-no-clippy-run.md:
  "Their loom arms carry #[allow(clippy::new_without_default)]" was false
  of read.rs's Published, which compiles only without loom and so has no
  loom arm. The sentence is deleted.
- kernel-loom/src/lib.rs, src/ci.rs, toyos/src/log/mod.rs: a sentence
  about a comment's own syntax, a comment restating the --test list
  below it, and a paragraph on LogTail::lost that line 87 and
  LogCursor::lost already carry, deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8
@Japabu
Japabu marked this pull request as ready for review October 3, 2026 16:26
@Japabu
Japabu enabled auto-merge October 3, 2026 16:26
Brings in #689 (virt_mask_windows judges the whole console), the fix
for the guest / suite red on this branch, with the rest of main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8
@Japabu
Japabu added this pull request to the merge queue Oct 3, 2026
Merged via the queue into main with commit 39c5fd5 Oct 3, 2026
3 checks passed
@Japabu
Japabu deleted the wt/toyos-logread branch October 3, 2026 17:37
Japabu added a commit that referenced this pull request Oct 3, 2026
- userland/logkeeper/src/main.rs: #693's SYS_LOG_READ refusal, which names
  a cursor ahead of a shard, under the `logkeeper:` head.
- src/sourcegate.rs: #687's two staged lines say `netstack: MAC`, as the
  program does.
- #687's new issue cites userland/netstack/src/main.rs:1605 and
  userland/netstack/src/dhcp.rs:149, the lines its netd citations now hold.
- #687's edit to the issue on the supervisor's claim of a PCI function
  lands on its renamed slug.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8
Japabu added a commit that referenced this pull request Oct 3, 2026
Carries #693 onto the moved loom package. Its `kernel-loom/tests/log_cursor.rs`
lands at `kernel/loom/tests/log_cursor.rs`, unchanged. Its `log_read` block in
the loom crate's root (the module, the `SHARDS` thread-local, `shards()` and
`install_shards()`, each `cfg(not(feature = "loom"))`) is taken whole, between
`log_registry` and `reap_gate`, with the module's `#[path]` re-rooted to
`../../src/log/read.rs`: main's `../../kernel/src/log/read.rs` resolves to
`kernel/kernel/src/log/read.rs` from `kernel/loom/src/`. `reap_gate` keeps this
branch's `../../src/sched/reap_gate.rs`.

`src/ci.rs` and `toyos-abi/src/syscall.rs` merged without a conflict; the
"kernel-loom without loom" step names `--test log_cursor` beside the two it had.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant