Support idmapped mounts, and give absent caller ids a type - #748
Merged
Conversation
cberner
force-pushed
the
claude/xfstests-skip-investigation-s9gdr1
branch
from
August 5, 2026 17:25
c8f2521 to
19000ae
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19000ae252
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Negotiating it is what lets a mount be idmapped: with the capability refused, mount_setattr(MOUNT_ATTR_IDMAP) over a fuser filesystem fails with EINVAL, and with it negotiated the same call succeeds. It is accepted only on a mount that carries MountOption::DefaultPermissions and SessionACL::All, and refused otherwise. Neither condition is fuser's choice. Without default_permissions the kernel refuses the connection outright - every request answered ECONNREFUSED - because with the caller's ids withheld its own checks are the only ones left. And once those ids are withheld fuser cannot tell the mounting user's requests from anyone else's, so SessionACL::Owner and RootAndOwner could no longer be enforced; refusing the capability is better than continuing to offer a restriction that has quietly stopped applying. What the kernel withholds, it withholds only from requests that do not create an inode. Those arrive with uid and gid set to the new FUSE_INVALID_UIDGID. The requests that do create one still carry ids, and they are the owner the new inode should get, already mapped - which is the only thing a filesystem needs them for once the kernel is doing the access checks. Request::uid() and Request::gid() say so rather than changing shape, since a filesystem that does not ask for the capability is never given an invalid id. The tests pin all three of these against a real kernel rather than against the encoding: which mount configurations may negotiate it, that a getattr arrives without ids while a mkdir arrives with them, and that the capability is what decides whether an idmapped mount can be created at all. This is the library capability alone. The simple example does not request it, and wiring it there is not a small change: the example does its own uid-based access checks, which is exactly what an idmapped mount stops it from being able to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2
Several comments described the change that introduced them rather than the behavior a reader arrives at, which stops being useful as soon as the change is history. They now state the rule and its reason: why tmpfile checks the parent for search permission when the operand is the directory itself, why nothing touches the parent's timestamps, and which kernel check copy_file_range mirrors. Comparisons to create() go, since in each case the reason alone carries it. Corrects one that was wrong. StatxAttributes said answering FUSE_STATX is what makes chattr +i visible to statx(2). It is not: fuse_do_statx() takes the mask, the creation time and the basic stats out of the reply and ignores the attributes, as StatxAttr::attributes already said. The two were written either side of finding that out and only one was corrected. It now points at the field where the behavior is documented rather than restating it, as does the example. Also drops a kernel version from StatxAttr::attributes. The behavior is the kernel's current one either way, and stamping a version invites a reader to think it changed in that release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2
cberner
force-pushed
the
claude/xfstests-skip-investigation-s9gdr1
branch
from
August 5, 2026 17:35
19000ae to
3da6162
Compare
cberner
changed the title
cberner
force-pushed
the
claude/xfstests-skip-investigation-s9gdr1
branch
from
August 5, 2026 23:04
11a1e83 to
70ee9bf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11a1e83aaf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70ee9bfb75
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
cberner
force-pushed
the
claude/xfstests-skip-investigation-s9gdr1
branch
from
August 5, 2026 23:11
70ee9bf to
276a1d9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 276a1d981a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Request::uid() and gid() return Option<u32>. The kernel withholds the caller's ids on an idmapped mount, from every request that does not create an inode, so a u32 was claiming something that is not always there. Reporting u32::MAX in their place asked every caller to know a sentinel, and to remember to test for it in code whose whole purpose is deciding who may do what. The requests that do create an inode are given an Owner instead. They are the only ones the kernel sends ids for, and what it sends is not the caller's ids but those ids mapped through the mount's idmapping - so on an idmapped mount, reading the owner off the request would have been wrong even where a value is present. Naming it separately keeps a single field from meaning two things. Which requests those are was measured rather than assumed: create, mkdir, mknod, symlink and tmpfile carry ids, and link does not, creating an entry rather than an inode. The simple example does not request FUSE_ALLOW_IDMAP, so it is always given ids. That invariant is stated once, in caller_uid and caller_gid, rather than at each of the ninety places it is relied on. Its five creating handlers take the owner from the argument. Its rename does not, since RENAME_WHITEOUT creates an inode but rename is not a request the kernel sends ids for; the caller's ids are the right answer there for any filesystem that has not asked for idmapped mounts. FUSE_INVALID_UIDGID goes back to being an implementation detail, which is all it is once the type says what it was standing in for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2
cberner
force-pushed
the
claude/xfstests-skip-investigation-s9gdr1
branch
from
August 5, 2026 23:19
276a1d9 to
1ae8a81
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three commits: the capability, a comment pass, and the API change that the
capability's first shape argued for.
1. Allow FUSE_ALLOW_IDMAP to be negotiated
InitFlags::FUSE_ALLOW_IDMAP(ABI 7.41) was inUNSUPPORTED_CAPABILITIES, witha comment giving two reasons. Both turned out to be right, so this does not
remove them - it makes them conditions.
What negotiating it buys. An idmapped mount. Measured both ways, building the
mount the way an idmapped one has to be built -
open_tree(OPEN_TREE_CLONE),mount_setattr,move_mount:mount_setattr(MOUNT_ATTR_IDMAP)EINVALWhy it is conditional. Accepted only where the session allows other users and
default_permissionsis in force - whether fromMountOption::DefaultPermissionsor from negotiating
InitFlags::FUSE_POSIX_ACL, which the kernel treats assetting it. Without
default_permissionsthe kernel refuses the connectionoutright: every request answers
ECONNREFUSED, measured. Without allow_other,fuser would be offering an owner-only ACL it can no longer enforce, since it can
no longer tell the mounting user's requests from anyone else's.
2. Say what the code does rather than what changed about it
A pass over comments from the recent example and statx work. Several described
the change that introduced them rather than the behavior a reader arrives at.
One was wrong:
StatxAttributessaid answeringFUSE_STATXis what makeschattr +ivisible tostatx(2). It is not -fuse_do_statx()ignores theattributes, as
StatxAttr::attributesalready said. The two were written eitherside of finding that out, and only one was corrected.
3. Give absent caller ids a type, and name the owner of a new inode
Breaking.
Request::uid()andgid()returnOption<u32>.The first shape of this PR reported
u32::MAXwhen the kernel withheld the ids,which asks every caller to know a sentinel and to remember to test for it in code
whose whole purpose is deciding who may do what. A
u32also claimed somethingthat is not always there.
The requests that create an inode take an
Ownerinstead. Those are the onlyones the kernel sends ids for, and what it sends is not the caller's ids but
those ids mapped through the mount's idmapping - so on an idmapped mount, reading
the owner off the request would have been wrong even where a value is present.
Which requests those are was measured rather than assumed:
create,mkdir,mknod,symlink,tmpfilelookup,getattr,open,link,rename,unlinklinkcreates an entry rather than an inode, which is why it is on the right.The example does not request the capability, so it is always given ids; that
invariant is stated once in
caller_uid/caller_gidrather than at each of theninety places it is relied on. Its
renamekeeps using the caller's ids for theRENAME_WHITEOUTinode, since rename is not a request the kernel sends ids for -the right answer for any filesystem that has not asked for idmapped mounts.
FUSE_INVALID_UIDGIDis unexported again, which is all it should be once thetype says what it was standing in for.
Testing
Five tests over the capability, pinning each claim against a real kernel rather
than against the encoding: which mount configurations may negotiate it (including
that POSIX ACLs satisfy it, and that a kernel not offering them does not), that a
getattrarrives with no ids while amkdiris given an owner, and that thecapability decides whether an idmapped mount can be created at all.
cargo test --allis 108 passed, 1 failed:mnt::test::mount_unmount_auto_unmount,which fails identically on unmodified master here for want of a
fusermount3binary. Clippy, rustdoc and the macOS check clean with
--deny warnings, the lastwith
--all-targetsso it builds tests too.The example's four behavior suites pass - chattr/immutable, noatime, O_TMPFILE and
btime - and the full xfstests suite is running against the rewritten call sites.
Not included
examples/simple.rsstill does not request the capability, so no xfstests changehere. generic/644, 645, 656, 689, 698 and 699 need it to, and that is a change of
a different kind: the example does its own uid-based access checks throughout,
which is precisely what an idmapped mount stops it from being able to do.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2