Skip to content

overlay: fix EPERM panic on directory rename and add noxattr - #14073

Open
anthops wants to merge 3 commits into
google:masterfrom
anthops:fix/14033-selfoverlay-userxattr-support
Open

overlay: fix EPERM panic on directory rename and add noxattr#14073
anthops wants to merge 3 commits into
google:masterfrom
anthops:fix/14033-selfoverlay-userxattr-support

Conversation

@anthops

@anthops anthops commented Aug 11, 2026

Copy link
Copy Markdown

Renaming a directory within an overlay whose upper layer is tmpfs, from within a nested user namespace (e.g. unshare -Urm), caused the sandbox to panic with:

unrecoverable overlayfs inconsistency: failed to make directory opaque: operation not permitted

This occurred becasue RenameAt unconditionally attempted to set trusted.overlay.opaque on the renamed directory after the rename, and panicked on failure. This is because trusted.* xattrs require CAP_SYS_ADMIN in the init user namespace, which is unavailable.

On native Linux, the overlay filesystem probes xattr support at mount time, setting ofs->noxattr on failure. Directory renames that require the opaque xattr return EXDEV, allowing userspace tools like mv to fall back to copy + delete.

Changes

  • Added noxattr detection at mount time, probing for trusted.overlay.*/user.overlay.* xattr support, similar to Linux's ovl_make_workdir().
  • Moved the opaque xattr set to before the rename to avoid filesystem inconsistency if the xattr set fails. This only occurs on pure-upper directories when newParent has lower layers that could show through, matching Linux's behaviour in ovl_rename_upper().
  • Replaced the panic with checkSetXattr, which returns EXDEV in noxattr mode, similar to Linux's ovl_set_opaque_xerr.
  • Added an early return in noxattr mode that rejects merge/lower directory renames with EXDEV. This is because gVisor does not implement redirect_dir (see ovl_set_redirect ). Without redirect support, there is no way to track where a merge directory moved to after a rename, so we instruct the userspace tool to fall back here.
  • Added OverlayfsDirectoryRenameInUserNamespaceNoxattrFallback test to mount.cc that tests the EXDEV return

Fixes #14033

Writing tests for the noxattr overlay support and EXDEV in RenameAt
revealed that the previous implementation unconditionally
set the opaque xattr on every directory rename, diverging from the
Linux kernel's ovl_rename_upper() behaviour.

Tightened the RenameAt directory rename logic to match Linux:
- In noxattr mode, reject merge/lower directory renames with EXDEV
  since gVisor does not support redirect_dir
  (see ovl_rename_start() -> ovl_can_move()).
- Only set the opaque xattr on pure-upper directories when newParent
  has lower layers that could show through
  (see ovl_rename_upper() -> ovl_set_opaque_xerr()).

Implemented test:
- Extracted OverlayDirRenameInUserNS test helper in mount.cc and created
  OverlayfsDirectoryRenameInUserNamespaceNoxattrFallback test (no userxattr, rename returns EXDEV).
- Changed noxattr probe warning to Debugf to avoid leak-check failures.
@google-cla

google-cla Bot commented Aug 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@anthops anthops changed the title overlay: fix EPERM panic on directory rename and add noxattr #14033 overlay: fix EPERM panic on directory rename and add noxattr Aug 11, 2026
@ayushr2

ayushr2 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

@anthops here is what Claude had to say:

I fetched the PR, read the changes in context, and re-ran the Linux experiments against the specific claims it makes. The direction is right — mount-time probe, checkSetXattr port, set-opaque-before-rename, EXDEV instead of panic all mirror Linux correctly. But there are three correctness bugs and a test that validates the wrong thing.

1. The probe both flips the xattr prefix and sets noxattr — these contradict each other

overlay.go probe block:

fs.noxattr = true
fs.xattrPrefix = linux.XATTR_USER_PREFIX + "overlay."
fs.xattrOpaque = fs.xattrPrefix + "opaque"

