From 1b016e9027fdf96f1e230fc3bf48aa8753c65676 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 27 Oct 2026 18:51:56 -0700 Subject: [PATCH] libct: simplify setReadonly, keep rootfs flags setReadonly remounts / with MS_BIND|MS_REMOUNT|MS_RDONLY. A bind remount sets the per-mount flags to exactly the ones given, so when not in a user namespace it silently clears nosuid, nodev, noexec and nosymfollow which may be set on the rootfs mount. This does not happen in a user namespace, where these flags are locked, so the remount fails, and the fallback keeps them. The fix is to always use the existing flags, not only on fallback. In other words, the fallback is no longer needed. While at it - use statfsToMountFlags to convert, since ST_* and MS_* values may differ (e.g. ST_NOSYMFOLLOW != MS_NOSYMFOLLOW); - do not take other flags (e.g *ATIME*) as the kernel preserves those on remount. Add an integration test. Signed-off-by: Kir Kolyshkin --- libcontainer/rootfs_linux.go | 14 +++++++------- tests/integration/mounts.bats | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 6d83a63c40c..1b1efc4cf23 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -1126,17 +1126,17 @@ func prepareRootInHostMountNS(config *configs.Config) error { } func setReadonly() error { - flags := uintptr(unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY) - - err := mount("", "/", "", flags, "") - if err == nil { - return nil - } + // A bind remount sets the per-mount flags to exactly those given, so + // keep the existing nosuid, nodev, noexec and nosymfollow (otherwise + // they are cleared, or the remount fails in a userns where they are + // locked). Atime flags are not passed, so the kernel preserves them. var s unix.Statfs_t if err := unix.Statfs("/", &s); err != nil { return &os.PathError{Op: "statfs", Path: "/", Err: err} } - flags |= uintptr(s.Flags) + keep := statfsToMountFlags(s) & (unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_NOSYMFOLLOW) + flags := uintptr(keep | unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY) + return mount("", "/", "", flags, "") } diff --git a/tests/integration/mounts.bats b/tests/integration/mounts.bats index 6ca4f948095..4ea2603ef8c 100644 --- a/tests/integration/mounts.bats +++ b/tests/integration/mounts.bats @@ -7,6 +7,7 @@ function setup() { } function teardown() { + umount -l "$ROOT/bundle/rootfs" 2>/dev/null || true teardown_bundle } @@ -401,3 +402,19 @@ test_mount_target() { rm -rf rootfs/tmp/hosts test_mount_target . /etc/hosts /tmp/hosts } + +@test "runc run [readonly rootfs keeps nosuid,nodev]" { + requires root + + mount --bind rootfs rootfs + mount -o remount,bind,nosuid,nodev rootfs + + update_config '.root.readonly = true' + # shellcheck disable=SC2016 + update_config --arg prog '$5 == "/" {print "," $6 ","}' \ + '.process.args = ["awk", $prog, "/proc/self/mountinfo"]' + run -0 runc run test_busybox + assert_output --partial ",ro," + assert_output --partial ",nosuid," + assert_output --partial ",nodev," +}