Skip to content

fix: ratime and rnostrictatime mount options failing with EINVAL #3467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kechigon wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: ratime and rnostrictatime mount options failing with EINVAL #3467

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d358d2
update crates/libcontainer/src/rootfs/utils.rs
kechigon Mar 22, 2026
a5bc94b
add tests for utils.rs
kechigon Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 136 additions & 1 deletion crates/libcontainer/src/rootfs/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,19 @@ pub fn parse_mount(m: &Mount) -> std::result::Result<MountOptionConfig, MountErr

if let Some(mount_attr) = &mut mount_attr {
if is_clear {
mount_attr.attr_clr |= flag;
if flag != 0 && flag & linux::MOUNT_ATTR__ATIME == flag {
Copy link
Copy Markdown
Member

@saku3 saku3 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to align this with runc’s implementation.

In other words, rnorelatime should be treated as the recursive form of “clear relatime”.
I do not think we need to make the low-level runtime more complex by handling it as a no-op and skipping it there.

That would also allow us to consolidate the if logic the way runc does.

https://github.com/lifubang/runc/blob/main/libcontainer/specconv/spec_linux.go#L1168

// https://man7.org/linux/man-pages/man2/mount_setattr.2.html
// When clearing an atime mode (e.g. ratime, rnostrictatime), the full
// MOUNT_ATTR__ATIME mask (0x70) must be used in attr_clr. The kernel
// rejects partial atime masks with EINVAL because the three atime modes
// (relatime/noatime/strictatime) are mutually exclusive and can only be
// changed atomically: clear all three bits, then set the desired one.
// Note: flag != 0 excludes rnorelatime (MOUNT_ATTR_RELATIME = 0x00),
// which has no bits to clear and should be a no-op.
mount_attr.attr_clr |= linux::MOUNT_ATTR__ATIME;
} else {
mount_attr.attr_clr |= flag;
}
} else {
mount_attr.attr_set |= flag;
if flag & linux::MOUNT_ATTR__ATIME == flag {
Expand Down Expand Up @@ -472,4 +484,127 @@ mod tests {

Ok(())
}

// Tests for the atime clearing fix:
// When clearing an atime mode (ratime, rnostrictatime), attr_clr must use the full
// MOUNT_ATTR__ATIME mask (0x70) rather than the individual flag, because the kernel
// rejects partial atime masks with EINVAL.
#[test]
fn test_parse_mount_ratime_uses_full_atime_mask() -> Result<()> {
// "ratime" clears MOUNT_ATTR_NOATIME (is_clear=true, flag=MOUNT_ATTR_NOATIME=0x10).
// attr_clr must be MOUNT_ATTR__ATIME (0x70), not MOUNT_ATTR_NOATIME (0x10).
let mount_option_config = parse_mount(
&MountBuilder::default()
.destination(PathBuf::from("/mnt"))
.source(PathBuf::from("/tmp/mounts_recursive"))
.options(vec!["rbind".to_string(), "ratime".to_string()])
.build()?,
)?;
assert_eq!(
mount_option_config.rec_attr,
Some(MountAttr {
attr_set: 0,
attr_clr: linux::MOUNT_ATTR__ATIME,
propagation: 0,
userns_fd: 0,
}),
"ratime should set attr_clr to the full MOUNT_ATTR__ATIME mask"
);

Ok(())
}

#[test]
fn test_parse_mount_rnostrictatime_uses_full_atime_mask() -> Result<()> {
// "rnostrictatime" clears MOUNT_ATTR_STRICTATIME (is_clear=true, flag=MOUNT_ATTR_STRICTATIME=0x20).
// attr_clr must be MOUNT_ATTR__ATIME (0x70), not MOUNT_ATTR_STRICTATIME (0x20).
let mount_option_config = parse_mount(
&MountBuilder::default()
.destination(PathBuf::from("/mnt"))
.source(PathBuf::from("/tmp/mounts_recursive"))
.options(vec!["rbind".to_string(), "rnostrictatime".to_string()])
.build()?,
)?;
assert_eq!(
mount_option_config.rec_attr,
Some(MountAttr {
attr_set: 0,
attr_clr: linux::MOUNT_ATTR__ATIME,
propagation: 0,
userns_fd: 0,
}),
"rnostrictatime should set attr_clr to the full MOUNT_ATTR__ATIME mask"
);

Ok(())
}

#[test]
fn test_parse_mount_rnorelatime_is_noop_for_attr_clr() -> Result<()> {
// "rnorelatime" clears MOUNT_ATTR_RELATIME (is_clear=true, flag=MOUNT_ATTR_RELATIME=0x00).
// flag == 0, so attr_clr must remain 0 (no-op: no bits to clear for relatime).
let mount_option_config = parse_mount(
&MountBuilder::default()
.destination(PathBuf::from("/mnt"))
.source(PathBuf::from("/tmp/mounts_recursive"))
.options(vec!["rbind".to_string(), "rnorelatime".to_string()])
.build()?,
)?;
assert_eq!(
mount_option_config.rec_attr,
Some(MountAttr {
attr_set: 0,
attr_clr: 0,
propagation: 0,
userns_fd: 0,
}),
"rnorelatime (flag=0) should be a no-op for attr_clr"
);

Ok(())
}

#[test]
fn test_parse_mount_atime_set_includes_full_atime_mask_in_attr_clr() -> Result<()> {
Copy link
Copy Markdown
Member

@saku3 saku3 Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the rnoatime and rstrictatime tests should be split into separate test cases.

// When setting an atime mode, both attr_set and attr_clr must include the flag/mask.
// rnoatime sets MOUNT_ATTR_NOATIME; attr_clr must include MOUNT_ATTR__ATIME (0x70).
let mount_option_config = parse_mount(
&MountBuilder::default()
.destination(PathBuf::from("/mnt"))
.source(PathBuf::from("/tmp/mounts_recursive"))
.options(vec!["rnoatime".to_string()])
.build()?,
)?;
assert_eq!(
mount_option_config.rec_attr,
Some(MountAttr {
attr_set: linux::MOUNT_ATTR_NOATIME,
attr_clr: linux::MOUNT_ATTR__ATIME,
propagation: 0,
userns_fd: 0,
}),
"rnoatime should set attr_set=MOUNT_ATTR_NOATIME and attr_clr=MOUNT_ATTR__ATIME"
);

// rstrictatime sets MOUNT_ATTR_STRICTATIME; attr_clr must include MOUNT_ATTR__ATIME.
let mount_option_config = parse_mount(
&MountBuilder::default()
.destination(PathBuf::from("/mnt"))
.source(PathBuf::from("/tmp/mounts_recursive"))
.options(vec!["rstrictatime".to_string()])
.build()?,
)?;
assert_eq!(
mount_option_config.rec_attr,
Some(MountAttr {
attr_set: linux::MOUNT_ATTR_STRICTATIME,
attr_clr: linux::MOUNT_ATTR__ATIME,
propagation: 0,
userns_fd: 0,
}),
"rstrictatime should set attr_set=MOUNT_ATTR_STRICTATIME and attr_clr=MOUNT_ATTR__ATIME"
);

Ok(())
}
}
Loading