Skip to content

core/mount: fix getUnprivilegedMountFlags iterating over indices instead of values#12939

Closed
lukefr09 wants to merge 2 commits intocontainerd:mainfrom
lukefr09:fix-unprivileged-mount-flags
Closed

core/mount: fix getUnprivilegedMountFlags iterating over indices instead of values#12939
lukefr09 wants to merge 2 commits intocontainerd:mainfrom
lukefr09:fix-unprivileged-mount-flags

Conversation

@lukefr09
Copy link
Copy Markdown
Contributor

Summary

  • getUnprivilegedMountFlags uses for flag := range unprivilegedFlags which iterates over slice indices (0–6) instead of the actual flag values (MS_RDONLY, MS_NODEV, etc.)
  • This was a porting error from moby/moby where the data structure was a map[uint64]string (where for k := range m yields keys). When converted to []int, the loop was not updated to for _, flag := range.
  • As a result, MS_NOEXEC, MS_NOATIME, MS_RELATIME, and MS_NODIRATIME are never detected or preserved during bind-mount remounts in user namespaces
  • MS_RDONLY (0x1), MS_NOSUID (0x2), and MS_NODEV (0x4) work by coincidence because their values equal low slice indices
  • In user namespace environments, this causes bind-mount remounts to fail with EPERM when any of the missed flags are locked on the parent mount

Fix

Change for flag := range to for _, flag := range to iterate over values.

Test plan

  • Verify bind mounts with ro option succeed in user namespace environments where MS_NOEXEC, MS_NOATIME, MS_RELATIME, or MS_NODIRATIME are locked on the parent mount
  • Verify rootless container startup with bind mounts on filesystems mounted with noexec or noatime

…ead of values

The loop `for flag := range unprivilegedFlags` iterates over slice
indices (0,1,2,3,4,5,6) rather than the actual flag values (MS_RDONLY,
MS_NODEV, etc). This was a porting error from moby/moby where the data
structure was a map (where `for k := range m` yields keys/values).

As a result, MS_NOEXEC, MS_NOATIME, MS_RELATIME, and MS_NODIRATIME are
never detected or preserved. In user namespaces, this causes bind-mount
remounts to fail with EPERM when any of these flags are locked on the
parent mount, because the kernel requires all CL_UNPRIVILEGED locked
flags to be preserved during remount.

MS_RDONLY (0x1), MS_NOSUID (0x2), and MS_NODEV (0x4) happened to work
by coincidence because their values equal low index numbers.

Fix by using `for _, flag := range` to iterate over values.

Signed-off-by: Luke Hinds <[email protected]>
Mounts a tmpfs with MS_NOEXEC, MS_NOATIME, and MS_NODIRATIME and
verifies that getUnprivilegedMountFlags detects all of them. These
three flags were the ones missed by the range-over-indices bug.

Also verifies that flags not present on the mount (MS_NOSUID,
MS_NODEV, MS_RDONLY) are not falsely reported.

Signed-off-by: Luke Hinds <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants