Close directfs dentry's control FD on gofer filesystem Release().

Otherwise we will leak these control FDs and the sandbox will continue holding
on to it. Note that the dentry cache is not destroyed on Release(). This can
prevent the container filesystems from being unmounted in multi-container
scenario due to EBUSY errors.

Updates #9834

Investigated-by: Andrei Vagin <avagin@google.com>
PiperOrigin-RevId: 596770126

pkg/sentry/fsimpl/gofer/dentry_impl.go

@@ -17,6 +17,7 @@ package gofer
 import (
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/fsutil"
@@ -147,6 +148,30 @@ func (d *dentry) updateHandles(ctx context.Context, h handle, readable, writable
 	}
 }
 
+// Preconditions:
+//   - d.handleMu must be locked.
+//   - !d.isSynthetic().
+func (d *dentry) closeHostFDs() {
+	// We can use RacyLoad() because d.handleMu is locked.
+	if d.readFD.RacyLoad() >= 0 {
+		_ = unix.Close(int(d.readFD.RacyLoad()))
+	}
+	if d.writeFD.RacyLoad() >= 0 && d.readFD.RacyLoad() != d.writeFD.RacyLoad() {
+		_ = unix.Close(int(d.writeFD.RacyLoad()))
+	}
+	d.readFD = atomicbitops.FromInt32(-1)
+	d.writeFD = atomicbitops.FromInt32(-1)
+	d.mmapFD = atomicbitops.FromInt32(-1)
+
+	switch dt := d.impl.(type) {
+	case *directfsDentry:
+		if dt.controlFD >= 0 {
+			_ = unix.Close(dt.controlFD)
+			dt.controlFD = -1
+		}
+	}
+}
+
 // updateMetadataLocked updates the dentry's metadata fields. The h parameter
 // is optional. If it is not provided, an appropriate FD should be chosen to
 // stat the remote file.
@@ -213,6 +238,7 @@ func (d *dentry) setStatLocked(ctx context.Context, stat *linux.Statx) (uint32,
 	}
 }
 
+// Precondition: d.handleMu must be locked.
 func (d *dentry) destroyImpl(ctx context.Context) {
 	switch dt := d.impl.(type) {
 	case *lisafsDentry:

pkg/sentry/fsimpl/gofer/directfs_dentry.go

@@ -91,7 +91,8 @@ func (fs *filesystem) getDirectfsRootDentry(ctx context.Context, rootHostFD int,
 type directfsDentry struct {
 	dentry
 
-	// controlFD is the host FD to this file. controlFD is immutable.
+	// controlFD is the host FD to this file. controlFD is immutable until
+	// destruction, which is synchronized with dentry.handleMu.
 	controlFD int
 
 	// controlFDLisa is a lisafs control FD on this dentry.
@@ -401,9 +402,11 @@ func fchown(fd, uid, gid int) error {
 	return unix.Fchownat(fd, "", uid, gid, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW)
 }
 
+// Precondition: d.handleMu must be locked.
 func (d *directfsDentry) destroy(ctx context.Context) {
 	if d.controlFD >= 0 {
 		_ = unix.Close(d.controlFD)
+		d.controlFD = -1
 	}
 	if d.controlFDLisa.Ok() {
 		d.controlFDLisa.Close(ctx, true /* flush */)

pkg/sentry/fsimpl/gofer/gofer.go

@@ -678,17 +678,8 @@ func (fs *filesystem) Release(ctx context.Context) {
 		d.cache.DropAll(mf)
 		d.dirty.RemoveAll()
 		d.dataMu.Unlock()
-		// Close host FDs if they exist. We can use RacyLoad() because d.handleMu
-		// is locked.
-		if d.readFD.RacyLoad() >= 0 {
-			_ = unix.Close(int(d.readFD.RacyLoad()))
-		}
-		if d.writeFD.RacyLoad() >= 0 && d.readFD.RacyLoad() != d.writeFD.RacyLoad() {
-			_ = unix.Close(int(d.writeFD.RacyLoad()))
-		}
-		d.readFD = atomicbitops.FromInt32(-1)
-		d.writeFD = atomicbitops.FromInt32(-1)
-		d.mmapFD = atomicbitops.FromInt32(-1)
+		// Close host FDs if they exist.
+		d.closeHostFDs()
 		d.handleMu.Unlock()
 	}
 	// There can't be any specialFileFDs still using fs, since each such