runsc: Mount "mandatory" mounts right after mounting the root.

The /proc and /sys mounts are "mandatory" in the sense that they should be
mounted in the sandbox even when they are not included in the spec. Runsc
treats /tmp similarly, because it is faster to use the internal tmpfs
implementation instead of proxying to the host.

However, the spec may contain submounts of these mandatory mounts (particularly
for /tmp). In those cases, we must mount our mandatory mounts before the
submount, otherwise the submount will be masked.

Since the mandatory mounts are all top-level directories, we can mount them
right after the root.

PiperOrigin-RevId: 203145635
Change-Id: Id69bae771d32c1a5b67e08c8131b73d9b42b2fbf

2 files changed, 51 insertions, 10 deletions

diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index a9b2f225a..f36bcdc2e 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -83,7 +83,7 @@ func createMountNamespace(userCtx context.Context, rootCtx context.Context, spec
 }
 
 // compileMounts returns the supported mounts from the mount spec, adding any
-// additional mounts that are required by the OCI specification.
+// mandatory mounts that are required by the OCI specification.
 func compileMounts(spec *specs.Spec) []specs.Mount {
 	// Keep track of whether proc, sys, and tmp were mounted.
 	var procMounted, sysMounted, tmpMounted bool
@@ -119,14 +119,15 @@ func compileMounts(spec *specs.Spec) []specs.Mount {
 
 	// Mount proc and sys even if the user did not ask for it, as the spec
 	// says we SHOULD.
+	var mandatoryMounts []specs.Mount
 	if !procMounted {
-		mounts = append(mounts, specs.Mount{
+		mandatoryMounts = append(mandatoryMounts, specs.Mount{
 			Type:        "proc",
 			Destination: "/proc",
 		})
 	}
 	if !sysMounted {
-		mounts = append(mounts, specs.Mount{
+		mandatoryMounts = append(mandatoryMounts, specs.Mount{
 			Type:        "sysfs",
 			Destination: "/sys",
 		})
@@ -136,11 +137,20 @@ func compileMounts(spec *specs.Spec) []specs.Mount {
 	// rely on the host /tmp, but this is a nice optimization, and fixes
 	// some apps that call mknod in /tmp.
 	if !tmpMounted {
-		mounts = append(mounts, specs.Mount{
+		// TODO: If the host /tmp (or a mount at /tmp) has
+		// files in it, we should overlay our tmpfs implementation over
+		// that. Until then, the /tmp mount will always appear empty at
+		// container creation.
+		mandatoryMounts = append(mandatoryMounts, specs.Mount{
 			Type:        "tmpfs",
 			Destination: "/tmp",
 		})
 	}
+
+	// The mandatory mounts should be ordered right after the root, in case
+	// there are submounts of these mandatory mounts already in the spec.
+	mounts = append(mounts[:0], append(mandatoryMounts, mounts[0:]...)...)
+
 	return mounts
 }
 
@@ -430,8 +440,8 @@ func addRestoreMount(conf *Config, renv *fs.RestoreEnvironment, m specs.Mount, f
 	if err != nil {
 		return err
 	}
-	// TODO: Fix this when we support all the mount types and make this a
-	// fatal error.
+	// TODO: Fix this when we support all the mount types and
+	// make this a fatal error.
 	if fsName == "" {
 		return nil
 	}
diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go
index 28d45b54b..30ec236e4 100644
--- a/runsc/boot/loader_test.go
+++ b/runsc/boot/loader_test.go
@@ -304,6 +304,37 @@ func TestCreateMountNamespace(t *testing.T) {
 			},
 			expectedPaths: []string{"/proc", "/dev", "/dev/fd-foo", "/dev/foo", "/dev/bar", "/sys"},
 		},
+		{
+			name: "mounts inside mandatory mounts",
+			spec: specs.Spec{
+				Root: &specs.Root{
+					Path:     os.TempDir(),
+					Readonly: true,
+				},
+				Mounts: []specs.Mount{
+					{
+						Destination: "/proc",
+						Type:        "tmpfs",
+					},
+					// We don't include /sys, and /tmp in
+					// the spec, since they will be added
+					// automatically.
+					//
+					// Instead, add submounts inside these
+					// directories and make sure they are
+					// visible under the mandatory mounts.
+					{
+						Destination: "/sys/bar",
+						Type:        "tmpfs",
+					},
+					{
+						Destination: "/tmp/baz",
+						Type:        "tmpfs",
+					},
+				},
+			},
+			expectedPaths: []string{"/proc", "/sys", "/sys/bar", "/tmp", "/tmp/baz"},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -495,13 +526,13 @@ func TestRestoreEnvironment(t *testing.T) {
 					},
 					"tmpfs": {
 						{
+							Dev: "none",
+						},
+						{
 							Dev:   "none",
 							Flags: fs.MountSourceFlags{NoAtime: true},
 							Data:  "uid=1022",
 						},
-						{
-							Dev: "none",
-						},
 					},
 					"devtmpfs": {
 						{
@@ -587,7 +618,7 @@ func TestRestoreEnvironment(t *testing.T) {
 			}
 		} else {
 			if !reflect.DeepEqual(*actualRenv, tc.expectedRenv) {
-				t.Fatalf("restore environments did not match for test:%s", tc.name)
+				t.Fatalf("restore environments did not match for test:%s\ngot:%+v\nwant:%+v\n", tc.name, *actualRenv, tc.expectedRenv)
 			}
 		}
 	}