diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index c302be086..ef764df5f 100644 --- a/libcontainer/rootfs_linux.go +++ b/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 } diff --git a/tests/integration/mounts_propagation_unsafe.bats b/tests/integration/mounts_propagation_unsafe.bats new file mode 100644 index 000000000..112c39bd5 --- /dev/null +++ b/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" ] +}