Skip to content

fix(vm): keep ro-share readdir and guest whiteouts working, shard create-family admission - #139

Open
lizhicui wants to merge 6 commits into
mainfrom
fix/fs-metadata-concurrency-and-whiteouts
Open

lizhicui wants to merge 6 commits into
mainfrom
fix/fs-metadata-concurrency-and-whiteouts

Conversation

@lizhicui

Copy link
Copy Markdown
Collaborator

Summary

  • ro-DAX payloads share readdir hung forever on hashed host directories. The directory-image builder rejected any backing d_off with bit 62/63 set (ext4 dx produces them for ~half the entries of any churned directory) with EOVERFLOW, and the patched guest kernel retried the byte-identical READINDEX indefinitely because only ENOSYS was a per-directory fallback (observed live on 84.58: ls on the payloads share spinning reading directory: Value too large until killed; python staged on the share could not even import). Unrepresentable cookies now answer ENOSYS — the pre-existing per-handle fallback signal — and the 0041 kernel patch treats EOVERFLOW/EINVAL the same way instead of looping on one offset; only ENOSYS still sets the connection-wide no-readindex flag. Wire-level coverage asserts the reply is -ENOSYS, never -EOVERFLOW.
  • Guest whiteout devices could not be created through virtiofs. Node security enforcement denies mknod of whiteout devices to sandbox processes while allowing every other device number (strace on the runner thread: mknodat(makedev(1,3)) succeeds, mknodat(makedev(0,0)) → EPERM, no audit record), so extracting a docker overlay2 store lost its 14 char(0,0) entries (tar rc=2). Whiteouts are only stat'ed and listed, never opened, so both attachment styles now store a host-safe placeholder file carrying a private user.pvisor.virtual_node record and project it back as the device on lookup/getattr/readdirplus (session registry on the passthrough/shim path — fgetxattr cannot serve O_PATH inodes; xattr projection on the overlay/daemon path). With the real CVE-Factory v3 payload (62,508 entries) tar now exits rc=0 with every entry present; known gap, recorded in the commit: the guest's cached stat still reports the placeholder type until revalidation, so find -type c is 0 inside the guest.
  • Metadata writes were globally serialized. Every mutation funneled through one exclusive operation lock regardless of queue count. mkdir/mknod/symlink/setxattr/removexattr now admit through 64 per-parent-directory shards under the shared guard; structural ops (rename/unlink/rmdir/link/setattr/create) and snapshot barriers keep the exclusive admission; experiment owners fall back automatically. The directory-names cache keeps its coherence via versioned publication; the scheduler steal set gains the newly parallelizable opcodes. Host service cost per create/setattr also drops: fresh-creation node publication takes the nodes lock once instead of three times, and setattr no longer repeats its fence + copy-up prepare twice per request.
  • Test hygiene: the descriptor-leak contract test counted every fd in /proc/self/fd, so sibling tests' tempdirs tripped it as phantom leaks (~1 in 8 full-suite runs); it now scopes to its own sandbox tree.

