Fix invalid ATIME mask when clearing recursive mount options #5096
+47
−0
Conversation
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
When clearing an access-time related flag (e.g. via 'ratime', 'rnostrictatime', 'rnorelatime'), only the specific bit was being added to recAttrClr (e.g. MOUNT_ATTR_NOATIME). The kernel's mount_setattr(2) requires that if any bit of the MOUNT_ATTR__ATIME mask is changed, the full mask must be present in attr_clr. Partial masks result in EINVAL. This patch ensures that if we are clearing a flag that is part of the ATIME mask, we add the full MOUNT_ATTR__ATIME mask to recAttrClr, allowing the kernel to atomically change the setting (e.g. resetting to relatime). Fixes: opencontainers#5095 Signed-off-by: arnavgogia20 <[email protected]>
Add regression test to ensure that clearing proper recursive access-time flags (ratime, rnostrictatime, rnorelatime) results in the full ATIME mask being added to attr_clr, satisfying kernel requirements. Signed-off-by: arnavgogia20 <[email protected]>
The definition of MOUNT_ATTR__ATIME in golang.org/x/sys/unix might vary or include bits (like NODIRATIME) that incorrectly trigger on some kernels or fail strict checks. This patch defines a local constant containing only the ATIME-related bits (NOATIME|RELATIME|STRICTATIME) to ensure correct and consistent behavior across build environments. This fixes CI failures where 0x70 (likely including NODIRATIME) was being used. Signed-off-by: arnavgogia20 <[email protected]>
Contributor
|
It’s best to refer to the kernel definitions and the https://github.com/torvalds/linux/blob/master/include/uapi/linux/mount.h#L120 Ultimately, what we need to specify in I also think we should verify runc’s actual behavior, not just |
Contributor
|
I'm inclined to close this for reasons similar to ones laid out here: #5097 (comment) but will keep it open for now, letting @arnavgogia20 address the comment above, and @lifubang to take a look as he's working on the same issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Summary
Fix an issue where clearing recursive mount ATIME options could generate an invalid partial ATIME mask, causing
mount_setattr(2)to fail withEINVAL.Problem
The
mount_setattr(2)syscall requires that if any ATIME-related bit is modified, the entireMOUNT_ATTR__ATIMEmask must be provided inattr_clr.While
parseMountOptionscorrectly handled this requirement when setting ATIME flags, the clear path (used byratime,rnorelatime,rnostrictatime) only cleared individual bits. This resulted in invalid partial masks (e.g.0x10) being passed to the kernel, triggeringEINVAL.Solution
parseMountOptionsto ensure that whenever an ATIME-related flag is cleared,unix.MOUNT_ATTR__ATIMEis included inattr_clr.Tests
TestParseMountOptionsRecursiveAtimeto validate correct behavior when clearing recursive ATIME options.Verification
Manually verified using a local reproduction script simulating
parseMountOptionsbehavior:ratime)0x10(invalid)ratime)0x30(valid full ATIME mask)Impact
mount_setattrsemantics.Related Issue
Fixes #5095