Skip to content

EROFS follow-ups: mount/umount hardening, layer splitting, go-erofs bump #2408

Description

@smoser

Follow-up work split out of #2249, which was descoped in response to review so
that the single-image writer and apko erofs ls could land on their own. Two of
the items below are bugs found in that review, not new features.

Intended order: #2249 → mount/umount → layering.

The full pre-descope tree is preserved on the feat/apko-erofs-full branch of
https://github.com/smoser/apko, so the follow-ups can branch from working code
rather than reconstruct it.

1. Mount / umount (lands next)

this is in #2415 .
Descoped in smoser/apko@1fe041ea: pkg/erofsmount/mount_linux.go,
driver_linux.go, state.go, stub_other.go and their tests, plus the
apko erofs mount and apko erofs umount subcommands.

  • Validate the state file before unmounting. Unmount executed umount,
    typically as root, on whatever paths <dest>/.apko-erofs-mount.json named;
    LoadState checked only JSON shape and schema version. Anyone able to write
    to dest — and the docs suggested shared locations like /mnt/x — could
    plant a state file naming /home and have root unmount it. Verify
    st.Dest == dest and reject any Mounts entry that escapes dest under
    filepath.Rel. Every path the mount side writes is under dest
    (merged, layers/NN), so nothing legitimate is lost.
  • Make partial-unmount recovery possible. On an EBUSY partway through,
    the error told the user to rerun apko erofs umount. That cannot succeed:
    Mounts is [merged, layers/NN…00], so the rerun's first step unmounts
    merged, which already came down, and exits 32 before reaching the busy
    layer. The state file is never rewritten on partial success, so recovery
    was manual umount plus hand-deleting the state file. Either tolerate a
    not-mounted result per entry, or drop successfully-unmounted entries from
    the state file as it goes.
  • Default to read-only. The default mount was writable and umount
    RemoveAlled upper with no flag or warning, silently discarding
    everything written through the mount. Read-only fits the verification use
    case the docs describe; writes become an opt-in.
  • Add a test seam. Mount/Unmount construct their Driver internally,
    so ~305 lines of orchestration (cleanup LIFO, state lifecycle, unmount
    policy) cannot be driven with a fake. driver_linux_test.go covered only
    the argv builders; nothing anywhere executed the orchestration.
  • Keep the re-added surface unexported where possible. StatePath,
    WriteState, LoadState, RemoveState, StateSchemaVersion, Driver,
    NewDriver and ResolveMode had no callers outside the package. Ls no
    longer needs the shared Options/Mode types — it takes a plain
    arch string — so they can come back package-private.
  • A privileged CI job: apko build --format erofs, kernel mount, fsck.
    This would be the first thing anywhere to execute the mount orchestration.

2. Multi-layer splitting

Descoped in smoser/apko@e5778f17: pkg/build/erofs_layers.go and
TestSplitErofsLayers. layering combined with format: erofs is now rejected
during config validation, with a guard in BuildLayers for library callers that
skip validation.

  • Fix package routing. The type assertion was on the wrong receiver and
    could never succeed in a real build. Real builds walk tarfs, whose
    memFileInfo.Sys() returns a fresh *archive/tar.Header with no
    Package method; Package() is a method on the FileInfo itself, which is
    what the tar split path asserts on. So every file landed in the top layer,
    each group layer held only ancestor directories plus a partial installed db
    (that case keys on the path string, so it still fired), and per-layer
    scanners saw a db claiming packages whose files were absent.
  • Add a tarfs-backed test. TestSplitErofsLayers drove the split
    through apkfs.NewMemFS(), which implements Package() nowhere, so the
    fixture had zero package-owned files by construction and passed identically
    with and without routing. A test through tarfs.New() with package-owned
    files, asserting a file is present in its group layer and absent from top,
    fails on the bug and passes on the fix.
  • Emit dir-only subtrees. Directories whose subtree contains no non-dir
    entry were never written to any layer: they were recorded during the walk
    and materialized only by emitAncestors, which runs for non-dir entries,
    with no post-walk sweep. So empty dirs and dir-only chains — /tmp,
    /run, /home, /var/empty, mount points — were absent from the merged
    view. Both siblings handle this: the tar split writes every walked dir into
    its owning layer, and writeErofs Mkdirs every dir unconditionally.
  • Stop leaking temp files and fds on error paths. No error return out of
    splitErofsLayers closed or removed the per-layer temp files, and each
    go-erofs Writer holds an unlinked spool fd that only Close releases —
    while Close also writes out the image, so it cannot serve as an abort.
    The CLI is bounded by its MkdirTemp/RemoveAll wrapper; library callers
    accumulate files and fds until process exit. An upstream Abort API would
    make this airtight.
  • Replace the silent packageToWriter miss fallback with an error. The tar
    path panics on a miss; the EROFS path fell back to top, which would mask
    grouping bugs the tar path surfaces loudly.
  • Whiteout support in Stack for genuinely layered images. The spec-correct
    encoding already landed in Add erofs support to apko. #2249; nothing apko produces today can contain a
    whiteout, since that needs an overlay-lower predecessor.

