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

PiperOrigin-RevId: 200768923
Change-Id: I4b8da10bcac296e8171fe6754abec5aabfec5e65

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,29 +79,6 @@ 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
 
-	// 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") {
-			log.Warningf("ignoring dev mount at %q", m.Destination)
-			continue
-		}
-		switch m.Destination {
-		case "/proc":
-			procMounted = true
-		case "/sys":
-			sysMounted = true
-		case "/tmp":
-			tmpMounted = true
-		}
-
-		if err := mountSubmount(ctx, spec, conf, mns, fds, m); err != nil {
-			return err
-		}
-	}
-
 	// Always mount /dev.
 	if err := mountSubmount(ctx, spec, conf, mns, nil, specs.Mount{
 		Type:        "devtmpfs",
@@ -117,6 +95,26 @@ func configureMounts(ctx context.Context, spec *specs.Spec, conf *Config, mns *f
 		return err
 	}
 
+	// Mount all submounts from the spec.
+	for _, m := range spec.Mounts {
+		if !specutils.IsSupportedDevMount(m) {
+			log.Warningf("ignoring dev mount at %q", m.Destination)
+			continue
+		}
+		switch filepath.Clean(m.Destination) {
+		case "/proc":
+			procMounted = true
+		case "/sys":
+			sysMounted = true
+		case "/tmp":
+			tmpMounted = true
+		}
+
+		if err := mountSubmount(ctx, spec, conf, mns, fds, m); 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)
 		}
 	}
 

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 {

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