merge #3990 into opencontainers/runc:main

Aleksa Sarai (1):
  configs: validate: add validation for bind-mount fsflags

LGTMs: kolyshkin AkihiroSuda

Author: cyphar

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

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{
 		{