From e0b1585586c65ff82f0d1bbd5bf968ca2d903e59 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Wed, 12 Apr 2023 11:57:46 -0700
Subject: [PATCH] Remove stale `vdDentry` variable from
 VirtualFilesystem.connectMountAt().

This commit reverts the seemingly synthetic changes made to
pkg/sentry/vfs/mount.go in 3e69f5d088d1 ("Add checklocks analyzer.").

The variable `vdDentry` was not updated in the for loop, even though `vd` was.
As a result if `vdDentry.isMounted()` evaluated to true (rare), then we keep
looping until `vd` is `DecRef()`d so much that it is destroyed.

Get rid of `vdDentry`, which adds another state to keep up-to-date and also
hurts the readability.

It is likely that this was triggering the linked Syzkaller bugs.

Reported-by: syzbot+affad616ecc20c7743c4@syzkaller.appspotmail.com
Reported-by: syzbot+ded449caf808803f3f10@syzkaller.appspotmail.com
PiperOrigin-RevId: 523765810
---
 pkg/sentry/vfs/mount.go | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go
index 767a4e71c..469431a21 100644
--- a/pkg/sentry/vfs/mount.go
+++ b/pkg/sentry/vfs/mount.go
@@ -306,19 +306,18 @@ func (vfs *VirtualFilesystem) ConnectMountAt(ctx context.Context, creds *auth.Cr
 //
 // +checklocks:vfs.mountMu
 func (vfs *VirtualFilesystem) connectMountAt(ctx context.Context, mnt *Mount, vd VirtualDentry) error {
-	vdDentry := vd.dentry
-	vdDentry.mu.Lock()
+	vd.dentry.mu.Lock()
 	for {
-		if vd.mount.umounted || vdDentry.dead {
-			vdDentry.mu.Unlock()
+		if vd.mount.umounted || vd.dentry.dead {
+			vd.dentry.mu.Unlock()
 			return linuxerr.ENOENT
 		}
 		// vd might have been mounted over between vfs.GetDentryAt() and
 		// vfs.mountMu.Lock().
-		if !vdDentry.isMounted() {
+		if !vd.dentry.isMounted() {
 			break
 		}
-		nextmnt := vfs.mounts.Lookup(vd.mount, vdDentry)
+		nextmnt := vfs.mounts.Lookup(vd.mount, vd.dentry)
 		if nextmnt == nil {
 			break
 		}
@@ -331,13 +330,13 @@ func (vfs *VirtualFilesystem) connectMountAt(ctx context.Context, mnt *Mount, vd
 		}
 		// This can't fail since we're holding vfs.mountMu.
 		nextmnt.root.IncRef()
-		vdDentry.mu.Unlock()
+		vd.dentry.mu.Unlock()
 		vd.DecRef(ctx)
 		vd = VirtualDentry{
 			mount:  nextmnt,
 			dentry: nextmnt.root,
 		}
-		vdDentry.mu.Lock()
+		vd.dentry.mu.Lock()
 	}
 	// TODO(gvisor.dev/issue/1035): Linux requires that either both the mount
 	// point and the mount root are directories, or neither are, and returns
@@ -346,7 +345,7 @@ func (vfs *VirtualFilesystem) connectMountAt(ctx context.Context, mnt *Mount, vd
 	vfs.mounts.seq.BeginWrite()
 	vfs.connectLocked(mnt, vd, mntns)
 	vfs.mounts.seq.EndWrite()
-	vdDentry.mu.Unlock()
+	vd.dentry.mu.Unlock()
 	return nil
 }