From e69c018749edd7c42098008ffd14a351060a3150 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Fri, 14 Apr 2023 17:28:16 -0700
Subject: [PATCH] Only ignore invalid mount options in mount annotations.

As of right now, if the mount options in the mount annotation contains even one
invalid option, then all the mount options are ignored. This may not be
desirable. Only ignore the invalid options.

PiperOrigin-RevId: 524424320
---
 runsc/boot/mount_hints.go      | 11 +----------
 runsc/boot/mount_hints_test.go | 19 ++++++++++++++++++
 runsc/specutils/fs.go          | 36 +++++++++++++++++++++++++---------
 3 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go
index 61a076350..6c7817a00 100644
--- a/runsc/boot/mount_hints.go
+++ b/runsc/boot/mount_hints.go
@@ -153,7 +153,7 @@ func (m *mountHint) setField(key, val string) error {
 		}
 		m.share = share
 	case "options":
-		return m.setOptions(val)
+		m.mount.Options = specutils.FilterMountOptions(strings.Split(val, ","))
 	default:
 		return fmt.Errorf("invalid mount annotation: %s=%s", key, val)
 	}
@@ -170,15 +170,6 @@ func (m *mountHint) setType(val string) error {
 	return nil
 }
 
-func (m *mountHint) setOptions(val string) error {
-	opts := strings.Split(val, ",")
-	if err := specutils.ValidateMountOptions(opts); err != nil {
-		return err
-	}
-	m.mount.Options = opts
-	return nil
-}
-
 func (m *mountHint) isSupported() bool {
 	// TODO(b/142076984): Only support tmpfs for now. Bind mounts require a
 	// common gofer to mount all shared volumes.
diff --git a/runsc/boot/mount_hints_test.go b/runsc/boot/mount_hints_test.go
index 321fcb2b6..a0a31239a 100644
--- a/runsc/boot/mount_hints_test.go
+++ b/runsc/boot/mount_hints_test.go
@@ -169,6 +169,25 @@ func TestPodMountHintsIgnore(t *testing.T) {
 	}
 }
 
+func TestIgnoreInvalidMountOptions(t *testing.T) {
+	spec := &specs.Spec{
+		Annotations: map[string]string{
+			MountPrefix + "mount1.source":  "foo",
+			MountPrefix + "mount1.type":    "tmpfs",
+			MountPrefix + "mount1.share":   "container",
+			MountPrefix + "mount1.options": "rw,invalid,private",
+		},
+	}
+	podHints, err := newPodMountHints(spec)
+	if err != nil {
+		t.Fatalf("newPodMountHints failed: %v", err)
+	}
+	mount1 := podHints.mounts["mount1"]
+	if want := []string{"rw", "private"}; !reflect.DeepEqual(want, mount1.mount.Options) {
+		t.Errorf("mount2 type, want: %q, got: %q", want, mount1.mount.Options)
+	}
+}
+
 func TestHintsCheckCompatible(t *testing.T) {
 	for _, tc := range []struct {
 		name        string
diff --git a/runsc/specutils/fs.go b/runsc/specutils/fs.go
index 2eb92ac89..ff792e728 100644
--- a/runsc/specutils/fs.go
+++ b/runsc/specutils/fs.go
@@ -22,6 +22,7 @@ import (
 
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/log"
 )
 
 type mapping struct {
@@ -126,24 +127,41 @@ func moptKey(opt string) string {
 	return strings.SplitN(opt, "=", 2)[0]
 }
 
+// FilterMountOptions filters out all invalid mount options.
+func FilterMountOptions(opts []string) []string {
+	out := make([]string, 0, len(opts))
+	for _, o := range opts {
+		if err := validateMountOption(o); err == nil {
+			out = append(out, o)
+		} else {
+			log.Warningf("mount option skipped %q: %v", o, err)
+		}
+	}
+	return out
+}
+
 // ValidateMountOptions validates that mount options are correct.
 func ValidateMountOptions(opts []string) error {
 	for _, o := range opts {
-		if ContainsStr(invalidOptions, o) {
-			return fmt.Errorf("mount option %q is not supported", o)
-		}
-		_, ok1 := optionsMap[o]
-		_, ok2 := propOptionsMap[o]
-		if !ok1 && !ok2 {
-			return fmt.Errorf("unknown mount option %q", o)
-		}
-		if err := validatePropagation(o); err != nil {
+		if err := validateMountOption(o); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+func validateMountOption(o string) error {
+	if ContainsStr(invalidOptions, o) {
+		return fmt.Errorf("mount option %q is not supported", o)
+	}
+	_, ok1 := optionsMap[o]
+	_, ok2 := propOptionsMap[o]
+	if !ok1 && !ok2 {
+		return fmt.Errorf("unknown mount option %q", o)
+	}
+	return validatePropagation(o)
+}
+
 // ValidateRootfsPropagation validates that rootfs propagation options are
 // correct.
 func validateRootfsPropagation(opt string) error {