From 669f4dbef8cabef5fe2c277af476d8e6ed4feb1b Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <cyphar@cyphar.com>
Date: Wed, 23 Aug 2023 02:28:32 +1000
Subject: [PATCH] configs: validate: add validation for bind-mount fsflags

Bind-mounts cannot have any filesystem-specific "data" arguments,
because the kernel ignores the data argument for MS_BIND and
MS_BIND|MS_REMOUNT and we cannot safely try to override the flags
because those would affect mounts on the host (these flags affect the
superblock).

It should be noted that there are cases where the filesystem-specified
flags will also be ignored for non-bind-mounts but those are kernel
quirks and there's no real way for us to work around them. And users
wouldn't get any real benefit from us adding guardrails to existing
kernel behaviour.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 libcontainer/configs/validate/validator.go    | 23 ++++++++++
 .../configs/validate/validator_test.go        | 43 +++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/libcontainer/configs/validate/validator.go b/libcontainer/configs/validate/validator.go
index a6736f26584..0df7242b38d 100644
--- a/libcontainer/configs/validate/validator.go
+++ b/libcontainer/configs/validate/validator.go
@@ -275,6 +275,26 @@ func cgroupsCheck(config *configs.Config) error {
 	return nil
 }
 
+func checkBindOptions(m *configs.Mount) error {
+	if !m.IsBind() {
+		return nil
+	}
+	// We must reject bind-mounts that also have filesystem-specific mount
+	// options, because the kernel will completely ignore these flags and we
+	// cannot set them per-mountpoint.
+	//
+	// It should be noted that (due to how the kernel caches superblocks), data
+	// options could also silently ignored for other filesystems even when
+	// doing a fresh mount, but there is no real way to avoid this (and it
+	// matches how everything else works). There have been proposals to make it
+	// possible for userspace to detect this caching, but this wouldn't help
+	// runc because the behaviour wouldn't even be desirable for most users.
+	if m.Data != "" {
+		return errors.New("bind mounts cannot have any filesystem-specific options applied")
+	}
+	return nil
+}
+
 func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
 	if !m.IsIDMapped() {
 		return nil
@@ -313,6 +333,9 @@ func mountsWarn(config *configs.Config) error {
 
 func mountsStrict(config *configs.Config) error {
 	for _, m := range config.Mounts {
+		if err := checkBindOptions(m); err != nil {
+			return fmt.Errorf("invalid mount %+v: %w", m, err)
+		}
 		if err := checkIDMapMounts(config, m); err != nil {
 			return fmt.Errorf("invalid mount %+v: %w", m, err)
 		}
diff --git a/libcontainer/configs/validate/validator_test.go b/libcontainer/configs/validate/validator_test.go
index 7de9b0f9061..a882fa64cce 100644
--- a/libcontainer/configs/validate/validator_test.go
+++ b/libcontainer/configs/validate/validator_test.go
@@ -421,6 +421,49 @@ func TestValidateMounts(t *testing.T) {
 	}
 }
 
+func TestValidateBindMounts(t *testing.T) {
+	testCases := []struct {
+		isErr bool
+		flags int
+		data  string
+	}{
+		{isErr: false, flags: 0, data: ""},
+		{isErr: false, flags: unix.MS_RDONLY | unix.MS_NOSYMFOLLOW, data: ""},
+
+		{isErr: true, flags: 0, data: "idmap"},
+		{isErr: true, flags: unix.MS_RDONLY, data: "custom_ext4_flag"},
+		{isErr: true, flags: unix.MS_NOATIME, data: "rw=foobar"},
+	}
+
+	for _, tc := range testCases {
+		for _, bind := range []string{"bind", "rbind"} {
+			bindFlag := map[string]int{
+				"bind":  unix.MS_BIND,
+				"rbind": unix.MS_BIND | unix.MS_REC,
+			}[bind]
+
+			config := &configs.Config{
+				Rootfs: "/var",
+				Mounts: []*configs.Mount{
+					{
+						Destination: "/",
+						Flags:       tc.flags | bindFlag,
+						Data:        tc.data,
+					},
+				},
+			}
+
+			err := Validate(config)
+			if tc.isErr && err == nil {
+				t.Errorf("%s mount flags:0x%x data:%v, expected error, got nil", bind, tc.flags, tc.data)
+			}
+			if !tc.isErr && err != nil {
+				t.Errorf("%s mount flags:0x%x data:%v, expected nil, got error %v", bind, tc.flags, tc.data, err)
+			}
+		}
+	}
+}
+
 func TestValidateIDMapMounts(t *testing.T) {
 	mapping := []configs.IDMap{
 		{