Add mount locking.

Mounts that come from a more privileged namespace must be locked so that they
cannot be unmounted from a less privileged namespace. This an important
consequence of having mount namespaces + mount propagation. See
https://man7.org/linux/man-pages/man7/mount_namespaces.7.html for full
detail.

PiperOrigin-RevId: 597322843

pkg/sentry/vfs/mount.go

@@ -118,6 +118,10 @@ type Mount struct {
 	// umounted is true. umounted is protected by VirtualFilesystem.mountMu.
 	umounted bool
 
+	// locked is true if the mount cannot be unmounted in the current mount
+	// namespace. It is analogous to MNT_LOCKED in Linux.
+	locked bool
+
 	// The lower 63 bits of writers is the number of calls to
 	// Mount.CheckBeginWrite() that have not yet been paired with a call to
 	// Mount.EndWrite(). The MSB of writers is set if MS_RDONLY is in effect.
@@ -133,6 +137,7 @@ func newMount(vfs *VirtualFilesystem, fs *Filesystem, root *Dentry, mntns *Mount
 		fs:       fs,
 		root:     root,
 		ns:       mntns,
+		locked:   opts.Locked,
 		isShared: false,
 		refs:     atomicbitops.FromInt64(1),
 	}
@@ -324,12 +329,45 @@ func (vfs *VirtualFilesystem) attachTreeLocked(ctx context.Context, mnt *Mount,
 	vfs.mounts.seq.EndWrite()
 	mp.dentry.mu.Unlock()
 	vfs.commitChildren(ctx, mnt)
+
+	var owner *auth.UserNamespace
+	if mntns := MountNamespaceFromContext(ctx); mntns != nil {
+		owner = mntns.Owner
+		mntns.DecRef(ctx)
+	}
 	for pmnt := range propMnts {
 		vfs.commitMount(ctx, pmnt)
+		if pmnt.parent().ns.Owner != owner {
+			vfs.lockMountTree(pmnt)
+		}
+		pmnt.locked = false
 	}
 	return nil
 }
 
+// +checklocks:vfs.mountMu
+func (vfs *VirtualFilesystem) lockMountTree(mnt *Mount) {
+	for _, m := range mnt.submountsLocked() {
+		// TODO(b/315839347): Add equivalents for MNT_LOCK_ATIME,
+		// MNT_LOCK_READONLY, etc.
+		m.locked = true
+	}
+}
+
+// +checklocks:vfs.mountMu
+func (vfs *VirtualFilesystem) mountHasLockedChildren(mnt *Mount, vd VirtualDentry) bool {
+	for child := range mnt.children {
+		mp := child.getKey()
+		if !mp.mount.fs.Impl().IsDescendant(vd, mp) {
+			continue
+		}
+		if child.locked {
+			return true
+		}
+	}
+	return false
+}
+
 // ConnectMountAt connects mnt at the path represented by target.
 //
 // Preconditions: mnt must be disconnected.
@@ -435,6 +473,7 @@ func (vfs *VirtualFilesystem) cloneMount(mnt *Mount, root *Dentry, mopts *MountO
 		}
 	}
 	clone.isShared = mnt.isShared
