diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 78d54f65f..9c97667e4 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/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: diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index bef7ea8d6..7b5c7518c 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/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 */) diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 5a5e9aa29..93fdbd333 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/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