From 0a580e0fbfa6b2f94e65d285a848f979f2af4767 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Tue, 24 Jan 2023 14:24:01 -0800
Subject: [PATCH] Fix vfs.disconnectLocked() to get invariant checks passing.

There were two issues with disconnectLocked():
1. One of the invariant checks was wrong. It intended to check if the
   mount was already disconnected. But it was doing the opposite.
2. mountTable.Remove() had an undocumented precondition that the mount
   being removed should have a valid mount.key. This precondition was
   being violated in disconnectLocked().

PiperOrigin-RevId: 504375499
---
 pkg/sentry/vfs/mount.go        | 4 ++--
 pkg/sentry/vfs/mount_unsafe.go | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go
index 76e5d611a..5ac49ca6b 100644
--- a/pkg/sentry/vfs/mount.go
+++ b/pkg/sentry/vfs/mount.go
@@ -659,7 +659,7 @@ func (vfs *VirtualFilesystem) connectLocked(mnt *Mount, vd VirtualDentry, mntns
 func (vfs *VirtualFilesystem) disconnectLocked(mnt *Mount) VirtualDentry {
 	vd := mnt.getKey()
 	if checkInvariants {
-		if vd.mount != nil {
+		if vd.mount == nil {
 			panic("VFS.disconnectLocked called on disconnected mount")
 		}
 		if mnt.ns.mountpoints[vd.dentry] == 0 {
@@ -669,7 +669,6 @@ func (vfs *VirtualFilesystem) disconnectLocked(mnt *Mount) VirtualDentry {
 			panic("VFS.disconnectLocked called on namespace with zero mounts.")
 		}
 	}
-	mnt.loadKey(VirtualDentry{})
 	delete(vd.mount.children, mnt)
 	vd.dentry.mounts.Add(math.MaxUint32) // -1
 	mnt.ns.mountpoints[vd.dentry]--
@@ -678,6 +677,7 @@ func (vfs *VirtualFilesystem) disconnectLocked(mnt *Mount) VirtualDentry {
 		delete(mnt.ns.mountpoints, vd.dentry)
 	}
 	vfs.mounts.removeSeqed(mnt)
+	mnt.loadKey(VirtualDentry{}) // Clear mnt.key.
 	vfsmpmounts := vfs.mountpoints[vd.dentry]
 	delete(vfsmpmounts, mnt)
 	if len(vfsmpmounts) == 0 {
diff --git a/pkg/sentry/vfs/mount_unsafe.go b/pkg/sentry/vfs/mount_unsafe.go
index 9499d6547..6ef24b3d8 100644
--- a/pkg/sentry/vfs/mount_unsafe.go
+++ b/pkg/sentry/vfs/mount_unsafe.go
@@ -329,7 +329,9 @@ func mtInsertLocked(slots unsafe.Pointer, cap uintptr, value unsafe.Pointer, has
 
 // Remove removes the given mount from mt.
 //
-// Preconditions: mt must contain mount.
+// Preconditions:
+//   - mt must contain mount.
+//   - mount.key should be valid.
 func (mt *mountTable) Remove(mount *Mount) {
 	mt.seq.BeginWrite()
 	mt.removeSeqed(mount)
@@ -338,9 +340,8 @@ func (mt *mountTable) Remove(mount *Mount) {
 
 // removeSeqed removes the given mount from mt.
 //
-// Preconditions:
+// Preconditions same as Remove() plus:
 //   - mt.seq must be in a writer critical section.
-//   - mt must contain mount.
 func (mt *mountTable) removeSeqed(mount *Mount) {
 	hash := mount.key.hash()
 	tcap := uintptr(1) << (mt.size.RacyLoad() & mtSizeOrderMask)