Fix bind mounts of filesystems with certain options set

Currently bind mounts of filesystems with nodev, nosuid, noexec,
noatime, relatime, strictatime, nodiratime options set fail in rootless
mode if the same options are not set for the bind mount.
For ro filesystems this was resolved by opencontainers#2570 by remounting again
with ro set.

Follow the same approach for nodev, nosuid, noexec, noatime, relatime,
strictatime, nodiratime but allow to revert back to the old behaviour
via the new `--no-mount-fallback` command line option.

Add a testcase to verify that bind mounts of filesystems with nodev,
nosuid, noexec, noatime options set work in rootless mode.
Add a testcase that mounts a nodev, nosuid, noexec, noatime filesystem
with a ro flag.
Add two further testcases that ensure that the above testcases would
fail if the `--no-mount-fallback` command line option is set.

* contrib/completions/bash/runc:
      Add `--no-mount-fallback` command line option for bash completion.

* create.go:
      Add `--no-mount-fallback` command line option.

* restore.go:
      Add `--no-mount-fallback` command line option.

* run.go:
      Add `--no-mount-fallback` command line option.

* libcontainer/configs/config.go:
      Add `NoMountFallback` field to the `Config` struct to store
      the command line option value.

* libcontainer/specconv/spec_linux.go:
      Add `NoMountFallback` field to the `CreateOpts` struct to store
      the command line option value and store it in the libcontainer
      config.

* utils_linux.go:
      Store the command line option value in the `CreateOpts` struct.

* libcontainer/rootfs_linux.go:
      In case that `--no-mount-fallback` is not set try to remount the
      bind filesystem again with the options nodev, nosuid, noexec,
      noatime, relatime, strictatime or nodiratime if they are set on
      the source filesystem.

* tests/integration/mounts_sshfs.bats:
      Add testcases and rework sshfs setup to allow specifying
      different mount options depending on the test case.

Signed-off-by: Ruediger Pluem <ruediger.pluem@vodafone.com>

contrib/completions/bash/runc

