Skip to content

fix: validated rkyv access on every disk-read path - #66

Merged
pathscale merged 2 commits into
mainfrom
fix/validated-page-reads
Aug 1, 2026
Merged

fix: validated rkyv access on every disk-read path#66
pathscale merged 2 commits into
mainfrom
fix/validated-page-reads

Conversation

@pathscale

Copy link
Copy Markdown
Owner

A process that dies mid-write (crash, SIGKILL, an exit without draining) leaves torn pages behind, and access_unchecked read them back as archived values whose relative pointers dangle anywhere in the address space. The reader then died of SIGBUS in whatever touched them next — usually mid-write, tearing the store further. On 2026-08-01 that cascade corrupted one production store (agencyzero's) four times in a single day. Run against the preserved poisoned store, the validator now reports the smoking gun cleanly: InvalidSubtreePointer { address: 5202627079, size: 7679975808 } — a 7.6GB wild pointer that used to be a bus error.

Every access_unchecked on a disk-read path is now validated rkyv::access with bytecheck — including the six inside the Persistable derive templates, which is where the production store's UB actually fired (UnsizedIndexPageUtility::from_bytes). Torn pages now surface as errors naming corruption on fallible paths, or a named panic where the Persistable::from_bytes contract is infallible. Serialization paths and same-process roundtrip tests keep unchecked access: those bytes never crossed a process boundary.

Cost: CheckBytes bounds on the generic read paths (satisfied automatically by every derived Archive type) and validation work at load time only — the in-memory hot path is untouched. All 39 tests pass; consumers compile with six mechanical where-clause extensions (see pathscale/WorkTable#179 for the paired change and the kill-mid-write repro test that this PR turns from signal-death into clean refusal).

Versions bumped to 0.4.1 / derive 0.3.16, ready to publish after merge.

pathscale pushed a commit to pathscale/WorkTable that referenced this pull request Aug 1, 2026
Pairs with pathscale/DataBucket#66 (validated rkyv access on disk reads),
pulled from its branch via a crates-io patch until 0.4.1 is published.

The consumer-side cost of validation is six where-clause extensions in the
space-index layer: CheckBytes on the archived key types, satisfied
automatically by every derived Archive type.

The torn-shutdown repro is now split against two bars. The active test,
test_torn_store_fails_clean_never_by_signal, holds the invariant the fix
delivers: a store torn by five mid-write kills never takes a process down
with a signal — every load either succeeds or refuses with an error naming
corruption, proven by round-tripping the kills and scanning in-process
through an unwind boundary. The full bar, test_store_survives_torn_shutdowns,
stays ignored: a dangling index link into a zeroed data region still reads
as a phantom row of empty fields that validates perfectly, and only
crash-consistent writes (WAL, shadow paging, page checksums) can meet it.

Run against agencyzero's real poisoned store, the stack now reports
InvalidSubtreePointer as a clean error where it previously died of SIGBUS:
the corruption cascade (each SIGBUS a mid-write death planting the next
tear) is broken.
A process that dies mid-write (crash, SIGKILL, an exit without draining)
leaves torn pages, and access_unchecked read them back as archived values
whose relative pointers dangle anywhere in the address space. The reader
then died of SIGBUS in whatever touched them, usually mid-write, tearing
the store further: on 2026-08-01 that cascade corrupted one production
store four times in a day, and the diagnostic was a validation error
carrying a 7.6GB subtree pointer.

Every access_unchecked on the read path, including the six inside the
Persistable derive templates, is now rkyv::access with bytecheck: a torn
page surfaces as an error naming corruption (fallible paths) or a named
panic (the infallible Persistable::from_bytes contract), while the store
on disk stays exactly as readable as it was. Serialization paths and
same-process roundtrip tests keep unchecked access; the bytes never
crossed a process boundary there.

The cost is CheckBytes bounds on the generic read paths, satisfied
automatically by every derived Archive type, and validation work at load
time only.
@pathscale
pathscale force-pushed the fix/validated-page-reads branch from 1d065c2 to adb59f7 Compare August 1, 2026 13:14
…uilds

This crate also runs at nanosecond scale, where even background-task CPU
is budgeted and a laptop benchmark proves nothing. So the safety and the
speed stop competing: every disk read routes through one switch,
access_archived, which validates with bytecheck under the default
validate-reads feature and compiles back to the exact access_unchecked it
was before under default-features = false. The CheckBytes bounds stay
unconditional so the API does not shift under the flag; derived Archive
types satisfy them for free. The derive templates emit calls to the
switch, and the crate aliases itself so its own derives resolve the path.

Both configurations build, test (39 each) and lint clean.
@pathscale

Copy link
Copy Markdown
Owner Author

Addressing the performance concern for latency-critical consumers: a coarse benchmark cannot prove no-impact at the nanosecond scale WorkTable runs at in HFT, so validation is now a cargo feature instead of unconditional. validate-reads is on by default (a torn page is a named error); default-features = false compiles every read back to the exact access_unchecked it was before, zero cost. One switch (access_archived) carries the cfg, the derive templates emit calls to it, and the CheckBytes bounds stay unconditional so the API surface is identical either way. Both configurations build, test, and lint clean.

Structurally, even with validation on: the in-memory insert/select path is untouched (in_memory row access stays unchecked), and validation runs only at table load and inside the background persistence task's disk reads. In-memory writes are never blocked by persistence, before or after this PR.

@pathscale
pathscale merged commit 58aaf1b into main Aug 1, 2026
2 checks passed
@pathscale
pathscale deleted the fix/validated-page-reads branch August 1, 2026 13:29
pathscale pushed a commit to pathscale/WorkTable that referenced this pull request Aug 1, 2026
Pairs with pathscale/DataBucket#66 (validated rkyv access on disk reads),
pulled from its branch via a crates-io patch until 0.4.1 is published.

The consumer-side cost of validation is six where-clause extensions in the
space-index layer: CheckBytes on the archived key types, satisfied
automatically by every derived Archive type.

The torn-shutdown repro is now split against two bars. The active test,
test_torn_store_fails_clean_never_by_signal, holds the invariant the fix
delivers: a store torn by five mid-write kills never takes a process down
with a signal — every load either succeeds or refuses with an error naming
corruption, proven by round-tripping the kills and scanning in-process
through an unwind boundary. The full bar, test_store_survives_torn_shutdowns,
stays ignored: a dangling index link into a zeroed data region still reads
as a phantom row of empty fields that validates perfectly, and only
crash-consistent writes (WAL, shadow paging, page checksums) can meet it.

Run against agencyzero's real poisoned store, the stack now reports
InvalidSubtreePointer as a clean error where it previously died of SIGBUS:
the corruption cascade (each SIGBUS a mid-write death planting the next
tear) is broken.
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