Validation

  • Live k8s integration (v1.36.2 / containerd 2.2.5, node 10.140.84.58), real CVE-Factory v3 payload (cve-2020-5274:v3, 62,508 entries / 2.46 GiB): payloads-share ls/os.listdir healthy where the pre-fix build spun; guest mknod c 0 0 succeeds where it returned EPERM; full virtiofs extraction rc=0, 62,508 entries where the pre-fix build returned rc=2 with 14 dropped devices; five-round business unpack p50 17.1 s (pre-campaign: 114.5 s on ecs08), tmpfs full extract 16.2 s, host tmpfs control 4.72 s.
  • Tests: cargo test -p pvisor-vm 654 pass (only the pre-existing cold_ram flake fails on a clean main checkout too), including new regressions: unrepresentable-cookie fallback + wire-level -ENOSYS (guest_index), whiteout placeholder projection across lookup/getattr/readdirplus/readdir types + xattr guards + rename-whiteout (overlay correctness), sharded admission overlap/serialization/exclusion/fallback (5 tests), passthrough EPERM fallback. rustfmt clean; clippy delta zero (one pre-existing api.rs warning on main).
  • Build/deploy: musl release rebuilt per stage and deployed live (final 02657eeb), per-pod binary swaps under runtime v2; node left at business baseline (injection catalogue, payloads dir and test image restored/cleaned). Interim states of the split were compile-checked (cargo check green at each commit's tree); campaign evidence and per-probe numbers in build/verify-unpack-fix/RESULTS.md (untracked ops dir).
  • Honest boundaries: the parallel-admission end-to-end gain is bounded by guest-side FUSE scheduling (16k pure-mkdir probe: 4.2 s both before and after on the deployed binaries — host-side overlap is proven by the unit probes, not yet by wall clock); the post-mknod stat-type cache gap is called out above; one perf candidate (post-create fstatat attribute fast path) was reverted after it broke boot on payload-injected pods — mechanism not fully isolated, kept out of this PR.

…representable

Host directories with hashed cookies (ext4 dx and friends) routinely
produce d_off values with bit 62/63 set; collect_page rejected them with
EOVERFLOW, and the patched guest kernel retried the byte-identical
READINDEX forever because only ENOSYS was treated as a per-directory
fallback. Listing a ro-DAX payloads share therefore hung ls in a tight
loop and broke python imports staged on the share (observed live on the
84.58 node: readdir spam until kill, statx of one entry failing once
through the same lookup-prefetch path).

Unrepresentable cookies, a first record that cannot fit the guest budget
and synthetic-id overflow now all return ENOSYS, the pre-existing signal
the guest caches per directory handle (and per inode for prefetch), so
enumeration degrades to ordinary READDIR instead of erroring. The 0041
directory-image kernel patch gains the matching amplifier fix: EOVERFLOW
and EINVAL from READINDEX switch the handle to passthrough too, instead
of retrying the same offset forever; only ENOSYS still sets the
connection-wide no-readindex flag. Host-side wire coverage asserts the
reply is -ENOSYS, never -EOVERFLOW.
The create/release/forget cycle test counted every fd in /proc/self/fd,
so unrelated tempdirs opened by concurrently running sibling tests in
the same process tripped the +4 threshold as phantom leaks (a full-suite
run on this tree showed foreign snapshot paths in the leaked-fd list
about one run in eight; isolated runs always passed). Count only
descriptors under the probe's own sandbox root: the leak being guarded
is the fs tree this test creates.
…attr after one prepare

created_entry took the nodes lock three times per create (inode
allocation, regular-file upper binding, guest lookup reference); the
sequence is now one critical section in publish_created_inode.setattr
on an upper file ran the full prepare cycle twice per FUSE request —
prepare_metadata_change and then writable_inner each fenced the path
and re-copied-up, and a real tar workload issues chown and utimes as
two separate SETATTRs, doubling everything. writable_inner_settled
skips the repeated fence and copy_up once prepare_metadata_change has
settled them under the same exclusive admission; only the passthrough
inode resolution and the regular-file binding remain.
Every metadata mutation funneled through one exclusive operation lock,
so even a four-queue device serialized mkdir/mknod/symlink/xattr writes
no matter how many host threads were available (live probe on 84.58:
62.5k-entry stores pinned at ~1.4 ms/op regardless of concurrency).
mkdir, mknod, symlink, setxattr and removexattr now take the shared
operation guard plus one of 64 shard mutexes keyed by parent directory
path — guard order is always shared-guard, path resolution, shard, and
paths only move under the exclusive guard, so a shard key stays stable
while held. Structural changes (rename, unlink, rmdir, link, setattr,
create) and every snapshot barrier keep the exclusive admission and
therefore still exclude all sharded admissions and readers.

The directory-names cache keeps its no-stale-listing contract under the
weaker exclusion by versioning: admissions bump and clear at both
admission and release, and a publication that captured an older version
is discarded instead of installed. Owners of the prepared-copy-up,
exclusive-upper-absence, logical-directory and negative-entry-cache
experiments keep their documented single-lock contracts verbatim — the
sharded admission falls back to the exclusive one whenever any of them
is enabled. The scheduler's steal set gains the newly parallelizable
create-family opcodes so a busy lane's backlog can be stolen.
…files

Node security enforcement denies mknod of whiteout devices to sandbox
processes while allowing every other device number (strace on 84.58:
same runner thread, mknodat(makedev(1,3)) succeeds, mknodat(makedev(0,0))
returns EPERM with no audit record), so a guest tar extracting a docker
overlay2 store failed its 14 char(0,0) entries with rc=2 and dropped
them from the tree.

Whiteouts are only stat'ed and listed, never opened, so both attachment
styles now store them host-safely and project them back. Passthrough
roots (the shim path): mknod falls back on EPERM to an empty 0600
placeholder carrying the private user.pvisor.virtual_node record, with
a session registry driving the lookup/getattr/readdirplus projection —
fgetxattr cannot serve this because passthrough inodes hold O_PATH
descriptors. Overlay roots (the daemon path): the same interception in
OverlayFs::mknod and create_guest_whiteout writes the record beside the
xattr-based projection already used by backing_attributes, readdirplus
types and the directory-names snapshot. The record is server-private:
guest setxattr/removexattr get EPERM, getxattr ENODATA, listings hide
it, and fingerprints/apply treat it as control state like
override_stat.

With this, the real CVE-Factory v3 payload (62,508 entries) extracts to
a virtiofs root with tar rc=0 and every entry present, up from rc=2
with 14 missing devices. A leftover gap is recorded honestly: after a
successful mknod the guest's cached stat still reports the placeholder
type until revalidation, so find -type c shows 0 inside the guest;
docker's readdir-based whiteout detection sees the projected DT_CHR,
stat-type consumers do not yet.
Three bulk edits landed wrong in the rush: apply.rs gained a stray
set_xattr argument at three call sites (the intended VIRTUAL_NODE_XATTR
entry belonged only to the private-control list assertion), the list
literal lost its indentation, and the new overlay code mixed u32 wire
modes with platform mode_t constants (macOS st_mode/S_IFMT are u16).
Also record the bilingual revision for the overlayfs concurrency edit
that the docs gate requires.

This branch has not been deployed

No deployments
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