[Japabu]

Storage stage B2 (design rev 13 §5): the bcachefs engine's reader and mkfs, which replace lib/bcachefs/src/upstream/.

Draft. B1 (#1025) and K1 (#1029), which this builds on, have landed; the branch is rebased onto main: two commits, then round 2's one.

Problem

lib/bcachefs/src/upstream/ was a read side for clean volumes written from upstream's GPL headers, by its own statement. The storage design replaces it with an engine written clean-room, and needs a ToyOS writer of a volume upstream's own fsck accepts before any later stage can be judged.

What it does

lib/bcachefs/src/engine implements metadata version 1.39 as bcachefs-tools v1.39.4 defines it, on one device. It was written only from the analyst's specification (A1–A10), the design and the black-box results, with btree ids and key-type numbers from the published Principles of Operation's lists (§11.3, §11.5), as the specification directs. No upstream source, provenance file or upstream/ code was read; the deleted module was opened only to find its one caller, tests/upstream_fixture.rs, deleted with it. Round 2 adds A11, the analyst's answers to the gaps B2 found.

• Reader (Volume): opens a clean volume; the superblock copy is chosen as A1.11 says and validated once. An encrypted volume's master key is unwrapped from its passphrase through K1's scrypt, at a cost the caller bounds. Btrees are walked from clean's roots with every bset's crc32c or ChaCha20/Poly1305 MAC checked before it is decrypted. On top: inodes, lookup by SipHash slot with probing, readdir by hash cursor, readlink, and reads over inline data and checked extents. Data checksums are type 5 (crc32c without inversion), type 1, or the 80- and 128-bit MACs. Every key is checked against its btree's allowed types: strict types on read, as upstream reads, and the full table in the writer and fsck.

Refused by name:

• an unclean volume (replay is B5), compression, reflink, erasure coding;
• more than one device, or a snapshotted root subvolume;
• other name hashes, nodes past 256 KiB, symlinks past 4 KiB, key types v1.39.4 does not define;
• on an encrypted volume, any node or extent without a MAC.
• mkfs writes §3.6's minimal complete volume, ROOT's (crc32c) or DATA's (ChaCha20/Poly1305, wide_macs, the key wrapped under a passphrase with the given scrypt cost):
  • three superblock copies and the layout copy;
  • an empty journal, and clean at sequence 0;
  • the root subvolume, snapshot, snapshot tree and root directory, with no lost+found;
  • the inode cursor at 2³¹;
  • alloc keys for used buckets only, freespace over the rest, a fragmentation LRU entry for a part-filled btree bucket, a backpointer per node, and A7.18's accounting;
  • the member's btree bitmap over every node.
• ToyOS fsck v0 (fsck): §3.6's superblock rules, identical copies but for offset and checksum, the root world as mkfs writes it, every node inside the bitmap, every key of a type its btree holds, and no bset or inode stamped with a journal sequence past clean's.
• toyos-crypto gains SipHash, moved from toyos-net-wire (which re-exports it), and public chacha::xor, poly1305::tag and poly1305::verify. The engine composes A10's construction from these; it is not RFC 8439's AEAD.

Decisions

• Every btree a v1 volume writes gets a root at mkfs, empty ones included. The node count then only grows with the keys its own nodes add: alloc keys, backpointers, the LRU entry and accounting. The layout is that count's least fixed point, computed from below and asserted monotone. Without empty roots an LRU btree that appears and disappears with a bucket's fill could oscillate.
• clean's sequence is 0, and every stamp is 0. BB-A1-1 asked whether that is accepted on B2's image: it is (below), and upstream resumes the journal from it.
• mkfs takes every identifier and key from the caller's Random, and every time from the caller. Its bytes are a function of both, and a test asserts it. B4's reproducible ROOT relies on this.
• Journal buckets are not zeroed. A clean mount does not read them, and a stale entry cannot match the magic derived from a fresh internal UUID.

Checks

Check	Result
cargo run -- --ci host at 62873ce, which runs every test below and clippy with the adopted lints	exit 0, 101 of 101 steps green
cargo run -- --build-only at 62873ce	exit 0

Host tests, reading upstream's own bytes:

• crc32c.img.gz: its whole tree, inline data, the symlink and directory link counts, and seq.txt read whole through its fourteen type-5 extents. The fixture's stored 0x7080C241 and A11's check values hold, and standard CRC-32C would give 0xE4E4C9C4 instead.
• encrypted.img.gz (80-bit MACs, crc64) and encrypted-wide-macs.img.gz (128-bit, crc128): unlocked and read whole, seq.txt byte for byte with every extent's MAC verified. A wrong passphrase, a locked open and scrypt past the caller's memory are each refused. A flipped byte in an extent, or in a node, fails its MAC.
• mkfs's volumes at 8 MiB, 32 MiB (alloc in three leaves under an interior root) and 1 GiB (two nodes per bucket, so an LRU entry): read back empty, ToyOS fsck clean.
• The encrypted volume unlocks from its 64-hex-character secret at N = 2, r = p = 1.
• A lost primary superblock is read from another copy and reported. Identical inputs give identical bytes.
• fsck finds a bset, and an inode, stamped past clean. A node without a MAC on an encrypted volume is refused.

Negative controls (patch applied, built, run, restored): extent MAC verification skipped; the bset stamp comparison dropped; the inode stamp comparison dropped; the encrypted-volume MAC requirement on nodes dropped. Each turns its test red.

The upstream judge: bcachefs-tools v1.39.4 (commit d997ad76e906, clean tree, built from its tag) under QEMU/HVF on Ubuntu 26.04.1 arm64, kernel 7.0.0-38. It always ran fsck -n -K < /dev/null, judged by §3.4's rule: exit 0 and no failure marker.

• B2's volumes pass: ROOT's at 32 MiB and 1 GiB, and DATA's at 64 MiB and 1 GiB, the latter unlocked with bcachefs unlock -k session -f pass in fsck's session keyring. Each fresh volume's run is recovering from clean shutdown, journal seq 0 through every pass, with nothing else printed.
• BB-A1-1 on B2's image: on ROOT's volumes, an upstream FUSE mount with ls, then one that writes seq 1 40000 and a file, then a read-back; on DATA's, the write and the read-back. Each passes the judge after, and every read-back is contents-ok. Upstream resumed the journal from 0. Sequence 0 with no stamps above it is settled.
• BB-A10-2 and BB-A9-3 on B2's volume: the hex secret unlocks it at the smallest scrypt parameters; fsck with stdin closed passes; the passphrase through stdin passes; a wrong secret is Error: incorrect passphrase, exit 1; with no key fsck exits 1 (error reading passphrase).
• BB-A8-1: no lost+found, exit 0.
• The judge's control: BB-A9-1's set rerun on a fresh upstream format. bad1k, bad1c, bad2, bad5, bad6, bad7 and zero each fail. bad1c exits 0 and is caught by btree node read error and checksum error alone. bad5's first run did not apply its damage, because my position parse missed, so kvdb refused and the image stayed clean. Rerun with the parse fixed, it fails.
• The same damage done to B2's own volumes fails: kill_btree_node -e keys and -e csum on the inodes node (the latter exit 0, markers only), a flipped key byte, an alloc counter and a deleted accounting key. On the encrypted volume, a broken MAC and a flipped byte fail too. Both untouched volumes pass.

Evidence: b2/ on pr-1032, commit 6263267d8161825dd9c3faf207abac029f79abb4: every command's stdout, stderr and exit status, the guest's build, the generator of the judged volumes and their hashes, which mkfs at 62873ce reproduces byte for byte, and the control patches.

No guest test runs here: B2 changes no booted behaviour. The loader and the file server still use the interim format, and the SipHash net-wire uses is the same function, moved.

Round 2 (A11 and review r1), head e51e660

• Type 5: read and verified, as above. issues/the-bcachefs-reader-refuses-crc32c-data-extents.md is closed.
• Key types: A11.7's table is enforced: the writer asserts it, the reader enforces it with A11.8's read-side exemption for non-strict types, and fsck checks the full table. Interior keys must be pointers of size 0, which B2's writer already wrote (A11.10).
• Blocker 1: each hostile input the review found is now refused at its boundary, with a module test:
  • a members field shorter than its header;
  • overflowing journal counts;
  • a link count past 2³²;
  • a dirent at the last offset;
  • a symlink past 4 KiB, before its allocation;
  • a copy offset past the device, which fsck reports.
• Fuzz: a structured host test mutates crc32c.img.gz and mkfs's unencrypted and encrypted volumes, about 5 s in all. It sweeps every superblock header word and each field's first 128 bytes with boundary values in all copies, gives nodes seeded random bytes and words, reseals checksums, and runs the reader and fsck over each mutation, which must never panic. The encrypted fixtures are left out because their default scrypt costs too much per mutation.
  • Its control: reverting the copy-offset fix makes it panic at that site in 0.4 s.
  • Reverted one at a time, the members, journal, link-count and dirent fixes do not trip it. Their inputs need structure a byte-level mutation rarely forms: a one-word members field, for instance, breaks the field walk before members are decoded. Their module tests hold them.
• Blocker 2: a root subvolume whose snapshot is not u32::MAX is refused, with a test.
• Suggestions taken:
  • a corrupt log2 N is reported as corrupt;
  • mkfs refuses a passphrase ending in a newline;
  • a MAC-typed node on an unencrypted volume is corrupt.
• Mutations: these four each turn their test red:
  • type 5 computed with CRC-32C's inversions;
  • the reader's strict-type check dropped;
  • fsck's key-type check dropped;
  • the writer's assertion dropped.
• Judge: mkfs's output is byte-identical at e51e660, so round 1's upstream judgement stands.
• Checks: cargo run -- --ci host at e51e660: exit 0, 101 of 101 steps green. cargo run -- --build-only: exit 0. Evidence: b2-r2/ on pr-1032, commit 19a01c971e8d130ea00567d7219204e0963101c6.

Unresolved

• The reader has no node cache: every lookup reads from the root. B5 owns the metadata cache.
• fsck v0 does not yet derive alloc, backpointers or accounting from the nodes (B3 grows it). Upstream's check_allocations compared them on every judged volume.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q

[Japabu]

Review of #1032 (storage B2), round 1, head 62873ce

Architecture

The ownership fits the approved design (rev 13 §3.6, §5 row B2, §6). lib/bcachefs/src/engine is the format's only owner. Bytes become typed values in superblock, node/bkey and value, and the reader, mkfs and fsck sit on top of those. toyos-crypto gains the primitives the format composes (chacha::xor, poly1305::tag/verify), and SipHash moves there because it is shared: net-wire re-exports the same function and the engine hashes names with it. Neither crate takes on bcachefs policy. mkfs takes every identifier from the caller's Random and every time from the caller. That matches §4.2's plan for B4, where the build passes in a deterministic Random derived from a digest. The empty-root decision keeps the node-count fixed point monotone, it is explained at BTREES, and the judge accepts it. upstream/ and tests/upstream_fixture.rs are deleted and leave no callers: a git grep outside issues/ finds no upstream::, src/upstream or upstream_fixture, and the one issue that named the test file is updated. Production is about 3 280 lines and tests about 445. That replaces 3 286 deleted lines, while adding mkfs, fsck and encryption, which the deleted read side lacked. The SipHash move is net zero.

On clean-room provenance I found no sign of contamination. Names, structure and error style differ from the deleted module. Spot-checked identifiers (FIRST_KEYS, bitmap_covers, in_one_bucket, record_bytes, KNOWN_FEATURES) do not occur in it, and NODE_MAGIC is the natural name. Every byte fact I checked traces to A1, A3, A5 or A10.

Spec conformance (checked, no finding)

• Superblock. The header offsets, the A1.8 checksum range, the A1.9 and A1.10 gates and A1.11's copy selection, with the later copy winning a tie, all match. mkfs writes flag words 0, 1 and 3–6 bit-identical to the fixture values in A1.13, plus wide_macs and the encryption type on DATA. The feature bits 0xFBA80, compat bits, ext flags, downgrade body (A1.35) and extent_type_u64s (A1.36) also match. The member flags and validation follow A1.24 and A1.25, and the bitmap shift is ceil_log2(end) − 6 (A1.27).
• Nodes, bsets and key packing.
  • The first and later record layouts follow A3.2–A3.5.
  • The checksum covers bytes 16 to the end of the key area (A3.12).
  • Merging is by position, with the later bset winning and deleted keys dropped. deleted may precede a live key at the same position (A3.11).
  • A blacklisted first bset is refused and later ones are skipped (A3.10).
  • Format validity follows A3.15, and packed fields are read from the top bit down (A3.17).
• Journal-empty clean state. clean's sequence is 0 and every stamp is 0. The records are the usage key-version record, two clocks and roots in id order (A1.30, A1.31). Upstream logs recovering from clean shutdown, journal seq 0 and resumes, which settles BB-A1-1.
• Alloc rule. There are alloc keys for the sb, journal and btree buckets only, with flags 0x23 and io times 1 (A6). Freespace covers the holes.
• Nonces and MACs.
  • Node nonce: (offset, node seq, journal seq low ^ 0x20000000).
  • First record: bytes 24..136 from block 0 and the keys from block 2.
  • Extent nonce: d0 = 8·n, d3 = hi ^ 0x10000000.
  • The MAC key comes from d3 ^ 0x80000000, and the MAC is over the ciphertext and is verified before decryption.
  • The key wrap uses nonce 0⁴ ‖ uuid[0..8] at counter 0.
  • This all matches A10.5 and A10.8–A10.14, and both upstream encrypted fixtures read whole.
• fsck v0. It covers §3.6's superblock rules, the copies being identical except for their own offset and checksum, the root world, the bitmap, and stamps ≤ clean for both bsets and inodes. Alloc, backpointer and accounting derivation are deferred to B3, as §5's B2 and B3 rows place them.

Findings

BLOCKER 1. Hostile images panic the reader or fsck. The crate claims that external input is refused at its boundary. [profile.toyos] inherits dev, so overflow checks are on in the loader and file server that will link this.

• lib/bcachefs/src/engine/superblock.rs:459: le16(body, 0) on a members_v2 field whose body is shorter than 2 bytes. A one-word field (len == 8) passes the size check at :283, its body is empty, and indexing panics. Check body.len() >= 8 before reading.
• lib/bcachefs/src/engine/superblock.rs:478: ranges.iter().map(|r| r.1).sum() runs before the per-range validity check. Two journal_v2 counts of 2⁶³ overflow u64 and panic. Validate first, or use checked_add.
• lib/bcachefs/src/engine/read.rs:210: stored + if dir { 2 } else { 1 } in u32. The NLINK field is validated only to 32 bits, so a stored u32::MAX panics. Use checked_add and refuse the inode as Corrupt.
• lib/bcachefs/src/engine/read.rs:436: next: k.pos.offset + 1. A dirent at offset u64::MAX panics in readdir. lookup already uses checked_add at :275.
• lib/bcachefs/src/engine/read.rs:316: readlink allocates inode.size bytes, and size is the on-disk u64. A hostile symlink size makes the allocation fail and the program abort. Bound it, for example by the extent range the read can fill or a stated symlink maximum, and refuse anything larger.
• lib/bcachefs/src/engine/fsck.rs:96: offset * SECTOR as u64 over the chosen superblock's layout offsets. read_copy uses checked_mul and skips a copy that does not fit, so an offset such as 2⁶⁰ survives into sb.header.layout and fsck panics. §7 runs fsck on DATA's first mount, so its input is external too.

Each needs a module test that feeds the malformed field or key and expects the named refusal. That is the cheapest sufficient test under root's policy, and today the engine has no malformed-input test beyond the checksum and MAC flips.

BLOCKER 2. Snapshots are not refused, and the PR says they are.

• lib/bcachefs/src/engine/read.rs:98-105 takes the root subvolume's snapshot and reads only keys at exactly that snapshot (:198, :296, :345). Once the root subvolume has been snapshotted, its snapshot is a child id and its keys are inherited from ancestors. The reader then answers NotFound for files that exist and reads their data as holes. That is a silent wrong answer, not a refusal.
• The PR's "Refused by name: … more than one device or snapshot" is false.
• Fix: refuse a root subvolume whose snapshot is not ROOT_SNAPSHOT (u32::MAX) as Unsupported, which matches §3.1's "root only (snapshot u32::MAX)". Add a module test.

SUGGESTION 1. lib/bcachefs/src/engine/crypt.rs:157-158: a corrupt crypt with log2 N of 0 or 32..255 is reported as "scrypt parameters past the caller's limits", and the message at :157 says "past 2^31" for a value above 255. Those are corrupt or unsupported parameters, not a caller limit.

SUGGESTION 2. lib/bcachefs/src/engine/mkfs.rs:86: NUL is refused by name, but a trailing \n is not. Upstream strips one trailing newline (A10.6), so a volume formatted with such a passphrase cannot be unlocked by upstream with the same bytes. Refusing it costs one condition and removes a documented caller precondition.

SUGGESTION 3. lib/bcachefs/src/engine/node.rs:146: a MAC-typed node on an unencrypted volume answers Locked. Corrupt describes it.

Evidence

• Host and build. The --ci host run (exit 0, 101/101) and the --build-only log are at 62873ce. The red run at 3843a01 is explained.
• Judge.
  • v1.39.4 at d997ad76e906 with a clean tree, fsck -n -K < /dev/null, and the §3.4 marker set in lib.sh.
  • ROOT 32 MiB and 1 GiB, and DATA 64 MiB and 1 GiB, pass fresh and after FUSE write and read-back (contents-ok).
  • DATA with no key exits 1 (error reading passphrase). The passphrase on stdin passes, with no marker in either stream. A wrong secret gives Error: incorrect passphrase, exit 1.
  • The control set (bad1k, bad1c, bad2, bad5 rerun, bad6, bad7, zero) and the same damage to B2's own unencrypted and encrypted volumes all fail. bad1c and t-csum are caught by markers alone, as BB-A9-1 says they must be.
  • gen/SHA256SUMS ties the judged images to mkfs at the head.
  • The 8 MiB, 512-bucket geometry is host-tested but not judged. That is optional.
• Mutations. All four patches are minimal and each turns its test red. However, every run used cargo test -p bcachefs --lib <filter> ("65 filtered out"). The evidence shows the targeted test going red, not that only that test does. The PR's "Each turns exactly its test red" should say "turns its test red". That correction is the orchestrator's.
• NOTICE and build/licence.rs. Both fixture entries carry the gz sha256, the raw sha256, the commands, the origin and the SPDX terms. The two licence.rs hashes match NOTICE. The NOTICE text no longer cites the deleted module.
• Issue records. The new defect issue follows issues/README.md. The three corrected records are accurate.

Follow-up noted

The coordinator reports that A11-gaps answers type-5 crc32c, per-btree key types and interior pointer size, and that the implementer is adding a commit. That commit should close issues/the-bcachefs-reader-refuses-crc32c-data-extents.md and make the_crc32c_fixture_reads_its_tree read seq.txt. It is not reviewed here.

SEND BACK: Blockers 1 and 2 remain.

[Japabu]

Review of #1032 (storage B2), round 2, head e51e660

This round reviews e51e660, which follows 62873ce, against round 1's findings and analyst/spec/A11-gaps.md. I read only A11 among the analyst files, not provenance or src.

Round 1 blockers: resolved

Blocker 1 (hostile images panicked). All six sites are fixed:

• superblock.rs:459: a members_v2 body shorter than 8 bytes is refused before le16.
• superblock.rs:478: the journal ranges are validated before they are summed. Each count is then at most nbuckets (≤ 2³¹), so the sum cannot overflow.
• read.rs:219: the link count uses checked_add and refuses with Corrupt.
• read.rs:454: readdir's next offset uses checked_add and refuses with Corrupt.
• read.rs:325: readlink refuses any size above MAX_SYMLINK (4096) before allocating.
• fsck.rs:99-104: a copy offset that does not fit the device is reported as Finding::Copy, not read.

Each site has a module test that builds the malformed field or key and expects the named refusal. I checked by reading the code that, before the fix, each of these tests panics or opens the volume (the dev profile keeps overflow checks), so each holds its fix. The seeded fuzz test (mutated_volumes_are_refused_without_a_panic) adds structured superblock-word and node mutations, with checksums recomputed so each mutation reaches the decoders behind it. It is deterministic and bounded, and the whole engine suite runs in 5.7 s. Its own control (f4) shows it catches the copy-offset revert. The PR states that the other reverts are held by module tests, not by the fuzz.

Blocker 2 (snapshots not refused). Fixed at read.rs:107-111: a root subvolume whose snapshot is not ROOT_SNAPSHOT is refused as Unsupported before any read, and a_snapshotted_root_subvolume_is_refused tests it.

Suggestions 1–3 (round 1). All three are taken:

• scrypt log2 N of 0 is refused as Corrupt in Kdf::values, and log2 N of 32 or more as "past 2^31".
• mkfs refuses a passphrase ending in a newline, with a test.
• A MAC-typed node on an unencrypted volume is now Corrupt.

New code against A11

• Type 5 (A11.1–A11.4).
  • crc32c_zero is the Castagnoli register with initial value 0 and no final XOR. Its test checks the A11.2 values 0x58E3FA20 for 123456789 and 0 over zeros.
  • Verification hashes all compressed sectors and compares against the stored checksum, zero-extended to 16 bytes, as A11.4 says. Types 1 and 5 share that path.
  • The fixture test reads seq.txt whole and checks the A11 cross-check values (0x7080C241, standard CRC-32C 0xE4E4C9C4, 0x96C86455, 0xE5BA59C6).
  • Control m5 (inverted parameters) turns it red.
  • The issue the-bcachefs-reader-refuses-crc32c-data-extents.md is deleted, and nothing in the tree still names it.
• Btree ids and key types (A11.13, A11.7).
  • Btree gains ids 21–28 with damage at 28, as A11.13 gives them.
  • leaf_types matches A11.7 row by row. I checked all 29 rows, including whiteout appearing only in extents, inodes, dirents, xattrs and damage, and set (not lru) in the lru btree.
  • Interior nodes allow exactly types 0, 5 and 18, with size 0 (A11.6, A11.11). clean's root records also require size 0 (A11.6).
• When the check applies (A11.8, A11.9).
  • The reader enforces the table in interior nodes and for strict types in leaves. The five non-strict types are deleted, whiteout, error, cookie and set.
  • Types 39 and above are refused as Unsupported (A1.47).
  • fsck enforces the full table as Finding::KeyType, as A11.9 permits.
  • The writer asserts both the table and the size-0 rule in node::encode. That is an internal invariant, so failing loudly is right.
  • Controls m6, m7 and m8 each turn their test red.

Findings

BLOCKER 1. lib/bcachefs/src/engine/mod.rs (kind::strict, its doc comment):

Types whose btree a read of a leaf does not check: upstream refuses them in the wrong btree only when it commits them.

That describes the five types for which strict returns false. The function itself answers whether a type is checked on read. The comment on this check is therefore inverted. Replace it with one line stating the actual contract, for example "Whether a leaf read checks this type against its btree; deleted, whiteout, error, cookie and set are checked only at commit (A11.8)", or delete it and let the matches! speak.

SUGGESTION 1. lib/bcachefs/src/engine/read.rs:181: check_kind now admits btree_ptr (5) in interior nodes. walk then refuses it as "interior key that is no btree pointer", which is false for that key. "btree_ptr v1 pointer" or "interior key other than btree_ptr_v2" would name it.

Evidence

• --ci host at e51e660 exits 0 with 101 of 101 steps green, including the engine's 79 tests. --build-only exits 0.
• The judge generator writes byte-identical images at e51e660 (SHA256SUMS matches). Round 1's upstream judgement therefore still applies, and nothing in this commit changes what mkfs writes.
• Controls m5–m8 and f1–f5 are recorded with their patches. As in round 1, they ran filtered to one test, so they show "its test turns red", not "exactly".

SEND BACK: one blocker remains, a one-line comment fix. Everything else is ready to land.