+	clone.locked = mnt.locked
 	if cloneType&makeFollowerClone != 0 || (cloneType&sharedToFollowerClone != 0 && mnt.isShared) {
 		mnt.followerList.PushFront(clone)
 		clone.leader = mnt
@@ -548,6 +587,9 @@ func (vfs *VirtualFilesystem) BindAt(ctx context.Context, creds *auth.Credential
 	if recursive {
 		clone, err = vfs.cloneMountTree(ctx, sourceVd.mount, sourceVd.dentry, 0, nil)
 	} else {
+		if vfs.mountHasLockedChildren(sourceVd.mount, sourceVd) {
+			return linuxerr.EINVAL
+		}
 		clone, err = vfs.cloneMount(sourceVd.mount, sourceVd.dentry, nil, 0)
 	}
 	if err != nil {
@@ -556,6 +598,7 @@ func (vfs *VirtualFilesystem) BindAt(ctx context.Context, creds *auth.Credential
 	cleanup.Release()
 
 	vfs.delayDecRef(clone)
+	clone.locked = false
 	if err := vfs.attachTreeLocked(ctx, clone, mp); err != nil {
 		vfs.abortUncomittedChildren(ctx, clone)
 		return err
@@ -618,6 +661,9 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti
 
 	vfs.lockMounts()
 	defer vfs.unlockMounts(ctx)
+	if vd.mount.locked {
+		return linuxerr.EINVAL
+	}
 	if !vfs.validInMountNS(ctx, vd.mount) {
 		return linuxerr.EINVAL
 	}
@@ -694,7 +740,7 @@ func (vfs *VirtualFilesystem) shouldUmount(mnt *Mount, opts *umountRecursiveOpti
 	if mnt.parent() == nil {
 		return true
 	}
-	// Always unmount if the parent is nor marked as unmounted.
+	// Always unmount if the parent is not marked as unmounted.
 	if !mnt.parent().umounted {
 		return true
 	}
@@ -703,6 +749,9 @@ func (vfs *VirtualFilesystem) shouldUmount(mnt *Mount, opts *umountRecursiveOpti
 	if !opts.disconnectHierarchy {
 		return false
 	}
+	if mnt.locked {
+		return false
+	}
 	return true
 }
 
@@ -711,6 +760,9 @@ func (vfs *VirtualFilesystem) shouldUmount(mnt *Mount, opts *umountRecursiveOpti
 // umountTreeLocked is analogous to Linux's fs/namespace.c:umount_tree().
 // +checklocks:vfs.mountMu
 func (vfs *VirtualFilesystem) umountTreeLocked(mnt *Mount, opts *umountRecursiveOptions) {
+	if opts.propagate {
+		vfs.unlockPropagationMounts(mnt)
+	}
 	umountMnts := mnt.submountsLocked()
 	for _, mnt := range umountMnts {
 		vfs.umount(mnt)
@@ -733,12 +785,17 @@ func (vfs *VirtualFilesystem) umountTreeLocked(mnt *Mount, opts *umountRecursive
 			}
 		}
 		if mnt.parent() != nil {
+			vfs.delayDecRef(mnt.getKey())
 			if vfs.shouldUmount(mnt, opts) {
-				vfs.delayDecRef(vfs.disconnectLocked(mnt))
+				vfs.disconnectLocked(mnt)
 			} else {
-				// Restore mnt in it's parent children list, but leave it marked as
-				// unmounted. These partly unmounted mounts are cleaned up in
-				// vfs.forgetDeadMountpoints and Mount.destroy.
+				// Restore mnt in it's parent children list with a reference, but leave
+				// it marked as unmounted. These partly unmounted mounts are cleaned up
+				// in vfs.forgetDeadMountpoints and Mount.destroy. We keep the extra
+				// reference on the mount but remove a reference on the mount point so
+				// that mount.Destroy is called when there are no other references on
+				// the parent.
+				mnt.IncRef()
 				mnt.parent().children[mnt] = struct{}{}
 			}
 		}
@@ -899,7 +956,8 @@ func (mnt *Mount) destroy(ctx context.Context) {
 		mnt.vfs.mounts.seq.EndWrite()
 	}
 
-	// Cleanup any leftover children.
+	// Cleanup any leftover children. The mount point has already been decref'd in
+	// umount so we just need to clean up the actual mounts.
 	if len(mnt.children) != 0 {
 		mnt.vfs.mounts.seq.BeginWrite()
 		for child := range mnt.children {
@@ -908,10 +966,8 @@ func (mnt *Mount) destroy(ctx context.Context) {
 					panic("children of a mount that has no references should already be marked as unmounted.")
 				}
 			}
-			vd := mnt.vfs.disconnectLocked(child)
-			if vd.Ok() {
-				mnt.vfs.delayDecRef(vd)
-			}
+			mnt.vfs.disconnectLocked(child)
+			mnt.vfs.delayDecRef(child)
 		}
 		mnt.vfs.mounts.seq.EndWrite()
 	}
@@ -1127,6 +1183,10 @@ func (vfs *VirtualFilesystem) PivotRoot(ctx context.Context, creds *auth.Credent
 	if newRoot.mount.root != newRoot.dentry {
 		return newRoot, oldRoot, linuxerr.EINVAL
 	}
+	// new_root must not be locked.
+	if newRoot.mount.locked {
+		return newRoot, oldRoot, linuxerr.EINVAL
+	}
 	// put_old must be at or underneath new_root.
 	if !vfs.isPathReachable(ctx, newRoot, putOld) {
 		return newRoot, oldRoot, linuxerr.EINVAL
@@ -1163,6 +1223,10 @@ func (vfs *VirtualFilesystem) PivotRoot(ctx context.Context, creds *auth.Credent
 	mp := vfs.disconnectLocked(newRoot.mount)
 	vfs.delayDecRef(mp)
 	rootMp := vfs.disconnectLocked(oldRoot.mount)
+	if oldRoot.mount.locked {
+		newRoot.mount.locked = true
+		oldRoot.mount.locked = false
+	}
 
 	putOld.IncRef()
 	vfs.connectLocked(oldRoot.mount, putOld, putOld.mount.ns)

pkg/sentry/vfs/namespace.go

@@ -188,6 +188,9 @@ func (vfs *VirtualFilesystem) CloneMountNamespace(
 	newns.root = newRoot
 	newns.root.ns = newns
 	vfs.commitChildren(ctx, newRoot)
+	if ns.Owner != newns.Owner {
+		vfs.lockMountTree(newRoot)
+	}
 	return newns, nil
 }
 

pkg/sentry/vfs/options.go

@@ -112,6 +112,10 @@ type MountOptions struct {
 
 	// GetFilesystemOptions contains options to FilesystemType.GetFilesystem().
 	GetFilesystemOptions GetFilesystemOptions
+
+	// Locked determines whether to lock this mount so it cannot be unmounted by
+	// normal user processes.
+	Locked bool
 }
 
 // OpenOptions contains options to VirtualFilesystem.OpenAt() and

pkg/sentry/vfs/propagation.go

@@ -515,6 +515,7 @@ func (vfs *VirtualFilesystem) propagateUmount(mnts []*Mount) []*Mount {
 		umountRestore
 	)
 	var toUmount []*Mount
+	noChildren := make(map[*Mount]struct{})
 	// Processed contains all the mounts that the algorithm has processed so far.
 	// If the mount maps to umountRestore, it should be restored after processing
 	// all the mounts. This happens in cases where a mount was speculatively
@@ -565,7 +566,7 @@ func (vfs *VirtualFilesystem) propagateUmount(mnts []*Mount) []*Mount {
 			// encounters a parent that's been visited.
 		loop:
 			for {
-				if child.umounted {
+				if _, ok := noChildren[child]; ok || child.umounted {
 					break
 				}
 				// If there are any children that have mountpoint != parent's root then
@@ -574,15 +575,25 @@ func (vfs *VirtualFilesystem) propagateUmount(mnts []*Mount) []*Mount {
 					if gchild.point() == child.root {
 						continue
 					}
+					_, isProcessed := processed[gchild]
+					_, hasNoChildren := noChildren[gchild]
+					if isProcessed && hasNoChildren {
+						continue
+					}
 					processed[child] = umountRestore
 					break loop
 				}
-				vfs.umount(child)
-				toUmount = append(toUmount, child)
-				child = child.parent()
+				if child.locked {
+					processed[child] = umountRestore
+					noChildren[child] = struct{}{}
+				} else {
+					vfs.umount(child)
+					toUmount = append(toUmount, child)
+				}
 				// If this parent was a mount that had to be restored because it had
 				// children, it might be safe to umount now that its child is gone. If
 				// it has been visited then it's already being umounted.
+				child = child.parent()
 				if _, ok := processed[child]; !ok {
 					break
 				}
@@ -623,6 +634,25 @@ func (vfs *VirtualFilesystem) propagateUmount(mnts []*Mount) []*Mount {
 	return toUmount
 }
 
+// unlockPropagationMounts sets locked to false for every mount that a umount
+// of mnt propagates to. It is analogous to fs/pnode.c:propagate_mount_unlock()
+// in Linux.
+//
+// +checklocks:vfs.mountMu
+func (vfs *VirtualFilesystem) unlockPropagationMounts(mnt *Mount) {
+	parent := mnt.parent()
+	if parent == nil {
+		return
+	}
+	for m := nextPropMount(parent, parent); m != nil; m = nextPropMount(m, parent) {
+		child := vfs.mounts.Lookup(m, mnt.point())
+		if child == nil {
+			continue
+		}
+		child.locked = false
+	}
+}
+
 // peerUnderRoot iterates through mnt's peers until it finds a mount that is in
 // ns and is reachable from root. This method is analogous to
 // fs/pnode.c:get_peer_under_root() in Linux.

runsc/boot/vfs.go

@@ -565,7 +565,7 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi
 	// read-only tmpfs here. It simplifies creation of containers without
 	// leaking the root file system.
 	mns, err := c.k.VFS().NewMountNamespace(ctx, creds, "rootfs", "tmpfs",
-		&vfs.MountOptions{ReadOnly: true}, c.k)
+		&vfs.MountOptions{ReadOnly: true, Locked: true}, c.k)
 	if err != nil {
 		return nil, fmt.Errorf("setting up mount namespace: %w", err)
 	}

test/syscalls/linux/mount.cc

@@ -2127,70 +2127,39 @@ TEST(MountTest, MountNamespacePropagation) {
 
 TEST(MountTest, MountNamespaceSlavesNewUserNamespace) {
   SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+  SKIP_IF(ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(GetAbsoluteTestTmpdir())));
+  const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
 
-  int sync_sks[2] = {};
-  ASSERT_THAT(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sync_sks),
-              SyscallSucceeds());
-
-  auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
-  auto const mnt = ASSERT_NO_ERRNO_AND_VALUE(
-      Mount("", dir.path(), kTmpfs, 0, "mode=0700", MNT_DETACH));
-  auto child_dir = JoinPath(dir.path(), "test");
-
+  const Cleanup dir_mount = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", dir.path(), kTmpfs, 0, "", MNT_DETACH));
   ASSERT_THAT(mount(NULL, dir.path().c_str(), NULL, MS_SHARED, NULL),
               SyscallSucceeds());
-  ASSERT_THAT(mkdir(child_dir.c_str(), 0700), SyscallSucceeds());
+  const TempPath child_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
 
-  int uid = geteuid();
-  int gid = getegid();
-  std::string umap_str = absl::StrFormat("0 %lu 1", uid);
-  std::string gmap_str = absl::StrFormat("0 %lu 1", gid);
+  const std::function<void()> parent = [&] {
+    TEST_CHECK_SUCCESS(
+        mount("child", child_dir.path().c_str(), kTmpfs, 0, NULL));
+    TEST_CHECK_SUCCESS(open(JoinPath(child_dir.path(), "foo").c_str(),
+                            O_CREAT | O_RDWR, 0777));
+  };
+  const std::function<void()> child = [&] {
+    TEST_CHECK_SUCCESS(access(JoinPath(child_dir.path(), "foo").c_str(), F_OK));
 
-  pid_t child = fork();
-  if (child == 0) {
-    close(sync_sks[0]);
-    chdir(dir.path().c_str());
-    TEST_CHECK(unshare(CLONE_NEWNS | CLONE_NEWUSER) == 0);
+    // These mount operations will not propagate to the other namespace
+    // because it is a slave mount.
+    TEST_CHECK_SUCCESS(umount2(child_dir.path().c_str(), MNT_DETACH));
+    TEST_CHECK_SUCCESS(
+        mount("test2", child_dir.path().c_str(), kTmpfs, 0, NULL));
+    TEST_CHECK_SUCCESS(
+        mknod(JoinPath(child_dir.path(), "boo").c_str(), 0777 | S_IFREG, 0));
 
-    // Setup uid and gid maps for child.
-    int fd = open("/proc/self/uid_map", O_WRONLY);
-    TEST_CHECK(fd > 0);
-    TEST_CHECK(write(fd, umap_str.c_str(), umap_str.size()) > 0);
-    TEST_CHECK(close(fd) == 0);
-
-    // setgroups isn't implemented in gVisor but is necessary for native tests.
-    fd = open("/proc/self/setgroups", O_WRONLY);
-    if (fd > 0) {
-      TEST_CHECK(write(fd, "deny", 4) > 0);
-      TEST_CHECK(close(fd) == 0);
-    }
-
-    fd = open("/proc/self/gid_map", O_WRONLY);
-    TEST_CHECK(fd > 0);
-    TEST_CHECK(write(fd, gmap_str.c_str(), gmap_str.size()) > 0);
-    TEST_CHECK(close(fd) == 0);
-
-    // Wait until uid and gid maps are setup.
-    TEST_CHECK(setuid(0) == 0);
-    TEST_CHECK(setgid(0) == 0);
-
-    // The child has been initialized. Kick the parent.
-    shutdown(sync_sks[1], SHUT_WR);
-    char s;
-    // Wait when the parent creates a test mount.
-    TEST_CHECK(read(sync_sks[1], &s, 1) == 0);
-    close(sync_sks[1]);
-
-    TEST_CHECK(access("test/foo", F_OK) == 0);
-
-    // These mount operations will not propagate to the other namespace because
-    // it is a slave mount.
-    TEST_CHECK(umount2("test", MNT_DETACH) == 0);
-    TEST_CHECK(mount("test2", "test", kTmpfs, 0, NULL) == 0);
-    TEST_CHECK(mknod(JoinPath("test", "boo").c_str(), 0777 | S_IFREG, 0) == 0);
+    TEST_CHECK_SUCCESS(umount2(child_dir.path().c_str(), MNT_DETACH));
+    // This should fail because the mount is locked.
+    TEST_CHECK_ERRNO(umount2(child_dir.path().c_str(), MNT_DETACH), EINVAL);
 
     // Check that there is a master entry in mountinfo.
-    fd = open("/proc/self/mountinfo", O_RDONLY);
+    int fd = open("/proc/self/mountinfo", O_RDONLY);
     TEST_CHECK(fd >= 0);
     std::string mountinfo;
     char child_mountinfo[0x8000];
@@ -2205,29 +2174,102 @@ TEST(MountTest, MountNamespaceSlavesNewUserNamespace) {
       }
     }
     TEST_CHECK(absl::StrContains(child_mountinfo, "master:"));
-
-    exit(0);
-  }
-  ASSERT_THAT(child, SyscallSucceeds());
-
-  close(sync_sks[1]);
-  char s;
-  // Wait when the child creates a user namespace.
-  ASSERT_THAT(read(sync_sks[0], &s, 1), SyscallSucceeds());
-  ASSERT_THAT(mount("child", child_dir.c_str(), "tmpfs", 0, NULL),
-              SyscallSucceeds());
-  EXPECT_NO_ERRNO(Open(JoinPath(child_dir, "foo"), O_CREAT | O_RDWR, 0777));
-  // The test mount has been created. Kick the child.
-  close(sync_sks[0]);
-
-  int status;
-  ASSERT_THAT(waitpid(child, &status, 0), SyscallSucceedsWithValue(child));
-  ASSERT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0);
+  };
+  EXPECT_THAT(InForkedUserMountNamespace(parent, child),
+              IsPosixErrorOkAndHolds(0));
 
   // Check that the test mount is still here.
-  EXPECT_EQ(Open(JoinPath(child_dir, "boo"), O_RDWR).error().errno_value(),
-            ENOENT);
-  EXPECT_THAT(umount2(child_dir.c_str(), MNT_DETACH), SyscallSucceeds());
+  EXPECT_EQ(
+      Open(JoinPath(child_dir.path(), "boo"), O_RDWR).error().errno_value(),
+      ENOENT);
+  EXPECT_THAT(umount2(child_dir.path().c_str(), MNT_DETACH), SyscallSucceeds());
+}
+
+TEST(MountTest, LockedMountStopsNonRecBind) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+  SKIP_IF(ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(GetAbsoluteTestTmpdir())));
+
+  const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  const Cleanup dir_mount = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", dir.path(), kTmpfs, 0, "", MNT_DETACH));
+  const TempPath child_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
+  const Cleanup child_mount = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", child_dir.path().c_str(), kTmpfs, 0, "", MNT_DETACH));
+
+  const std::function<void()> child_fn = [&] {
+    std::string foo_dir = JoinPath(dir.path(), "foo");
+    TEST_CHECK_SUCCESS(mkdir(foo_dir.c_str(), 0700));
+    TEST_CHECK_ERRNO(
+        mount(dir.path().c_str(), foo_dir.c_str(), "", MS_BIND, ""), EINVAL);
+    TEST_CHECK_SUCCESS(
+        mount(dir.path().c_str(), foo_dir.c_str(), "", MS_BIND | MS_REC, ""));
+  };
+  EXPECT_THAT(InForkedUserMountNamespace([] {}, child_fn),
+              IsPosixErrorOkAndHolds(0));
+}
+
+// This test checks that a mount tree that propagates from a more privileged
+// mount namespace cannot be partially unmounted. It must be unmounted as a
+// single unit as described in point 4 of the notes in
+// https://man7.org/linux/man-pages/man7/mount_namespaces.7.html. This test
+// also checks that unmounting a propagated mount tree does not reveal the
+// contents of overmounted filesystems from the more privileged mount namespace.
+TEST(MountTest, UmountPropagatedSubtreeFromPrivilegedNS) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+  SKIP_IF(ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(GetAbsoluteTestTmpdir())));
+
+  const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  const Cleanup dir_mount = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount(dir.path(), dir.path(), "", MS_BIND, "", MNT_DETACH));
+  ASSERT_THAT(mount(NULL, dir.path().c_str(), NULL, MS_SHARED, NULL),
+              SyscallSucceeds());
+
+  const TempPath child_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
+  const TempPath sibling_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
+  const Cleanup child_mount = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", child_dir.path(), kTmpfs, 0, "", MNT_DETACH));
+  ASSERT_THAT(mount(NULL, child_dir.path().c_str(), NULL, MS_PRIVATE, NULL),
+              SyscallSucceeds());
+
+  const TempPath grandchild_dir =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(child_dir.path()));
+  ASSERT_THAT(open(JoinPath(grandchild_dir.path(), "foo").c_str(),
+                   O_CREAT | O_RDWR, 0777),
+              SyscallSucceeds());
+  const Cleanup grandchild_mnt = ASSERT_NO_ERRNO_AND_VALUE(
+      Mount("", grandchild_dir.path(), kTmpfs, 0, "", MNT_DETACH));
+  ASSERT_THAT(
+      mount(NULL, grandchild_dir.path().c_str(), NULL, MS_PRIVATE, NULL),
+      SyscallSucceeds());
+  const std::string grandsibling_dir =
+      JoinPath(sibling_dir.path(), Basename(grandchild_dir.path()));
+
+  const std::function<void()> parent = [&] {
+    TEST_CHECK_SUCCESS(mount(child_dir.path().c_str(),
+                             sibling_dir.path().c_str(), "", MS_BIND | MS_REC,
+                             ""));
+    TEST_CHECK_SUCCESS(
+        mount("", sibling_dir.path().c_str(), "", MS_PRIVATE | MS_REC, ""));
+  };
+  // You can umount an entire subtree that propagated from a more privileged
+  // mount namespace, but can't umount only part of the subtree.
+  const std::function<void()> child = [&] {
+    TEST_CHECK_ERRNO(umount2(grandsibling_dir.c_str(), MNT_DETACH), EINVAL);
+    int dirfd = open(sibling_dir.path().c_str(), O_RDONLY | O_DIRECTORY);
+    TEST_CHECK(dirfd >= 0);
+    TEST_CHECK_SUCCESS(umount2(sibling_dir.path().c_str(), MNT_DETACH));
+    // Check to ensure you cannot access an overmounted file with openat after
+    // the mount unit has been destroyed.
+    TEST_CHECK_ERRNO(
+        openat(dirfd, JoinPath(Basename(grandchild_dir.path()), "foo").c_str(),
+               O_RDONLY),
+        ENOENT);
+  };
+  EXPECT_THAT(InForkedUserMountNamespace(parent, child),
+              IsPosixErrorOkAndHolds(0));
 }
 
 TEST(MountTest, MountFailsOnPseudoFilesystemMountpoint) {

test/syscalls/linux/pivot_root.cc

@@ -549,6 +549,62 @@ TEST(PivotRootTest, UnreachableNewRootFails) {
   EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
 }
 
+TEST(PivotRootTest, LockedNewRootFails) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_CHROOT)));
+
+  auto root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  ASSERT_THAT(mount("", root.path().c_str(), "tmpfs", 0, "mode=0700"),
+              SyscallSucceeds());
+  auto new_root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(root.path()));
+  ASSERT_THAT(mount("", new_root.path().c_str(), "tmpfs", 0, "mode=0700"),
+              SyscallSucceeds());
+  const std::string new_root_path = JoinPath("/", Basename(new_root.path()));
+  auto put_old =
+      ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(new_root.path()));
+  const std::string put_old_path =
+      JoinPath(new_root_path, "/", Basename(put_old.path()));
+  ASSERT_THAT(mount("", put_old.path().c_str(), "tmpfs", 0, "mode=0700"),
+              SyscallSucceeds());
+
+  const std::function<void()> rest = [&] {
+    TEST_CHECK_SUCCESS(chroot(root.path().c_str()));
+    TEST_CHECK_ERRNO(
+        syscall(__NR_pivot_root, new_root_path.c_str(), put_old_path.c_str()),
+        EINVAL);
+  };
+  EXPECT_THAT(InForkedUserMountNamespace([] {}, rest),
+              IsPosixErrorOkAndHolds(0));
+}
+
+TEST(PivotRootTest, OldRootUnlocked) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_CHROOT)));
+
+  auto root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
+  ASSERT_THAT(mount("", root.path().c_str(), "tmpfs", 0, "mode=0700"),
+              SyscallSucceeds());
+  auto new_root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(root.path()));
+  const std::string new_root_path = JoinPath("/", Basename(new_root.path()));
+
+  // The root mount will be locked when pivot_root is called but the new_root
+  // and put_old mounts won't be.
+  const std::function<void()> rest = [&] {
+    TEST_CHECK_SUCCESS(chroot(root.path().c_str()));
+    TEST_CHECK_SUCCESS(mount("", new_root_path.c_str(), "tmpfs", 0, ""));
+    std::string put_old_path = JoinPath(new_root_path, "put_old");
+    TEST_CHECK_SUCCESS(mkdir(put_old_path.c_str(), 0700));
+    TEST_CHECK_SUCCESS(mount("", put_old_path.c_str(), "tmpfs", 0, ""));
+    TEST_CHECK_SUCCESS(
+        syscall(__NR_pivot_root, new_root_path.c_str(), put_old_path.c_str()));
+    // The old root is no longer locked and can be unmounted.
+    TEST_CHECK_SUCCESS(
+        umount2(JoinPath("/", Basename(put_old_path)).c_str(), MNT_DETACH));
+  };
+  EXPECT_THAT(InForkedUserMountNamespace([] {}, rest),
+              IsPosixErrorOkAndHolds(0));
+}
+
 }  // namespace
 
 }  // namespace testing

