From 59483ac4df08fb189eb10b93580a115f5dde7137 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Wed, 15 Nov 2023 19:11:22 -0800
Subject: [PATCH] Fix how mount options are consumed for bind mounts.

This change fixes 2 issues:
- Earlier we were consuming certain mount options in spec.Mount.Options via
  parseAndFilterOptions(), but not removing the consumed options from the list.
  This was causing a lot of "ignoring unknown mount option" logs, even though
  the mount option was applied. Changed parseAndFilterOptions() to return both
  the remaining and consumed options. Only remaining options are passed ahead.
- Gofer mount options like `overlayfs_stale_read` were being ignored.
  runsc/cmd/gofer.go:adjustMountOptions() was setting this option if the
  host filesystem was an overlayfs. Added gofer specific logic to consume
  certain support mount options.

PiperOrigin-RevId: 582878231
---
 pkg/sentry/fsimpl/gofer/gofer.go |  3 +++
 runsc/boot/vfs.go                | 28 ++++++++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index ff72cbac1..5a5e9aa29 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -98,6 +98,9 @@ const (
 	cacheRemoteRevalidating  = "remote_revalidating"
 )
 
+// SupportedMountOptions is the set of mount options that can be set externally.
+var SupportedMountOptions = []string{moptOverlayfsStaleRead, moptDisableFileHandleSharing}
+
 const (
 	defaultMaxCachedDentries  = 1000
 	maxCachedNegativeChildren = 1000
diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go
index 4f42e1886..8844a0c54 100644
--- a/runsc/boot/vfs.go
+++ b/runsc/boot/vfs.go
@@ -300,20 +300,22 @@ func goferMountData(fd int, fa config.FileAccessType, conf *config.Config) []str
 	return opts
 }
 
-// parseAndFilterOptions parses a MountOptions slice and filters by the allowed
-// keys.
-func parseAndFilterOptions(opts []string, allowedKeys ...string) ([]string, error) {
-	var out []string
+// consumeMountOptions consumes mount options from opts based on allowedKeys
+// and returns the remaining and consumed options.
+func consumeMountOptions(opts []string, allowedKeys ...string) ([]string, []string, error) {
+	var rem, out []string
 	for _, o := range opts {
 		ok, err := parseMountOption(o, allowedKeys...)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		if ok {
 			out = append(out, o)
+		} else {
+			rem = append(rem, o)
 		}
 	}
-	return out, nil
+	return rem, out, nil
 }
 
 func parseMountOption(opt string, allowedKeys ...string) (bool, error) {
@@ -826,6 +828,7 @@ func (c *containerMounter) mountSubmount(ctx context.Context, spec *specs.Spec,
 func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, productName string) (string, *vfs.MountOptions, error) {
 	fsName := m.mount.Type
 	var (
+		mopts        = m.mount.Options
 		data         []string
 		internalData any
 	)
@@ -847,7 +850,7 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo,
 
 	case tmpfs.Name:
 		var err error
-		data, err = parseAndFilterOptions(m.mount.Options, tmpfsAllowedData...)
+		mopts, data, err = consumeMountOptions(mopts, tmpfsAllowedData...)
 		if err != nil {
 			return "", nil, err
 		}
@@ -866,14 +869,19 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo,
 			// Check that an FD was provided to fails fast.
 			return "", nil, fmt.Errorf("gofer mount requires a connection FD")
 		}
-		data = goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf)
+		var err error
+		mopts, data, err = consumeMountOptions(mopts, gofer.SupportedMountOptions...)
+		if err != nil {
+			return "", nil, err
+		}
+		data = append(data, goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf)...)
 		internalData = gofer.InternalFilesystemOptions{
 			UniqueID: m.mount.Destination,
 		}
 
 	case cgroupfs.Name:
 		var err error
-		data, err = parseAndFilterOptions(m.mount.Options, cgroupfs.SupportedMountOptions...)
+		mopts, data, err = consumeMountOptions(mopts, cgroupfs.SupportedMountOptions...)
 		if err != nil {
 			return "", nil, err
 		}
@@ -883,7 +891,7 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo,
 		return "", nil, nil
 	}
 
-	opts := ParseMountOptions(m.mount.Options)
+	opts := ParseMountOptions(mopts)
 	opts.GetFilesystemOptions = vfs.GetFilesystemOptions{
 		Data:          strings.Join(data, ","),
 		InternalData:  internalData,