Fix panic when overriding /dev files with VFS2

VFS1 skips over mounts that overrides files in /dev because the list of
files is hardcoded. This is not needed for VFS2 and a recent change
lifted this restriction. However, parts of the code were still skipping
/dev mounts even in VFS2, causing the loader to panic when it ran short
of FDs to connect to the gofer.

PiperOrigin-RevId: 365858436

runsc/boot/fs.go

@@ -110,7 +110,7 @@ func compileMounts(spec *specs.Spec, vfs2Enabled bool) []specs.Mount {
 
 	// Mount all submounts from the spec.
 	for _, m := range spec.Mounts {
-		if !vfs2Enabled && !specutils.IsVFS1SupportedDevMount(m) {
+		if !specutils.IsSupportedDevMount(m, vfs2Enabled) {
 			log.Warningf("ignoring dev mount at %q", m.Destination)
 			continue
 		}

runsc/cmd/gofer.go

@@ -176,7 +176,7 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 
 	mountIdx := 1 // first one is the root
 	for _, m := range spec.Mounts {
-		if specutils.Is9PMount(m) {
+		if specutils.Is9PMount(m, conf.VFS2) {
 			cfg := fsgofer.Config{
 				ROMount:           isReadonlyMount(m.Options) || conf.Overlay,
 				HostUDS:           conf.FSGoferHostUDS,
@@ -350,7 +350,7 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 // creates directories as needed.
 func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
 	for _, m := range mounts {
-		if m.Type != "bind" || !specutils.IsVFS1SupportedDevMount(m) {
+		if !specutils.Is9PMount(m, conf.VFS2) {
 			continue
 		}
 
@@ -390,7 +390,7 @@ func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
 func resolveMounts(conf *config.Config, mounts []specs.Mount, root string) ([]specs.Mount, error) {
 	cleanMounts := make([]specs.Mount, 0, len(mounts))
 	for _, m := range mounts {
-		if m.Type != "bind" || !specutils.IsVFS1SupportedDevMount(m) {
+		if !specutils.Is9PMount(m, conf.VFS2) {
 			cleanMounts = append(cleanMounts, m)
 			continue
 		}

runsc/container/container.go

@@ -886,7 +886,7 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *config.Config, bu
 	// Add root mount and then add any other additional mounts.
 	mountCount := 1
 	for _, m := range spec.Mounts {
-		if specutils.Is9PMount(m) {
+		if specutils.Is9PMount(m, conf.VFS2) {
 			mountCount++
 		}
 	}

runsc/specutils/specutils.go

@@ -332,14 +332,20 @@ func capsFromNames(names []string, skipSet map[linux.Capability]struct{}) (auth.
 	return auth.CapabilitySetOfMany(caps), nil
 }
 
-// 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 != "" && IsVFS1SupportedDevMount(m)
+// Is9PMount returns true if the given mount can be mounted as an external
+// gofer.
+func Is9PMount(m specs.Mount, vfs2Enabled bool) bool {
+	return m.Type == "bind" && m.Source != "" && IsSupportedDevMount(m, vfs2Enabled)
 }
 
-// IsVFS1SupportedDevMount returns true if m.Destination does not specify a
+// IsSupportedDevMount returns true if m.Destination does not specify a
 // path that is hardcoded by VFS1's implementation of /dev.
-func IsVFS1SupportedDevMount(m specs.Mount) bool {
+func IsSupportedDevMount(m specs.Mount, vfs2Enabled bool) bool {
+	// VFS2 has no hardcoded files under /dev, so everything is allowed.
+	if vfs2Enabled {
+		return true
+	}
+
 	// See pkg/sentry/fs/dev/dev.go.
 	var existingDevices = []string{
 		"/dev/fd", "/dev/stdin", "/dev/stdout", "/dev/stderr",

test/e2e/integration_test.go

@@ -399,15 +399,15 @@ func TestTmpFile(t *testing.T) {
 
 // TestTmpMount checks that mounts inside '/tmp' are not overridden.
 func TestTmpMount(t *testing.T) {
-	ctx := context.Background()
 	dir, err := ioutil.TempDir(testutil.TmpDir(), "tmp-mount")
 	if err != nil {
 		t.Fatalf("TempDir(): %v", err)
 	}
-	want := "123"
+	const want = "123"
 	if err := ioutil.WriteFile(filepath.Join(dir, "file.txt"), []byte("123"), 0666); err != nil {
 		t.Fatalf("WriteFile(): %v", err)
 	}
+	ctx := context.Background()
 	d := dockerutil.MakeContainer(ctx, t)
 	defer d.CleanUp(ctx)
 
@@ -430,6 +430,48 @@ func TestTmpMount(t *testing.T) {
 	}
 }
 
+// Test that it is allowed to mount a file on top of /dev files, e.g.
+// /dev/random.
+func TestMountOverDev(t *testing.T) {
+	if usingVFS2, err := dockerutil.UsingVFS2(); !usingVFS2 {
+		t.Skip("VFS1 doesn't allow /dev/random to be mounted.")
+	} else if err != nil {
+		t.Fatalf("Failed to read config for runtime %s: %v", dockerutil.Runtime(), err)
+	}
+
+	random, err := ioutil.TempFile(testutil.TmpDir(), "random")
+	if err != nil {
+		t.Fatal("ioutil.TempFile() failed:", err)
+	}
+	const want = "123"
+	if _, err := random.WriteString(want); err != nil {
+		t.Fatalf("WriteString() to %q: %v", random.Name(), err)
+	}
+
+	ctx := context.Background()
+	d := dockerutil.MakeContainer(ctx, t)
+	defer d.CleanUp(ctx)
+
+	opts := dockerutil.RunOpts{
+		Image: "basic/alpine",
+		Mounts: []mount.Mount{
+			{
+				Type:   mount.TypeBind,
+				Source: random.Name(),
+				Target: "/dev/random",
+			},
+		},
+	}
+	cmd := "dd count=1 bs=5 if=/dev/random 2> /dev/null"
+	got, err := d.Run(ctx, opts, "sh", "-c", cmd)
+	if err != nil {
+		t.Fatalf("docker run failed: %v", err)
+	}
+	if want != got {
+		t.Errorf("invalid file content, want: %q, got: %q", want, got)
+	}
+}
+
 // TestSyntheticDirs checks that submounts can be created inside a readonly
 // mount even if the target path does not exist.
 func TestSyntheticDirs(t *testing.T) {