summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/sentry/fsimpl/gofer/directory.go4
-rw-r--r--pkg/sentry/fsimpl/gofer/filesystem.go11
-rw-r--r--pkg/sentry/fsimpl/gofer/gofer.go202
-rw-r--r--pkg/sentry/fsimpl/gofer/regular_file.go20
-rw-r--r--pkg/sentry/fsimpl/gofer/save_restore.go4
5 files changed, 155 insertions, 86 deletions
diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go
index ce1b2a390..3b5927702 100644
--- a/pkg/sentry/fsimpl/gofer/directory.go
+++ b/pkg/sentry/fsimpl/gofer/directory.go
@@ -98,7 +98,9 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) {
uid: uint32(opts.kuid),
gid: uint32(opts.kgid),
blockSize: usermem.PageSize, // arbitrary
- hostFD: -1,
+ readFD: -1,
+ writeFD: -1,
+ mmapFD: -1,
nlink: uint32(2),
}
refsvfs2.Register(child)
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index bbb01148b..7ab298019 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -1167,18 +1167,21 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
// Incorporate the fid that was opened by lcreate.
useRegularFileFD := child.fileType() == linux.S_IFREG && !d.fs.opts.regularFilesUseSpecialFileFD
if useRegularFileFD {
+ openFD := int32(-1)
+ if fdobj != nil {
+ openFD = int32(fdobj.Release())
+ }
child.handleMu.Lock()
if vfs.MayReadFileWithOpenFlags(opts.Flags) {
child.readFile = openFile
if fdobj != nil {
- child.hostFD = int32(fdobj.Release())
+ child.readFD = openFD
+ child.mmapFD = openFD
}
- } else if fdobj != nil {
- // Can't use fdobj if it's not readable.
- fdobj.Close()
}
if vfs.MayWriteFileWithOpenFlags(opts.Flags) {
child.writeFile = openFile
+ child.writeFD = openFD
}
child.handleMu.Unlock()
}
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 6f82ce61b..b59f62652 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -548,11 +548,16 @@ func (fs *filesystem) Release(ctx context.Context) {
d.cache.DropAll(mf)
d.dirty.RemoveAll()
d.dataMu.Unlock()
- // Close the host fd if one exists.
- if d.hostFD >= 0 {
- syscall.Close(int(d.hostFD))
- d.hostFD = -1
+ // Close host FDs if they exist.
+ if d.readFD >= 0 {
+ syscall.Close(int(d.readFD))
}
+ if d.writeFD >= 0 && d.readFD != d.writeFD {
+ syscall.Close(int(d.writeFD))
+ }
+ d.readFD = -1
+ d.writeFD = -1
+ d.mmapFD = -1
d.handleMu.Unlock()
}
// There can't be any specialFileFDs still using fs, since each such
@@ -726,15 +731,17 @@ type dentry struct {
// - If this dentry represents a regular file or directory, readFile is the
// p9.File used for reads by all regularFileFDs/directoryFDs representing
- // this dentry.
+ // this dentry, and readFD (if not -1) is a host FD equivalent to readFile
+ // used as a faster alternative.
//
// - If this dentry represents a regular file, writeFile is the p9.File
- // used for writes by all regularFileFDs representing this dentry.
+ // used for writes by all regularFileFDs representing this dentry, and
+ // writeFD (if not -1) is a host FD equivalent to writeFile used as a
+ // faster alternative.
//
- // - If this dentry represents a regular file, hostFD is the host FD used
- // for memory mappings and I/O (when applicable) in preference to readFile
- // and writeFile. hostFD is always readable; if !writeFile.isNil(), it must
- // also be writable. If hostFD is -1, no such host FD is available.
+ // - If this dentry represents a regular file, mmapFD is the host FD used
+ // for memory mappings. If mmapFD is -1, no such FD is available, and the
+ // internal page cache implementation is used for memory mappings instead.
//
// These fields are protected by handleMu.
//
@@ -742,10 +749,17 @@ type dentry struct {
// either p9.File transitions from closed (isNil() == true) to open
// (isNil() == false), it may be mutated with handleMu locked, but cannot
// be closed until the dentry is destroyed.
+ //
+ // readFD and writeFD may or may not be the same file descriptor. mmapFD is
+ // always either -1 or equal to readFD; if !writeFile.isNil() (the file has
+ // been opened for writing), it is additionally either -1 or equal to
+ // writeFD.
handleMu sync.RWMutex `state:"nosave"`
readFile p9file `state:"nosave"`
writeFile p9file `state:"nosave"`
- hostFD int32 `state:"nosave"`
+ readFD int32 `state:"nosave"`
+ writeFD int32 `state:"nosave"`
+ mmapFD int32 `state:"nosave"`
dataMu sync.RWMutex `state:"nosave"`
@@ -829,7 +843,9 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma
uid: uint32(fs.opts.dfltuid),
gid: uint32(fs.opts.dfltgid),
blockSize: usermem.PageSize,
- hostFD: -1,
+ readFD: -1,
+ writeFD: -1,
+ mmapFD: -1,
}
d.pf.dentry = d
if mask.UID {
@@ -1469,10 +1485,15 @@ func (d *dentry) destroyLocked(ctx context.Context) {
}
d.readFile = p9file{}
d.writeFile = p9file{}
- if d.hostFD >= 0 {
- syscall.Close(int(d.hostFD))
- d.hostFD = -1
+ if d.readFD >= 0 {
+ syscall.Close(int(d.readFD))
}
+ if d.writeFD >= 0 && d.readFD != d.writeFD {
+ syscall.Close(int(d.writeFD))
+ }
+ d.readFD = -1
+ d.writeFD = -1
+ d.mmapFD = -1
d.handleMu.Unlock()
if !d.file.isNil() {
@@ -1584,7 +1605,8 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool
d.handleMu.RUnlock()
}
- fdToClose := int32(-1)
+ var fdsToCloseArr [2]int32
+ fdsToClose := fdsToCloseArr[:0]
invalidateTranslations := false
d.handleMu.Lock()
if (read && d.readFile.isNil()) || (write && d.writeFile.isNil()) || trunc {
@@ -1615,56 +1637,88 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool
return err
}
- if d.hostFD < 0 && h.fd >= 0 && openReadable && (d.writeFile.isNil() || openWritable) {
- // We have no existing FD, and the new FD meets the requirements
- // for d.hostFD, so start using it.
- d.hostFD = h.fd
- } else if d.hostFD >= 0 && d.writeFile.isNil() && openWritable {
- // We have an existing read-only FD, but the file has just been
- // opened for writing, so we need to start supporting writable memory
- // mappings. This may race with callers of d.pf.FD() using the existing
- // FD, so in most cases we need to delay closing the old FD until after
- // invalidating memmap.Translations that might have observed it.
- if !openReadable || h.fd < 0 {
- // We don't have a read/write FD, so we have no FD that can be
- // used to create writable memory mappings. Switch to using the
- // internal page cache.
- invalidateTranslations = true
- fdToClose = d.hostFD
- d.hostFD = -1
- } else if d.fs.opts.overlayfsStaleRead {
- // We do have a read/write FD, but it may not be coherent with
- // the existing read-only FD, so we must switch to mappings of
- // the new FD in both the application and sentry.
- if err := d.pf.hostFileMapper.RegenerateMappings(int(h.fd)); err != nil {
- d.handleMu.Unlock()
- ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to replace sentry mappings of old FD with mappings of new FD: %v", err)
- h.close(ctx)
- return err
+ // Update d.readFD and d.writeFD.
+ if h.fd >= 0 {
+ if openReadable && openWritable && (d.readFD < 0 || d.writeFD < 0 || d.readFD != d.writeFD) {
+ // Replace existing FDs with this one.
+ if d.readFD >= 0 {
+ // We already have a readable FD that may be in use by
+ // concurrent callers of d.pf.FD().
+ if d.fs.opts.overlayfsStaleRead {
+ // If overlayfsStaleRead is in effect, then the new FD
+ // may not be coherent with the existing one, so we
+ // have no choice but to switch to mappings of the new
+ // FD in both the application and sentry.
+ if err := d.pf.hostFileMapper.RegenerateMappings(int(h.fd)); err != nil {
+ d.handleMu.Unlock()
+ ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to replace sentry mappings of old FD with mappings of new FD: %v", err)
+ h.close(ctx)
+ return err
+ }
+ fdsToClose = append(fdsToClose, d.readFD)
+ invalidateTranslations = true
+ d.readFD = h.fd
+ } else {
+ // Otherwise, we want to avoid invalidating existing
+ // memmap.Translations (which is expensive); instead, use
+ // dup3 to make the old file descriptor refer to the new
+ // file description, then close the new file descriptor
+ // (which is no longer needed). Racing callers of d.pf.FD()
+ // may use the old or new file description, but this
+ // doesn't matter since they refer to the same file, and
+ // any racing mappings must be read-only.
+ if err := syscall.Dup3(int(h.fd), int(d.readFD), syscall.O_CLOEXEC); err != nil {
+ oldFD := d.readFD
+ d.handleMu.Unlock()
+ ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to dup fd %d to fd %d: %v", h.fd, oldFD, err)
+ h.close(ctx)
+ return err
+ }
+ fdsToClose = append(fdsToClose, h.fd)
+ h.fd = d.readFD
+ }
+ } else {
+ d.readFD = h.fd
}
- invalidateTranslations = true
- fdToClose = d.hostFD
- d.hostFD = h.fd
- } else {
- // We do have a read/write FD. To avoid invalidating existing
- // memmap.Translations (which is expensive), use dup3 to make
- // the old file descriptor refer to the new file description,
- // then close the new file descriptor (which is no longer
- // needed). Racing callers of d.pf.FD() may use the old or new
- // file description, but this doesn't matter since they refer
- // to the same file, and any racing mappings must be read-only.
- if err := syscall.Dup3(int(h.fd), int(d.hostFD), syscall.O_CLOEXEC); err != nil {
- oldHostFD := d.hostFD
- d.handleMu.Unlock()
- ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to dup fd %d to fd %d: %v", h.fd, oldHostFD, err)
- h.close(ctx)
- return err
+ if d.writeFD != h.fd && d.writeFD >= 0 {
+ fdsToClose = append(fdsToClose, d.writeFD)
+ }
+ d.writeFD = h.fd
+ d.mmapFD = h.fd
+ } else if openReadable && d.readFD < 0 {
+ d.readFD = h.fd
+ // If the file has not been opened for writing, the new FD may
+ // be used for read-only memory mappings. If the file was
+ // previously opened for reading (without an FD), then existing
+ // translations of the file may use the internal page cache;
+ // invalidate those mappings.
+ if d.writeFile.isNil() {
+ invalidateTranslations = !d.readFile.isNil()
+ d.mmapFD = h.fd
+ }
+ } else if openWritable && d.writeFD < 0 {
+ d.writeFD = h.fd
+ if d.readFD >= 0 {
+ // We have an existing read-only FD, but the file has just
+ // been opened for writing, so we need to start supporting
+ // writable memory mappings. However, the new FD is not
+ // readable, so we have no FD that can be used to create
+ // writable memory mappings. Switch to using the internal
+ // page cache.
+ invalidateTranslations = true
+ d.mmapFD = -1
}
- fdToClose = h.fd
+ } else {
+ // The new FD is not useful.
+ fdsToClose = append(fdsToClose, h.fd)
}
- } else {
- // h.fd is not useful.
- fdToClose = h.fd
+ } else if openWritable && d.writeFD < 0 && d.mmapFD >= 0 {
+ // We have an existing read-only FD, but the file has just been
+ // opened for writing, so we need to start supporting writable
+ // memory mappings. However, we have no writable host FD. Switch to
+ // using the internal page cache.
+ invalidateTranslations = true
+ d.mmapFD = -1
}
// Switch to new fids.
@@ -1698,8 +1752,8 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool
d.mappings.InvalidateAll(memmap.InvalidateOpts{})
d.mapsMu.Unlock()
}
- if fdToClose >= 0 {
- syscall.Close(int(fdToClose))
+ for _, fd := range fdsToClose {
+ syscall.Close(int(fd))
}
return nil
@@ -1709,7 +1763,7 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool
func (d *dentry) readHandleLocked() handle {
return handle{
file: d.readFile,
- fd: d.hostFD,
+ fd: d.readFD,
}
}
@@ -1717,7 +1771,7 @@ func (d *dentry) readHandleLocked() handle {
func (d *dentry) writeHandleLocked() handle {
return handle{
file: d.writeFile,
- fd: d.hostFD,
+ fd: d.writeFD,
}
}
@@ -1730,16 +1784,24 @@ func (d *dentry) syncRemoteFile(ctx context.Context) error {
// Preconditions: d.handleMu must be locked.
func (d *dentry) syncRemoteFileLocked(ctx context.Context) error {
// If we have a host FD, fsyncing it is likely to be faster than an fsync
- // RPC.
- if d.hostFD >= 0 {
+ // RPC. Prefer syncing write handles over read handles, since some remote
+ // filesystem implementations may not sync changes made through write
+ // handles otherwise.
+ if d.writeFD >= 0 {
ctx.UninterruptibleSleepStart(false)
- err := syscall.Fsync(int(d.hostFD))
+ err := syscall.Fsync(int(d.writeFD))
ctx.UninterruptibleSleepFinish(false)
return err
}
if !d.writeFile.isNil() {
return d.writeFile.fsync(ctx)
}
+ if d.readFD >= 0 {
+ ctx.UninterruptibleSleepStart(false)
+ err := syscall.Fsync(int(d.readFD))
+ ctx.UninterruptibleSleepFinish(false)
+ return err
+ }
if !d.readFile.isNil() {
return d.readFile.fsync(ctx)
}
diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go
index dc8a890cb..652142ecc 100644
--- a/pkg/sentry/fsimpl/gofer/regular_file.go
+++ b/pkg/sentry/fsimpl/gofer/regular_file.go
@@ -326,7 +326,7 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error)
// dentry.readHandleLocked() without locking dentry.dataMu.
rw.d.handleMu.RLock()
h := rw.d.readHandleLocked()
- if (rw.d.hostFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
+ if (rw.d.mmapFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
n, err := h.readToBlocksAt(rw.ctx, dsts, rw.off)
rw.d.handleMu.RUnlock()
rw.off += n
@@ -446,7 +446,7 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro
// without locking dentry.dataMu.
rw.d.handleMu.RLock()
h := rw.d.writeHandleLocked()
- if (rw.d.hostFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
+ if (rw.d.mmapFD >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
n, err := h.writeFromBlocksAt(rw.ctx, srcs, rw.off)
rw.off += n
rw.d.dataMu.Lock()
@@ -648,7 +648,7 @@ func (fd *regularFileFD) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpt
return syserror.ENODEV
}
d.handleMu.RLock()
- haveFD := d.hostFD >= 0
+ haveFD := d.mmapFD >= 0
d.handleMu.RUnlock()
if !haveFD {
return syserror.ENODEV
@@ -669,7 +669,7 @@ func (d *dentry) mayCachePages() bool {
return true
}
d.handleMu.RLock()
- haveFD := d.hostFD >= 0
+ haveFD := d.mmapFD >= 0
d.handleMu.RUnlock()
return haveFD
}
@@ -727,7 +727,7 @@ func (d *dentry) CopyMapping(ctx context.Context, ms memmap.MappingSpace, srcAR,
// Translate implements memmap.Mappable.Translate.
func (d *dentry) Translate(ctx context.Context, required, optional memmap.MappableRange, at usermem.AccessType) ([]memmap.Translation, error) {
d.handleMu.RLock()
- if d.hostFD >= 0 && !d.fs.opts.forcePageCache {
+ if d.mmapFD >= 0 && !d.fs.opts.forcePageCache {
d.handleMu.RUnlock()
mr := optional
if d.fs.opts.limitHostFDTranslation {
@@ -881,7 +881,7 @@ func (d *dentry) Evict(ctx context.Context, er pgalloc.EvictableRange) {
// cannot implement both vfs.DentryImpl.IncRef and memmap.File.IncRef.
//
// dentryPlatformFile is only used when a host FD representing the remote file
-// is available (i.e. dentry.hostFD >= 0), and that FD is used for application
+// is available (i.e. dentry.mmapFD >= 0), and that FD is used for application
// memory mappings (i.e. !filesystem.opts.forcePageCache).
//
// +stateify savable
@@ -892,8 +892,8 @@ type dentryPlatformFile struct {
// by dentry.dataMu.
fdRefs fsutil.FrameRefSet
- // If this dentry represents a regular file, and dentry.hostFD >= 0,
- // hostFileMapper caches mappings of dentry.hostFD.
+ // If this dentry represents a regular file, and dentry.mmapFD >= 0,
+ // hostFileMapper caches mappings of dentry.mmapFD.
hostFileMapper fsutil.HostFileMapper
// hostFileMapperInitOnce is used to lazily initialize hostFileMapper.
@@ -918,12 +918,12 @@ func (d *dentryPlatformFile) DecRef(fr memmap.FileRange) {
func (d *dentryPlatformFile) MapInternal(fr memmap.FileRange, at usermem.AccessType) (safemem.BlockSeq, error) {
d.handleMu.RLock()
defer d.handleMu.RUnlock()
- return d.hostFileMapper.MapInternal(fr, int(d.hostFD), at.Write)
+ return d.hostFileMapper.MapInternal(fr, int(d.mmapFD), at.Write)
}
// FD implements memmap.File.FD.
func (d *dentryPlatformFile) FD() int {
d.handleMu.RLock()
defer d.handleMu.RUnlock()
- return int(d.hostFD)
+ return int(d.mmapFD)
}
diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go
index 17849dcc0..c90071e4e 100644
--- a/pkg/sentry/fsimpl/gofer/save_restore.go
+++ b/pkg/sentry/fsimpl/gofer/save_restore.go
@@ -139,7 +139,9 @@ func (d *dentry) beforeSave() {
// afterLoad is invoked by stateify.
func (d *dentry) afterLoad() {
- d.hostFD = -1
+ d.readFD = -1
+ d.writeFD = -1
+ d.mmapFD = -1
if atomic.LoadInt64(&d.refs) != -1 {
refsvfs2.Register(d)
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-05 10:38:40 +0000