runsc: support /dev bind mount which does not conflict with default /dev mount.

PiperOrigin-RevId: 200768923
Change-Id: I4b8da10bcac296e8171fe6754abec5aabfec5e65

3 files changed, 108 insertions, 34 deletions

diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index 3113f1857..7786e4d4a 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -35,6 +35,7 @@ import (
 	"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
 	"gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs"
 	"gvisor.googlesource.com/gvisor/pkg/syserror"
+	"gvisor.googlesource.com/gvisor/runsc/specutils"
 )
 
 type fdDispenser struct {
@@ -78,16 +79,29 @@ func configureMounts(ctx context.Context, spec *specs.Spec, conf *Config, mns *f
 	// Keep track of whether proc, sys, and tmp were mounted.
 	var procMounted, sysMounted, tmpMounted bool
 
+	// Always mount /dev.
+	if err := mountSubmount(ctx, spec, conf, mns, nil, specs.Mount{
+		Type:        "devtmpfs",
+		Destination: "/dev",
+	}); err != nil {
+		return err
+	}
+
+	// Always mount /dev/pts.
+	if err := mountSubmount(ctx, spec, conf, mns, nil, specs.Mount{
+		Type:        "devpts",
+		Destination: "/dev/pts",
+	}); err != nil {
+		return err
+	}
+
 	// Mount all submounts from the spec.
 	for _, m := range spec.Mounts {
-		// OCI spec uses many different mounts for the things inside of '/dev'. We
-		// have a single mount at '/dev' that is always mounted, regardless of
-		// whether it was asked for, as the spec says we SHOULD.
-		if strings.HasPrefix(m.Destination, "/dev") {
+		if !specutils.IsSupportedDevMount(m) {
 			log.Warningf("ignoring dev mount at %q", m.Destination)
 			continue
 		}
-		switch m.Destination {
+		switch filepath.Clean(m.Destination) {
 		case "/proc":
 			procMounted = true
 		case "/sys":
@@ -101,22 +115,6 @@ func configureMounts(ctx context.Context, spec *specs.Spec, conf *Config, mns *f
 		}
 	}
 
-	// Always mount /dev.
-	if err := mountSubmount(ctx, spec, conf, mns, nil, specs.Mount{
-		Type:        "devtmpfs",
-		Destination: "/dev",
-	}); err != nil {
-		return err
-	}
-
-	// Always mount /dev/pts.
-	if err := mountSubmount(ctx, spec, conf, mns, nil, specs.Mount{
-		Type:        "devpts",
-		Destination: "/dev/pts",
-	}); err != nil {
-		return err
-	}
-
 	// Mount proc and sys even if the user did not ask for it, as the spec
 	// says we SHOULD.
 	if !procMounted {
@@ -282,18 +280,13 @@ func mountSubmount(ctx context.Context, spec *specs.Spec, conf *Config, mns *fs.
 
 	// If there are submounts, we need to overlay the mount on top of a
 	// ramfs with stub directories for submount paths.
-	//
-	// We do not do this for /dev, since there will usually be submounts in
-	// the spec, but our devfs implementation contains all the necessary
-	// directories and files (well, most of them anyways).
-	if m.Destination != "/dev" {
-		submounts := subtargets(m.Destination, spec.Mounts)
-		if len(submounts) > 0 {
-			log.Infof("Adding submount overlay over %q", m.Destination)
-			inode, err = addSubmountOverlay(ctx, inode, submounts)
-			if err != nil {
-				return fmt.Errorf("error adding submount overlay: %v", err)
-			}
+	mounts := specutils.SupportedMounts(spec.Mounts)
+	submounts := subtargets(m.Destination, mounts)
+	if len(submounts) > 0 {
+		log.Infof("Adding submount overlay over %q", m.Destination)
+		inode, err = addSubmountOverlay(ctx, inode, submounts)
+		if err != nil {
+			return fmt.Errorf("error adding submount overlay: %v", err)
 		}
 	}
 
diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go
index ca78c2cd6..a7f59f775 100644
--- a/runsc/boot/loader_test.go
+++ b/runsc/boot/loader_test.go
@@ -256,6 +256,51 @@ func TestCreateMountNamespace(t *testing.T) {
 			expectedPaths: []string{"/foo", "/foo/bar", "/foo/bar/baz", "/foo/qux",
 				"/foo/qux-quz", "/foo/some/very/very/deep/path", "/proc", "/dev", "/sys"},
 		},
+		{
+			name: "mount inside /dev",
+			spec: specs.Spec{
+				Root: &specs.Root{
+					Path:     os.TempDir(),
+					Readonly: true,
+				},
+				Mounts: []specs.Mount{
+					{
+						Destination: "/proc",
+						Type:        "tmpfs",
+					},
+					{
+						Destination: "/dev",
+						Type:        "tmpfs",
+					},
+					{
+						// Mounted by runsc by default.
+						Destination: "/dev/fd",
+						Type:        "tmpfs",
+					},
+					{
+						// Mount with the same prefix.
+						Destination: "/dev/fd-foo",
+						Source:      testFile.Name(),
+						Type:        "bind",
+					},
+					{
+						// Unsupported fs type.
+						Destination: "/dev/mqueue",
+						Type:        "mqueue",
+					},
+					{
+						Destination: "/dev/foo",
+						Type:        "tmpfs",
+					},
+					{
+						Destination: "/dev/bar",
+						Source:      testFile.Name(),
+						Type:        "bind",
+					},
+				},
+			},
+			expectedPaths: []string{"/proc", "/dev", "/dev/fd-foo", "/dev/foo", "/dev/bar", "/sys"},
+		},
 	}
 
 	for _, tc := range testCases {
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index 3161360b4..0bb462eb5 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -195,7 +195,43 @@ func capsFromNames(names []string) (auth.CapabilitySet, error) {
 
 // Is9PMount returns true if the given mount can be mounted as an external gofer.
 func Is9PMount(m specs.Mount) bool {
-	return m.Type == "bind" && m.Source != "" && !strings.HasPrefix(m.Destination, "/dev")
+	return m.Type == "bind" && m.Source != "" && IsSupportedDevMount(m)
+}
+
+// IsSupportedDevMount returns true if the mount is a supported /dev mount.
+// Only mount that does not conflict with runsc default /dev mount is
+// supported.
+func IsSupportedDevMount(m specs.Mount) bool {
+	// These are devices exist inside sentry. See pkg/sentry/fs/dev/dev.go
+	var existingDevices = []string{
+		"/dev/fd", "/dev/stdin", "/dev/stdout", "/dev/stderr",
+		"/dev/null", "/dev/zero", "/dev/full", "/dev/random",
+		"/dev/urandom", "/dev/shm", "/dev/pts", "/dev/ptmx",
+	}
+	dst := filepath.Clean(m.Destination)
+	if dst == "/dev" {
+		// OCI spec uses many different mounts for the things inside of '/dev'. We
+		// have a single mount at '/dev' that is always mounted, regardless of
+		// whether it was asked for, as the spec says we SHOULD.
+		return false
+	}
+	for _, dev := range existingDevices {
+		if dst == dev || strings.HasPrefix(dst, dev+"/") {
+			return false
+		}
+	}
+	return true
+}
+
+// SupportedMounts filters out unsupported mounts.
+func SupportedMounts(mounts []specs.Mount) []specs.Mount {
+	var newMounts []specs.Mount
+	for _, m := range mounts {
+		if IsSupportedDevMount(m) {
+			newMounts = append(newMounts, m)
+		}
+	}
+	return newMounts
 }
 
 // BinPath returns the real path to self, resolving symbolink links. This is done