diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD index 36111538c..d2134db5a 100644 --- a/pkg/sentry/fsimpl/gofer/BUILD +++ b/pkg/sentry/fsimpl/gofer/BUILD @@ -10,8 +10,20 @@ go_template_instance( prefix = "dentry", template = "//pkg/ilist:generic_list", types = { - "Element": "*dentry", - "Linker": "*dentry", + "Element": "*dentryListElem", + "Linker": "*dentryListElem", + }, +) + +go_template_instance( + name = "special_fd_list", + out = "special_fd_list.go", + package = "gofer", + prefix = "specialFD", + template = "//pkg/ilist:generic_list", + types = { + "Element": "*specialFileFD", + "Linker": "*specialFileFD", }, ) @@ -41,6 +53,7 @@ go_library( "revalidate.go", "save_restore.go", "socket.go", + "special_fd_list.go", "special_file.go", "symlink.go", "time.go", diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index 6156f4990..eba8f641e 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -140,6 +140,8 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { panic(fmt.Sprintf("failed to create synthetic file of unrecognized type: %v", opts.mode.FileType())) } child.pf.dentry = child + child.cacheEntry.d = child + child.syncableListEntry.d = child child.vfsd.Init(child) d.cacheNewChildLocked(child, opts.name) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 9ef905d2e..aecd652bb 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -41,12 +41,12 @@ import ( func (fs *filesystem) Sync(ctx context.Context) error { // Snapshot current syncable dentries and special file FDs. fs.syncMu.Lock() - ds := make([]*dentry, 0, len(fs.syncableDentries)) - for d := range fs.syncableDentries { - ds = append(ds, d) + ds := make([]*dentry, 0, fs.syncableDentries.Len()) + for elem := fs.syncableDentries.Front(); elem != nil; elem = elem.Next() { + ds = append(ds, elem.d) } - sffds := make([]*specialFileFD, 0, len(fs.specialFileFDs)) - for sffd := range fs.specialFileFDs { + sffds := make([]*specialFileFD, 0, fs.specialFileFDs.Len()) + for sffd := fs.specialFileFDs.Front(); sffd != nil; sffd = sffd.Next() { sffds = append(sffds, sffd) } fs.syncMu.Unlock() diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index ee8430f2d..22da7c121 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -181,8 +181,8 @@ type filesystem struct { // syncableDentries contains all non-synthetic dentries. specialFileFDs // contains all open specialFileFDs. These fields are protected by syncMu. syncMu sync.Mutex `state:"nosave"` - syncableDentries map[*dentry]struct{} - specialFileFDs map[*specialFileFD]struct{} + syncableDentries dentryList + specialFileFDs specialFDList // inoByQIDPath maps previously-observed QID.Paths to inode numbers // assigned to those paths. inoByQIDPath is not preserved across @@ -487,15 +487,13 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, err } fs := &filesystem{ - mfp: mfp, - opts: fsopts, - iopts: iopts, - clock: ktime.RealtimeClockFromContext(ctx), - devMinor: devMinor, - syncableDentries: make(map[*dentry]struct{}), - specialFileFDs: make(map[*specialFileFD]struct{}), - inoByQIDPath: make(map[uint64]uint64), - inoByKey: make(map[inoKey]uint64), + mfp: mfp, + opts: fsopts, + iopts: iopts, + clock: ktime.RealtimeClockFromContext(ctx), + devMinor: devMinor, + inoByQIDPath: make(map[uint64]uint64), + inoByKey: make(map[inoKey]uint64), } // Did the user configure a global dentry cache? @@ -681,7 +679,8 @@ func (fs *filesystem) Release(ctx context.Context) { mf := fs.mfp.MemoryFile() fs.syncMu.Lock() - for d := range fs.syncableDentries { + for elem := fs.syncableDentries.Front(); elem != nil; elem = elem.Next() { + d := elem.d d.handleMu.Lock() d.dataMu.Lock() if h := d.writeHandleLocked(); h.isOpen() { @@ -839,11 +838,17 @@ type dentry struct { // this dentry. cachingMu sync.Mutex `state:"nosave"` - // If cached is true, dentryEntry links dentry into - // filesystem.dentryCache.dentries. cached and dentryEntry are protected by - // cachingMu. + // If cached is true, this dentry is part of filesystem.dentryCache. cached + // is protected by cachingMu. cached bool - dentryEntry + + // cacheEntry links dentry into filesystem.dentryCache.dentries. It is + // protected by filesystem.dentryCache.mu. + cacheEntry dentryListElem + + // syncableListEntry links dentry into filesystem.syncableDentries. It is + // protected by filesystem.syncMu. + syncableListEntry dentryListElem dirMu sync.Mutex `state:"nosave"` @@ -995,6 +1000,13 @@ type dentry struct { watches vfs.Watches } +// +stateify savable +type dentryListElem struct { + // d is the dentry that this elem represents. + d *dentry + dentryEntry +} + // dentryAttrMask returns a p9.AttrMask enabling all attributes used by the // gofer client. func dentryAttrMask() p9.AttrMask { @@ -1040,6 +1052,8 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma mmapFD: atomicbitops.FromInt32(-1), } d.pf.dentry = d + d.cacheEntry.d = d + d.syncableListEntry.d = d if mask.UID { d.uid = atomicbitops.FromUint32(dentryUIDFromP9UID(attr.UID)) } @@ -1083,7 +1097,7 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma d.vfsd.Init(d) refsvfs2.Register(d) fs.syncMu.Lock() - fs.syncableDentries[d] = struct{}{} + fs.syncableDentries.PushBack(&d.syncableListEntry) fs.syncMu.Unlock() return d, nil } @@ -1112,8 +1126,9 @@ func (fs *filesystem) newDentryLisa(ctx context.Context, ino *lisafs.Inode) (*de mmapFD: atomicbitops.FromInt32(-1), controlFDLisa: fs.clientLisa.NewFD(ino.ControlFD), } - d.pf.dentry = d + d.cacheEntry.d = d + d.syncableListEntry.d = d if ino.Stat.Mask&linux.STATX_UID != 0 { d.uid = atomicbitops.FromUint32(dentryUIDFromLisaUID(lisafs.UID(ino.Stat.UID))) } @@ -1157,7 +1172,7 @@ func (fs *filesystem) newDentryLisa(ctx context.Context, ino *lisafs.Inode) (*de d.vfsd.Init(d) refsvfs2.Register(d) fs.syncMu.Lock() - fs.syncableDentries[d] = struct{}{} + fs.syncableDentries.PushBack(&d.syncableListEntry) fs.syncMu.Unlock() return d, nil } @@ -1928,15 +1943,15 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo d.fs.dentryCache.mu.Lock() // If d is already cached, just move it to the front of the LRU. if d.cached { - d.fs.dentryCache.dentries.Remove(d) - d.fs.dentryCache.dentries.PushFront(d) + d.fs.dentryCache.dentries.Remove(&d.cacheEntry) + d.fs.dentryCache.dentries.PushFront(&d.cacheEntry) d.fs.dentryCache.mu.Unlock() d.cachingMu.Unlock() return } // Cache the dentry, then evict the least recently used cached dentry if // the cache becomes over-full. - d.fs.dentryCache.dentries.PushFront(d) + d.fs.dentryCache.dentries.PushFront(&d.cacheEntry) d.fs.dentryCache.dentriesLen++ d.cached = true shouldEvict := d.fs.dentryCache.dentriesLen > d.fs.dentryCache.maxCachedDentries @@ -1958,7 +1973,7 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo func (d *dentry) removeFromCacheLocked() { if d.cached { d.fs.dentryCache.mu.Lock() - d.fs.dentryCache.dentries.Remove(d) + d.fs.dentryCache.dentries.Remove(&d.cacheEntry) d.fs.dentryCache.dentriesLen-- d.fs.dentryCache.mu.Unlock() d.cached = false @@ -1988,8 +2003,8 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { return } - if victim.fs == fs { - victim.evictLocked(ctx) // +checklocksforce: owned as precondition, victim.fs == fs + if victim.d.fs == fs { + victim.d.evictLocked(ctx) // +checklocksforce: owned as precondition, victim.fs == fs return } @@ -1999,7 +2014,7 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { // each others' renameMu. fs.renameMu.Unlock() defer fs.renameMu.Lock() - victim.evict(ctx) + victim.d.evict(ctx) } // Preconditions: @@ -2140,7 +2155,7 @@ func (d *dentry) destroyLocked(ctx context.Context) { // Remove d from the set of syncable dentries. d.fs.syncMu.Lock() - delete(d.fs.syncableDentries, d) + d.fs.syncableDentries.Remove(&d.syncableListEntry) d.fs.syncMu.Unlock() } diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index addd0c451..58a06e9fa 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/pkg/sentry/fsimpl/gofer/gofer_test.go @@ -26,11 +26,10 @@ import ( func TestDestroyIdempotent(t *testing.T) { ctx := contexttest.Context(t) fs := filesystem{ - mfp: pgalloc.MemoryFileProviderFromContext(ctx), - syncableDentries: make(map[*dentry]struct{}), - inoByQIDPath: make(map[uint64]uint64), - inoByKey: make(map[inoKey]uint64), - clock: time.RealtimeClockFromContext(ctx), + mfp: pgalloc.MemoryFileProviderFromContext(ctx), + inoByQIDPath: make(map[uint64]uint64), + inoByKey: make(map[inoKey]uint64), + clock: time.RealtimeClockFromContext(ctx), // Test relies on no dentry being held in the cache. dentryCache: &dentryCache{maxCachedDentries: 0}, } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 0b152b130..5b42fddef 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -61,7 +61,7 @@ func (fs *filesystem) PrepareSave(ctx context.Context) error { // Buffer pipe data so that it's available for reading after restore. (This // is a legacy VFS1 feature.) fs.syncMu.Lock() - for sffd := range fs.specialFileFDs { + for sffd := fs.specialFileFDs.Front(); sffd != nil; sffd = sffd.Next() { if sffd.dentry().fileType() == linux.S_IFIFO && sffd.vfsfd.IsReadable() { if err := sffd.savePipeData(ctx); err != nil { fs.syncMu.Unlock() @@ -231,7 +231,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest // ENXIO if another specialFileFD represents the read end of the same pipe. // This is consistent with VFS1. haveWriteOnlyPipes := false - for fd := range fs.specialFileFDs { + for fd := fs.specialFileFDs.Front(); fd != nil; fd = fd.Next() { if fd.dentry().fileType() == linux.S_IFIFO && !fd.vfsfd.IsReadable() { haveWriteOnlyPipes = true continue @@ -241,7 +241,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest } } if haveWriteOnlyPipes { - for fd := range fs.specialFileFDs { + for fd := fs.specialFileFDs.Front(); fd != nil; fd = fd.Next() { if fd.dentry().fileType() == linux.S_IFIFO && !fd.vfsfd.IsReadable() { if err := fd.completeRestore(ctx); err != nil { return err @@ -360,8 +360,8 @@ func (d *dentry) restoreDescendantsRecursive(ctx context.Context, opts *vfs.Comp if child == nil { continue } - if _, ok := d.fs.syncableDentries[child]; !ok { - // child is synthetic. + // child is synthetic if it does not exist in fs.syncableDentries. + if child.syncableListEntry.Next() == nil && child.syncableListEntry.Prev() == nil && d.fs.syncableDentries.Front() != &child.syncableListEntry { continue } if err := child.restoreRecursive(ctx, opts); err != nil { diff --git a/pkg/sentry/fsimpl/gofer/special_file.go b/pkg/sentry/fsimpl/gofer/special_file.go index 06b3e235c..7336eaf11 100644 --- a/pkg/sentry/fsimpl/gofer/special_file.go +++ b/pkg/sentry/fsimpl/gofer/special_file.go @@ -44,6 +44,7 @@ import ( // +stateify savable type specialFileFD struct { fileDescription + specialFDEntry // releaseMu synchronizes the closing of fd.handle with fd.sync(). It's safe // to access fd.handle without locking for operations that require a ref to @@ -116,7 +117,7 @@ func newSpecialFileFD(h handle, mnt *vfs.Mount, d *dentry, flags uint32) (*speci return nil, err } d.fs.syncMu.Lock() - d.fs.specialFileFDs[fd] = struct{}{} + d.fs.specialFileFDs.PushBack(fd) d.fs.syncMu.Unlock() if fd.vfsfd.IsWritable() && (d.mode.Load()&0111 != 0) { metric.SuspiciousOperationsMetric.Increment("opened_write_execute_file") @@ -140,7 +141,7 @@ func (fd *specialFileFD) Release(ctx context.Context) { fs := fd.vfsfd.Mount().Filesystem().Impl().(*filesystem) fs.syncMu.Lock() - delete(fs.specialFileFDs, fd) + fs.specialFileFDs.Remove(fd) fs.syncMu.Unlock() } diff --git a/pkg/state/BUILD b/pkg/state/BUILD index 92c51879b..b8a675970 100644 --- a/pkg/state/BUILD +++ b/pkg/state/BUILD @@ -11,8 +11,7 @@ go_template_instance( template = "//pkg/ilist:generic_list", types = { "Element": "*objectEncodeState", - "ElementMapper": "deferredMapper", - "Linker": "*deferredEntry", + "Linker": "*objectEncodeState", }, ) diff --git a/pkg/state/encode.go b/pkg/state/encode.go index 157ac0dd2..9f15c3c2e 100644 --- a/pkg/state/encode.go +++ b/pkg/state/encode.go @@ -839,11 +839,6 @@ func WriteHeader(w wire.Writer, length uint64, object bool) error { }) } -// deferredMapper is for the deferred list. -type deferredMapper struct{} - -func (deferredMapper) linkerFor(oes *objectEncodeState) *deferredEntry { return &oes.deferredEntry } - // addrSetFunctions is used by addrSet. type addrSetFunctions struct{}