3. Compression

Already open as #2406.

Note for whoever picks it up: melange#2605 pins a fork of apko and passes
erofs+zstd[,level=N] format strings that current apko rejects after the split,
so melange's compressed mode stays dead until #2406 merges and melange re-pins.

4. Bump go-erofs past v0.3.1

handled in #2412
erofs/go-erofs#41 fixed two halves of one bug — Mkdir dropping
setuid/setgid/sticky on write, and FileInfo.Mode() misreporting them on read —
but it merged 2026-08-02 while v0.3.1 was tagged 2026-07-21, so the pinned
version has neither.

  • [ x] Once a release containing multiarch support #41 is tagged, bump and re-check whether the
    Chmod after Mkdir/Mknod/Create in pkg/build/erofs.go is still
    load-bearing.
  • [ x] Until then, FileInfo.Mode() from the pinned reader cannot be trusted for
    special bits. Read *erofs.Stat.Mode off Sys() instead, which is what
    pkg/erofsmount/ls.go does.

Hardlinks are a separate matter and not blocked on a bump: go-erofs has no API to
point two names at one inode and none is proposed, so hardlinks stay materialized
as independent copies. SetNlink sets the reported count without sharing the
inode, so it would only make the metadata lie. Spec §3.7 permits materializing
links, and this is now documented in docs/erofs.md.

5. Smaller items from the second review round

Non-blocking notes from @mattmoor's approval of #2249 (2026-08-18). Grouped
here rather than fixed in that PR so it could land.

Everything in this section except the layer-horizon fix landed in #2410.

Stack merge algorithm diverges from kernel overlayfs in two corners

Both predate #2249 — the .wh. code it replaced had the same shape — and both
need two or more layers, so neither is reachable for an apko-produced image
today. Reproduced against the merged code:

Layers Stack shows kernel overlayfs shows
L0 d/x; L1 whiteout on d; L2 dir d/y d = [x y] d = [y]
top a opaque containing a/b; lower a/b/c a/b = [c] a/b = []
  1. A middle-layer whiteout (or plain file) at a directory's own name does not
    cut off lower layers
    when a higher layer recreates the directory.
  2. Opacity is not inherited by descendant directories. The kernel's cut
    covers the whole subtree, because the lower directory never participates.

One root cause for both: lookup and mergeDir treat a readDirOn failure as
"the parent is absent in this layer", which is indistinguishable from "the
parent is a tombstone or a non-directory here". A per-directory layer horizon
computed during descent fixes both.

  • Implement the layer horizon, with tests for both rows above.
  • Until then, soften docs/erofs.md's "the merged view the kernel would
    assemble" to name the known deviations. That line is currently stronger
    than what is delivered. — erofs: first batch of #2408 follow-ups #2410, which names both deviations above and
    links here.

Best done alongside the layered-whiteout item in section 2, since layered ls
only becomes reachable for apko's own output once splitting returns.

isOpaqueDir swallows stat errors

isOpaqueDir returns false when statOn fails, so a corrupt xattr region in
an untrusted layer silently un-hides content instead of surfacing the error.
isWhiteout is partially self-correcting by comparison, since the leaked
entry's own Stat re-raises.

  • Propagate a statOn error that is not fs.ErrNotExist. Note this is the
    same "cannot distinguish absent from broken" confusion as the horizon item
    above, so the two may want fixing together. — erofs: first batch of #2408 follow-ups #2410.
  • isWhiteout got the same (bool, error) treatment in erofs: first batch of #2408 follow-ups #2410, so the two
    tombstone checks no longer disagree about errors. Its failure mode was the
    milder one — an unreadable char device read as live, occupying the name
    rather than leaking lower entries — but it was still a guess made from an
    inode that could not be read.

Error message for application/vnd.erofs+zstd

A spec-legal compressed EROFS layer hits the mediaType check in
ReadOCILayers first and is told the command "only handles EROFS images",
which reads oddly for an EROFS image.

os.features on the index descriptor: pin the base-image case

#2249 copies a config's os.features onto the index platform descriptor for
every format. BuildImageFromLayers DeepCopies the base image's config, so a
tar build on a base image whose config already carries os.features now
surfaces it on the index descriptor, changing that index's digest. That is what
§5.4 asks for, and the squash message for #2249 calls it out, but the test
added there pins only the empty-base case.

§3.7 citation is too strong in two places

pkg/build/erofs.go and docs/erofs.md both say §3.7 permits materializing
hardlinks. §3.7's materialize-or-fail rule governs cross-layer hardlinks;
for same-layer links the spec is silent, so apko's behavior is conformant but
not blessed by that section.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions