Merge pull request #5200 from xujihui1985/fix/rootfs-propagation

fix(libcontainer): preserve rootfs slave propagation for rslave containers

Author: kolyshkin

libcontainer/rootfs_linux.go

@@ -241,8 +241,8 @@ func prepareRootfs(pipe *syncSocket, iConfig *initConfig) (err error) {
 	// to the current root ("/"), and not to the old rootfs before it becomes "/". Applying the
 	// flag in prepareRoot would affect the host mount namespace if the container's
 	// root mount is shared.
-	// MS_PRIVATE is skipped as rootfsParentMountPrivate() is already called.
-	if config.RootPropagation != 0 && config.RootPropagation&unix.MS_PRIVATE == 0 {
+	// MS_PRIVATE or MS_SLAVE is skipped as rootfsParentMountPropagation() is already called.
+	if config.RootPropagation != 0 && config.RootPropagation&(unix.MS_PRIVATE|unix.MS_SLAVE) == 0 {
 		if err := mount("", "/", "", uintptr(config.RootPropagation), ""); err != nil {
 			return fmt.Errorf("unable to apply root propagation flags: %w", err)
 		}
@@ -1061,19 +1061,27 @@ func mknodDevice(destDir *os.File, destName string, node *devices.Device) error
 	return nil
 }
 
-// rootfsParentMountPrivate ensures rootfs parent mount is private.
+func rootfsParentMountPropagationFlags(rootPropagation int) uintptr {
+	if rootPropagation&unix.MS_SLAVE != 0 {
+		return unix.MS_SLAVE
+	}
+	return unix.MS_PRIVATE
+}
+
+// rootfsParentMountPropagation ensures rootfs parent mount is not shared.
 // This is needed for two reasons:
 //   - pivot_root() will fail if parent mount is shared;
-//   - when we bind mount rootfs, if its parent is not private, the new mount
+//   - when we bind mount rootfs, if its parent is (r)shared, the new mount
 //     will propagate (leak!) to parent namespace and we don't want that.
-func rootfsParentMountPrivate(path string) error {
+func rootfsParentMountPropagation(path string, rootPropagation int) error {
 	var err error
+	flags := rootfsParentMountPropagationFlags(rootPropagation)
 	// Assuming path is absolute and clean (this is checked in
 	// libcontainer/validate). Any error other than EINVAL means we failed,
 	// and EINVAL means this is not a mount point, so traverse up until we
 	// find one.
 	for {
-		err = unix.Mount("", path, "", unix.MS_PRIVATE, "")
+		err = unix.Mount("", path, "", flags, "")
 		if err == nil {
 			return nil
 		}
@@ -1083,9 +1091,9 @@ func rootfsParentMountPrivate(path string) error {
 		path = filepath.Dir(path)
 	}
 	return &mountError{
-		op:     "remount-private",
+		op:     "remount-propagation",
 		target: path,
-		flags:  unix.MS_PRIVATE,
+		flags:  flags,
 		err:    err,
 	}
 }
@@ -1099,7 +1107,7 @@ func prepareRoot(config *configs.Config) error {
 		return err
 	}
 
-	if err := rootfsParentMountPrivate(config.Rootfs); err != nil {
+	if err := rootfsParentMountPropagation(config.Rootfs, config.RootPropagation); err != nil {
 		return err
 	}
 

tests/integration/mounts_propagation_unsafe.bats

@@ -0,0 +1,74 @@
+#!/usr/bin/env bats
+
+load helpers
+
+function require_mount_namespace_tools() {
+	command -v unshare >/dev/null || skip "test requires unshare"
+	command -v nsenter >/dev/null || skip "test requires nsenter"
+}
+
+function in_mount_namespace() {
+	local cwd
+	cwd="$(pwd)"
+	nsenter --mount="$ISOLATED_MNTNS" -- sh -c "cd \"\$1\" && shift && exec \"\$@\"" sh "$cwd" "$@"
+}
+
+function setup_isolated_mount_namespace() {
+	ISOLATED_MNTNS_DIR="$(mktemp -d "$BATS_RUN_TMPDIR/mntns.XXXXXX")"
+	mount --bind "$ISOLATED_MNTNS_DIR" "$ISOLATED_MNTNS_DIR"
+	mount --make-private "$ISOLATED_MNTNS_DIR"
+
+	ISOLATED_MNTNS="$ISOLATED_MNTNS_DIR/testns"
+	touch "$ISOLATED_MNTNS"
+	if ! unshare --mount="$ISOLATED_MNTNS" mount --make-rprivate /; then
+		rm -f "$ISOLATED_MNTNS"
+		umount "$ISOLATED_MNTNS_DIR" 2>/dev/null || true
+		rmdir "$ISOLATED_MNTNS_DIR" 2>/dev/null || true
+		fail "failed to bind isolated mount namespace"
+	fi
+}
+
+function teardown_isolated_mount_namespace() {
+	if [ -n "${ISOLATED_MNTNS_DIR:-}" ]; then
+		umount -l "$ISOLATED_MNTNS_DIR" 2>/dev/null || true
+		rmdir "$ISOLATED_MNTNS_DIR" 2>/dev/null || true
+	fi
+}
+
+function __runc_in_mount_namespace() {
+	setup_runc_cmdline
+	in_mount_namespace "${RUNC_CMDLINE[@]}" "$@"
+}
+
+function make_rootfs_shared() {
+	in_mount_namespace mount --make-rshared /
+}
+
+function runc_in_mount_namespace() {
+	CMDNAME="$(basename "$RUNC")" sane_run __runc_in_mount_namespace "$@"
+}
+
+function setup() {
+	requires root
+	require_mount_namespace_tools
+
+	setup_isolated_mount_namespace
+	make_rootfs_shared
+	setup_debian
+}
+
+function teardown() {
+	teardown_bundle
+	teardown_isolated_mount_namespace
+}
+
+@test "runc run [rootfsPropagation slave]" {
+	# make sure the rootfs mount is slave before running the test
+	update_config ' .linux.rootfsPropagation = "slave" '
+
+	update_config ' .process.args = ["findmnt", "--noheadings", "-o", "PROPAGATION", "/"] '
+
+	runc_in_mount_namespace run test_slave_rootfs
+	[ "$status" -eq 0 ]
+	[ "$output" = "private,slave" ]
+}