Linux does not do this. ovl_make_workdir() sets ofs->noxattr = true and keeps the trusted prefix; it only prints "try mounting with 'userxattr' option" and leaves the choice to the admin. The PR does both, and they cancel out badly:

  • The reported bug isn't actually fixed, just downgraded. After the flip, user.overlay.opaque is perfectly writable — I confirmed on the host that setxattr("user.overlay.opaque") succeeds in the same userns where the trusted one gets EPERM. But checkSetXattr short-circuits on fs.noxattr before trying, so the rename in EPERM when renaming directory within overlay whose upper layer is tmpfs, from within a nested user namespace #14033 still returns EXDEV when it could simply have worked.
  • MkdirAt and RenameAt now disagree. filesystem.go:737 and :755 still call vfsObj.SetXattrAt directly with fs.xattrOpaque — now user.overlay.opaque — and those succeed. So the filesystem writes user-prefixed opaque markers on mkdir while claiming it can't write them on rename.
  • Reads silently change meaning. lookupLocked (filesystem.go:299-306) reads fs.xattrOpaque. Any pre-existing trusted.overlay.opaque on the upper — e.g. written by an earlier mount of the same upper from the root userns — is now ignored, so lower-layer entries that were meant to be hidden reappear.
  • Silently honoring user.*, which any file owner can write, is exactly what Linux makes an explicit opt-in.

Pick one: Linux-faithful (keep trusted.*, noxattr = true, EXDEV), or gofer-style (flip to userxattr, noxattr = false, rename just works). The PR description says the former; the code implements a broken hybrid. Note the field comment — "In this case, user xattrs (user.overlay.*) are used instead" — documents behavior that checkSetXattr contradicts.

2. The new failure point sits after destination whiteouts were already removed

The opaque set is inserted at filesystem.go:1292, i.e. after the block at 1242-1268 that strips whiteouts from the replaced directory. The lookupLayerUpperWhiteout branch (1260-1267) unlinks the whiteout at newpop and is not tracked by needRecreateWhiteouts, so cleanupRecreateWhiteouts() cannot restore it.

Reachable case: mv src dst where src is a pure-upper dir, dst was previously deleted (whiteout in upper over a lower dst), and newParent is merged. Under noxattr the new check returns EXDEV deterministically — and the whiteout is already gone, so the lower dst reappears in the merged view. mv then falls back to copy and merges into content that was supposed to stay deleted.

There's a pre-existing hole here (the RenameAt failure at :1284 has the same gap), but that's a rare I/O error; this PR converts it into a guaranteed path. Move the opaque set above the renamed.isDir() whiteout-removal block.

3. Opaque on the source dir isn't reverted when the rename fails

Linux only does the pre-rename opaque set for pure upper dirs — dir.c:1291 is the else if arm, reached only when !ovl_type_merge_or_lower(old). Marking a pure-upper dir opaque is semantically a no-op, so leaving it behind on failure is harmless.

The PR's condition is renamed.isDir() && len(newParent.lowerVDs) > 0, which drops the pure-upper precondition. In non-noxattr mode a merge dir hits this, and if vfsObj.RenameAt then fails, the source directory is left permanently opaque and its lower-layer contents vanish. The old code set opaque after a successful rename, so this wasn't possible before. Either add len(renamed.lowerVDs) == 0 (and keep the post-rename set for merge dirs, which gVisor needs since it has no redirect_dir), or remove the xattr on the failure path.

4. The new test asserts gVisor-only behavior based on a false premise

// Native Linux allows plain rename() even in noxattr mode,
SKIP_IF(!IsRunningOnGvisor());

That comment is wrong. I ran the test's exact scenario on Linux 6.12 — pure-upper src renamed over a lower-backed dst, overlay mounted in a userns without userxattr:

mount opts: ...,redirect_dir=nofollow,index=off,uuid=null
pure-upper src -> lower-backed dst: rc=-1 errno=EXDEV

Linux returns EXDEV here too (dir.c:1300, ovl_set_opaque_xerr(..., -EXDEV) with noxattr set). Drop the SKIP_IF — the test should cross-check against Linux, which is the whole point of the syscall suite. As written it locks in gVisor behavior with no reference.

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.

EPERM when renaming directory within overlay whose upper layer is tmpfs, from within a nested user namespace

2 participants