Skip to content

Commit

Permalink
fsimpl/gofer: hold extra dentry reference if endpoint != nil
Browse files Browse the repository at this point in the history
Fixes #11202

PiperOrigin-RevId: 700163466
  • Loading branch information
nixprime authored and gvisor-bot committed Nov 26, 2024
1 parent 8079a6c commit 0e6fe26
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 16 deletions.
4 changes: 3 additions & 1 deletion pkg/sentry/fsimpl/gofer/directfs_dentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,10 @@ func (d *directfsDentry) bindAt(ctx context.Context, name string, creds *auth.Cr
hbep.ResetBoundSocketFD(ctx)
return nil, err
}
// Set the endpoint on the newly created child dentry.
// Set the endpoint on the newly created child dentry, and take the
// corresponding extra dentry reference.
child.endpoint = opts.Endpoint
child.IncRef()
return child, nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/sentry/fsimpl/gofer/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,8 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b
if child.isSynthetic() {
parent.syntheticChildren--
child.decRefNoCaching()
} else if child.endpoint != nil {
child.decRefNoCaching()
}
ds = appendDentry(ds, child)
}
Expand Down Expand Up @@ -1508,6 +1510,8 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if replaced.isSynthetic() {
newParent.syntheticChildren--
replaced.decRefNoCaching()
} else if replaced.endpoint != nil {
replaced.decRefNoCaching()
}
ds = appendDentry(ds, replaced)
// Remove the replaced entry from its parent's cache.
Expand Down
34 changes: 21 additions & 13 deletions pkg/sentry/fsimpl/gofer/gofer.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ func (fs *filesystem) Release(ctx context.Context) {
// root dentry failed in GetFilesystem.
if refs.GetLeakMode() != refs.NoLeakChecking && fs.root != nil {
fs.renameMu.Lock()
fs.root.releaseSyntheticRecursiveLocked(ctx)
fs.root.releaseExtraRefsRecursiveLocked(ctx)
fs.evictAllCachedDentriesLocked(ctx)
fs.renameMu.Unlock()

Expand All @@ -741,13 +741,14 @@ func (fs *filesystem) Release(ctx context.Context) {
fs.vfsfs.VirtualFilesystem().PutAnonBlockDevMinor(fs.devMinor)
}

// releaseSyntheticRecursiveLocked traverses the tree with root d and decrements
// the reference count on every synthetic dentry. Synthetic dentries have one
// reference for existence that should be dropped during filesystem.Release.
// releaseExtraRefsRecursiveLocked traverses the tree with root d and
// decrements the reference count on every synthetic dentry and dentry with
// endpoint != nil. Such dentries have one reference for existence that should
// be dropped during filesystem.Release.
//
// Precondition: d.fs.renameMu is locked for writing.
func (d *dentry) releaseSyntheticRecursiveLocked(ctx context.Context) {
if d.isSynthetic() {
func (d *dentry) releaseExtraRefsRecursiveLocked(ctx context.Context) {
if d.isSynthetic() || d.endpoint != nil {
d.decRefNoCaching()
d.checkCachingLocked(ctx, true /* renameMuWriteLocked */)
}
Expand All @@ -760,7 +761,7 @@ func (d *dentry) releaseSyntheticRecursiveLocked(ctx context.Context) {
d.childrenMu.Unlock()
for _, child := range children {
if child != nil {
child.releaseSyntheticRecursiveLocked(ctx)
child.releaseExtraRefsRecursiveLocked(ctx)
}
}
}
Expand Down Expand Up @@ -799,10 +800,12 @@ type dentry struct {

// refs is the reference count. Each dentry holds a reference on its
// parent, even if disowned. An additional reference is held on all
// synthetic dentries until they are unlinked or invalidated. When refs
// reaches 0, the dentry may be added to the cache or destroyed. If refs ==
// -1, the dentry has already been destroyed. refs is accessed using atomic
// memory operations.
// synthetic dentries, and all dentries for which endpoint is non-nil,
// until they are unlinked or invalidated. (Only a single additional
// reference is held on synthetic dentries that also have endpoint != nil.)
// When refs reaches 0, the dentry may be added to the cache or destroyed.
// If refs == -1, the dentry has already been destroyed. refs is accessed
// using atomic memory operations.
refs atomicbitops.Int64

// fs is the owning filesystem. fs is immutable.
Expand Down Expand Up @@ -984,8 +987,13 @@ type dentry struct {
haveTarget bool
target string

// If this dentry represents a synthetic socket file, endpoint is the
// transport endpoint bound to this file.
// If this dentry represents a socket file, endpoint is the transport
// endpoint bound to this file.
//
// endpoint often originates from vfs.MknodOptions.Endpoint, in which case
// it can't be recovered if the dentry is evicted from the dentry cache.
// Consequently, an extra reference is held on dentries for which endpoint
// is non-nil to prevent eviction.
endpoint transport.BoundEndpoint

// If this dentry represents a synthetic named pipe, pipe is the pipe
Expand Down
4 changes: 3 additions & 1 deletion pkg/sentry/fsimpl/gofer/lisafs_dentry.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,10 @@ func (d *lisafsDentry) mknod(ctx context.Context, name string, creds *auth.Crede
hbep.ResetBoundSocketFD(ctx)
return nil, err
}
// Set the endpoint on the newly created child dentry.
// Set the endpoint on the newly created child dentry, and take the
// corresponding extra dentry reference.
child.endpoint = opts.Endpoint
child.IncRef()
return child, nil
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/sentry/fsimpl/gofer/revalidate.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
rc.DecRef(ctx)
}
d.decRefNoCaching()
if d.isSynthetic() || d.endpoint != nil {
d.decRefNoCaching()
}

// Re-evaluate its caching status (i.e. if it has 0 references, drop it).
// The dentry will be reloaded next time it's accessed.
Expand All @@ -253,7 +256,6 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem,
// +checklocks:parent.childrenMu
func (d *dentry) deleteSynthetic(parent *dentry, ds **[]*dentry) {
d.setDeleted()
d.decRefNoCaching()
*ds = appendDentry(*ds, d)
parent.syntheticChildren--
parent.clearDirentsLocked()
Expand Down

0 comments on commit 0e6fe26

Please sign in to comment.