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 <kolyshkin@gmail.com>

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, "")
 }
 

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,"
+}