@@ -461,6 +461,7 @@ _runc_run() {
 	   --no-subreaper
 	   --no-pivot
 	   --no-new-keyring
+	   --no-mount-fallback
 	"
 
 	local options_with_args="
@@ -567,6 +568,7 @@ _runc_create() {
 	   --help
 	   --no-pivot
 	   --no-new-keyring
+	   --no-mount-fallback
 	"
 
 	local options_with_args="
@@ -627,6 +629,7 @@ _runc_restore() {
 	   --no-pivot
 	   --auto-dedup
 	   --lazy-pages
+	   --no-mount-fallback
 	"
 
 	local options_with_args="

create.go

@@ -51,6 +51,10 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 			Name:  "preserve-fds",
 			Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

libcontainer/configs/config.go

@@ -212,6 +212,10 @@ type Config struct {
 	// RootlessCgroups is set when unlikely to have the full access to cgroups.
 	// When RootlessCgroups is set, cgroups errors are ignored.
 	RootlessCgroups bool `json:"rootless_cgroups,omitempty"`
+
+	// Do not try to remount a bind mount again after the first attempt failed on source
+	// filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set
+	NoMountFallback bool `json:"no_mount_fallback,omitempty"`
 }
 
 type (

libcontainer/rootfs_linux.go

@@ -35,6 +35,7 @@ type mountConfig struct {
 	cgroup2Path     string
 	rootlessCgroups bool
 	cgroupns        bool
+	noMountFallback bool
 }
 
 // mountEntry contains mount data specific to a mount point.
@@ -79,6 +80,7 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds []int) (err
 		cgroup2Path:     iConfig.Cgroup2Path,
 		rootlessCgroups: iConfig.RootlessCgroups,
 		cgroupns:        config.Namespaces.Contains(configs.NEWCGROUP),
+		noMountFallback: config.NoMountFallback,
 	}
 	for i, m := range config.Mounts {
 		entry := mountEntry{Mount: m}
@@ -473,7 +475,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 		// first check that we have non-default options required before attempting a remount
 		if m.Flags&^(unix.MS_REC|unix.MS_REMOUNT|unix.MS_BIND) != 0 {
 			// only remount if unique mount options are set
-			if err := remount(m, rootfs); err != nil {
+			if err := remount(m, rootfs, c.noMountFallback); err != nil {
 				return err
 			}
 		}
@@ -1062,24 +1064,33 @@ func writeSystemProperty(key, value string) error {
 	return os.WriteFile(path.Join("/proc/sys", keyPath), []byte(value), 0o644)
 }
 
-func remount(m mountEntry, rootfs string) error {
+func remount(m mountEntry, rootfs string, noMountFallback bool) error {
 	return utils.WithProcfd(rootfs, m.Destination, func(dstFD string) error {
 		flags := uintptr(m.Flags | unix.MS_REMOUNT)
 		err := mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
 		if err == nil {
 			return nil
 		}
-		// Check if the source has ro flag...
+		// Check if the source has flags set according to noMountFallback
 		src := m.src()
 		var s unix.Statfs_t
 		if err := unix.Statfs(src, &s); err != nil {
 			return &os.PathError{Op: "statfs", Path: src, Err: err}
 		}
-		if s.Flags&unix.MS_RDONLY != unix.MS_RDONLY {
+		var checkflags int
+		if noMountFallback {
+			// Check for ro only
+			checkflags = unix.MS_RDONLY
+		} else {
+			// Check for ro, nodev, noexec, nosuid, noatime, relatime, strictatime,
+			// nodiratime
+			checkflags = unix.MS_RDONLY | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NOATIME | unix.MS_RELATIME | unix.MS_STRICTATIME | unix.MS_NODIRATIME
+		}
+		if int(s.Flags)&checkflags == 0 {
 			return err
 		}
-		// ... and retry the mount with ro flag set.
-		flags |= unix.MS_RDONLY
+		// ... and retry the mount with flags found above.
+		flags |= uintptr(int(s.Flags) & checkflags)
 		return mountViaFDs(m.Source, m.srcFD, m.Destination, dstFD, m.Device, flags, "")
 	})
 }

libcontainer/specconv/spec_linux.go

@@ -312,6 +312,7 @@ type CreateOpts struct {
 	Spec             *specs.Spec
 	RootlessEUID     bool
 	RootlessCgroups  bool
+	NoMountFallback  bool
 }
 
 // getwd is a wrapper similar to os.Getwd, except it always gets
@@ -358,6 +359,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 		NoNewKeyring:    opts.NoNewKeyring,
 		RootlessEUID:    opts.RootlessEUID,
 		RootlessCgroups: opts.RootlessCgroups,
+		NoMountFallback: opts.NoMountFallback,
 	}
 
 	for _, m := range spec.Mounts {

restore.go

@@ -98,6 +98,10 @@ using the runc checkpoint command.`,
 			Value: "",
 			Usage: "Specify an LSM mount context to be used during restore.",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

run.go

@@ -64,6 +64,10 @@ command(s) that get executed on start, edit the args parameter of the spec. See
 			Name:  "preserve-fds",
 			Usage: "Pass N additional file descriptors to the container (stdio + $LISTEN_FDS + N in total)",
 		},
+		cli.BoolFlag{
+			Name:  "no-mount-fallback",
+			Usage: "Do not fallback when the specific configuration is not applicable (e.g., do not try to remount a bind mount again after the first attempt failed on source filesystems that have nodev, noexec, nosuid, noatime, relatime, strictatime, nodiratime set)",
+		},
 	},
 	Action: func(context *cli.Context) error {
 		if err := checkArgs(context, 1, exactArgs); err != nil {

tests/integration/mounts_sshfs.bats

@@ -3,7 +3,20 @@
 load helpers
 
 function setup() {
-	# Create a ro fuse-sshfs mount; skip the test if it's not working.
+	setup_busybox
+	update_config '.process.args = ["/bin/echo", "Hello World"]'
+}
+
+function teardown() {
+	# Some distros do not have fusermount installed
+	# as a dependency of fuse-sshfs, and good ol' umount works.
+	fusermount -u "$DIR" || umount "$DIR"
+
+	teardown_bundle
+}
+
+function setup_sshfs() {
+	# Create a fuse-sshfs mount; skip the test if it's not working.
 	local sshfs="sshfs
 		-o UserKnownHostsFile=/dev/null
 		-o StrictHostKeyChecking=no
@@ -12,30 +25,86 @@ function setup() {
 	DIR="$BATS_RUN_TMPDIR/fuse-sshfs"
 	mkdir -p "$DIR"
 
-	if ! $sshfs -o ro rootless@localhost: "$DIR"; then
+	if ! $sshfs -o "$1" rootless@localhost: "$DIR"; then
 		skip "test requires working sshfs mounts"
 	fi
+}
 
-	setup_busybox
-	update_config '.process.args = ["/bin/echo", "Hello World"]'
+@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
+	setup_sshfs "ro"
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	[ "$status" -eq 0 ]
 }
 
-function teardown() {
-	# New distros (Fedora 35) do not have fusermount installed
-	# as a dependency of fuse-sshfs, and good ol' umount works.
-	fusermount -u "$DIR" || umount "$DIR"
+@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	# The "sync" option is used to trigger a remount with the below options.
+	# It serves no further purpose. Otherwise only a bind mount without
+	# applying the below options will be done.
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
+				}]'
 
-	teardown_bundle
+	runc run test_busybox
+	[ "$status" -eq 0 ]
 }
 
-@test "runc run [rw bind mount of a ro fuse sshfs mount]" {
+@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
 	update_config '	  .mounts += [{
 					type: "bind",
 					source: "'"$DIR"'",
 					destination: "/mnt",
-					options: ["rw", "rprivate", "nosuid", "nodev", "rbind"]
+					options: ["rbind", "ro"]
 				}]'
 
 	runc run test_busybox
 	[ "$status" -eq 0 ]
 }
+
+@test "runc run [dev,exec,suid,atime bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	# The "sync" option is used to trigger a remount with the below options.
+	# It serves no further purpose. Otherwise only a bind mount without
+	# applying the below options will be done.
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["dev", "suid", "exec", "atime", "rprivate", "rbind", "sync"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	# The above will fail as we added --no-mount-fallback which causes us not to
+	# try to remount a bind mount again after the first attempt failed on source
+	# filesystems that have nodev, noexec, nosuid, noatime set.
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
+}
+
+@test "runc run [ro bind mount of a nodev,nosuid,noexec,noatime fuse sshfs mount without fallback]" {
+	setup_sshfs "nodev,nosuid,noexec,noatime"
+	update_config '	  .mounts += [{
+					type: "bind",
+					source: "'"$DIR"'",
+					destination: "/mnt",
+					options: ["rbind", "ro"]
+				}]'
+
+	runc run --no-mount-fallback test_busybox
+	# The above will fail as we added --no-mount-fallback which causes us not to
+	# try to remount a bind mount again after the first attempt failed on source
+	# filesystems that have nodev, noexec, nosuid, noatime set.
+	[ "$status" -ne 0 ]
+	[[ "$output" == *"runc run failed: unable to start container process: error during container init: error mounting"*"operation not permitted"* ]]
+}

utils_linux.go

@@ -175,6 +175,7 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (*libcon
 		Spec:             spec,
 		RootlessEUID:     os.Geteuid() != 0,
 		RootlessCgroups:  rootlessCg,
+		NoMountFallback:  context.Bool("no-mount-fallback"),
 	})
 	if err != nil {
 		return nil, err