test/util/BUILD

@@ -213,9 +213,11 @@ cc_library(
     deps = [
         ":cleanup",
         ":file_descriptor",
+        ":logging",
         ":posix_error",
         ":save_util",
         ":test_util",
+        "@com_google_absl//absl/strings:str_format",
         gtest,
         "@com_google_absl//absl/strings",
     ],

test/util/multiprocess_util.cc

@@ -19,11 +19,18 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <sys/prctl.h>
+#include <sys/socket.h>
 #include <unistd.h>
 
+#include <functional>
+#include <string>
+
+#include "gtest/gtest.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_format.h"
 #include "test/util/cleanup.h"
 #include "test/util/file_descriptor.h"
+#include "test/util/logging.h"
 #include "test/util/posix_error.h"
 #include "test/util/save_util.h"
 #include "test/util/test_util.h"
@@ -172,5 +179,73 @@ PosixErrorOr<int> InForkedProcess(const std::function<void()>& fn) {
   return status;
 }
 
+PosixErrorOr<int> InForkedUserMountNamespace(
+    const std::function<void()>& parent, const std::function<void()>& child) {
+  std::string umap_str = absl::StrFormat("0 %lu 1", geteuid());
+  std::string gmap_str = absl::StrFormat("0 %lu 1", getegid());
+  int sync_sks[2] = {};
+  TEST_CHECK_SUCCESS(socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sync_sks));
+
+  pid_t pid = fork();
+  if (pid == 0) {
+    TEST_CHECK_SUCCESS(close(sync_sks[0]));
+    TEST_CHECK(unshare(CLONE_NEWNS | CLONE_NEWUSER) == 0);
+
+    // Setup uid and gid maps for child.
+    int fd = open("/proc/self/uid_map", O_WRONLY);
+    TEST_CHECK(fd > 0);
+    TEST_CHECK(write(fd, umap_str.c_str(), umap_str.size()) > 0);
+    TEST_CHECK(close(fd) == 0);
+
+    // setgroups isn't implemented in gVisor but is necessary for native
+    // tests.
+    fd = open("/proc/self/setgroups", O_WRONLY);
+    if (fd > 0) {
+      TEST_CHECK(write(fd, "deny", 4) > 0);
+      TEST_CHECK(close(fd) == 0);
+    }
+
+    fd = open("/proc/self/gid_map", O_WRONLY);
+    TEST_CHECK(fd > 0);
+    TEST_CHECK(write(fd, gmap_str.c_str(), gmap_str.size()) > 0);
+    TEST_CHECK(close(fd) == 0);
+
+    // Wait until uid and gid maps are setup.
+    TEST_CHECK(setuid(0) == 0);
+    TEST_CHECK(setgid(0) == 0);
+
+    // Mount/user namespace setup is complete. Now run the parent function.
+    TEST_CHECK_SUCCESS(shutdown(sync_sks[1], SHUT_WR));
+    char s;
+    // Wait for the parent function to be complete.
+    TEST_CHECK(read(sync_sks[1], &s, 1) == 0);
+    TEST_CHECK_SUCCESS(close(sync_sks[1]));
+    // Parent function is complete. Now run the child function.
+    child();
+    TEST_CHECK_MSG(!::testing::Test::HasFailure(),
+                   "EXPECT*/ASSERT* failed. These are not async-signal-safe "
+                   "and must not be called from fn.");
+    _exit(0);
+  }
+  MaybeSave();
+  if (pid < 0) {
+    return PosixError(errno, "fork failed");
+  }
+
+  close(sync_sks[1]);
+  char s;
+  // Wait for mount/user namespace setup to be complete.
+  TEST_CHECK_SUCCESS(read(sync_sks[0], &s, 1));
+  parent();
+  // Now start the child function.
+  TEST_CHECK_SUCCESS(close(sync_sks[0]));
+
+  int status;
+  if (waitpid(pid, &status, 0) < 0) {
+    return PosixError(errno, "waitpid failed");
+  }
+  return status;
+}
+
 }  // namespace testing
 }  // namespace gvisor

test/util/multiprocess_util.h

@@ -18,6 +18,7 @@
 #include <unistd.h>
 
 #include <algorithm>
+#include <functional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -127,6 +128,16 @@ inline PosixErrorOr<Cleanup> ForkAndExecveat(int32_t dirfd,
 // Use TEST_CHECK variants instead.
 PosixErrorOr<int> InForkedProcess(const std::function<void()>& fn);
 
+// Sets up a new user and mount namespace in a forked subprocess using unshare,
+// then runs the parent function in the parent subprocess. Once that returns, it
+// runs the child function in the child process and returns the exit status of
+// the child process.
+//
+// All calls must be async-signal-safe in the child function. Use of
+// ASSERT/EXPECT functions is prohibited. Use TEST_CHECK variants instead.
+PosixErrorOr<int> InForkedUserMountNamespace(
+    const std::function<void()>& parent, const std::function<void()>& child);
+
 }  // namespace testing
 }  // namespace gvisor