summaryrefslogtreecommitdiffhomepage
path: root/pkg/sentry/fsimpl/gofer
diff options
context:
space:
mode:
Diffstat (limited to 'pkg/sentry/fsimpl/gofer')
-rw-r--r--pkg/sentry/fsimpl/gofer/BUILD12
-rw-r--r--pkg/sentry/fsimpl/gofer/directory.go55
-rw-r--r--pkg/sentry/fsimpl/gofer/filesystem.go202
-rw-r--r--pkg/sentry/fsimpl/gofer/gofer.go66
-rw-r--r--pkg/sentry/fsimpl/gofer/gofer_test.go3
5 files changed, 194 insertions, 144 deletions
diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD
index 99d1e3f8f..acd061905 100644
--- a/pkg/sentry/fsimpl/gofer/BUILD
+++ b/pkg/sentry/fsimpl/gofer/BUILD
@@ -15,12 +15,24 @@ go_template_instance(
},
)
+go_template_instance(
+ name = "fstree",
+ out = "fstree.go",
+ package = "gofer",
+ prefix = "generic",
+ template = "//pkg/sentry/vfs/genericfstree:generic_fstree",
+ types = {
+ "Dentry": "dentry",
+ },
+)
+
go_library(
name = "gofer",
srcs = [
"dentry_list.go",
"directory.go",
"filesystem.go",
+ "fstree.go",
"gofer.go",
"handle.go",
"handle_unsafe.go",
diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go
index 49d9f859b..d02691232 100644
--- a/pkg/sentry/fsimpl/gofer/directory.go
+++ b/pkg/sentry/fsimpl/gofer/directory.go
@@ -29,13 +29,25 @@ func (d *dentry) isDir() bool {
return d.fileType() == linux.S_IFDIR
}
+// Preconditions: filesystem.renameMu must be locked. d.dirMu must be locked.
+// d.isDir(). child must be a newly-created dentry that has never had a parent.
+func (d *dentry) cacheNewChildLocked(child *dentry, name string) {
+ d.IncRef() // reference held by child on its parent
+ child.parent = d
+ child.name = name
+ if d.children == nil {
+ d.children = make(map[string]*dentry)
+ }
+ d.children[name] = child
+}
+
// Preconditions: d.dirMu must be locked. d.isDir(). fs.opts.interop !=
// InteropModeShared.
func (d *dentry) cacheNegativeChildLocked(name string) {
- if d.negativeChildren == nil {
- d.negativeChildren = make(map[string]struct{})
+ if d.children == nil {
+ d.children = make(map[string]*dentry)
}
- d.negativeChildren[name] = struct{}{}
+ d.children[name] = nil
}
type directoryFD struct {
@@ -80,34 +92,32 @@ func (fd *directoryFD) IterDirents(ctx context.Context, cb vfs.IterDirentsCallba
// Preconditions: d.isDir(). There exists at least one directoryFD representing d.
func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
- // 9P2000.L's readdir does not specify behavior in the presence of
- // concurrent mutation of an iterated directory, so implementations may
- // duplicate or omit entries in this case, which violates POSIX semantics.
- // Thus we read all directory entries while holding d.dirMu to exclude
- // directory mutations. (Note that it is impossible for the client to
- // exclude concurrent mutation from other remote filesystem users. Since
- // there is no way to detect if the server has incorrectly omitted
- // directory entries, we simply assume that the server is well-behaved
- // under InteropModeShared.) This is inconsistent with Linux (which appears
- // to assume that directory fids have the correct semantics, and translates
- // struct file_operations::readdir calls directly to readdir RPCs), but is
- // consistent with VFS1.
- //
- // NOTE(b/135560623): In particular, some gofer implementations may not
- // retain state between calls to Readdir, so may not provide a coherent
- // directory stream across in the presence of mutation.
-
+ // NOTE(b/135560623): 9P2000.L's readdir does not specify behavior in the
+ // presence of concurrent mutation of an iterated directory, so
+ // implementations may duplicate or omit entries in this case, which
+ // violates POSIX semantics. Thus we read all directory entries while
+ // holding d.dirMu to exclude directory mutations. (Note that it is
+ // impossible for the client to exclude concurrent mutation from other
+ // remote filesystem users. Since there is no way to detect if the server
+ // has incorrectly omitted directory entries, we simply assume that the
+ // server is well-behaved under InteropModeShared.) This is inconsistent
+ // with Linux (which appears to assume that directory fids have the correct
+ // semantics, and translates struct file_operations::readdir calls directly
+ // to readdir RPCs), but is consistent with VFS1.
+
+ // filesystem.renameMu is needed for d.parent, and must be locked before
+ // dentry.dirMu.
d.fs.renameMu.RLock()
- defer d.fs.renameMu.RUnlock()
d.dirMu.Lock()
defer d.dirMu.Unlock()
if d.dirents != nil {
+ d.fs.renameMu.RUnlock()
return d.dirents, nil
}
// It's not clear if 9P2000.L's readdir is expected to return "." and "..",
// so we generate them here.
- parent := d.vfsd.ParentOrSelf().Impl().(*dentry)
+ parent := genericParentOrSelf(d)
dirents := []vfs.Dirent{
{
Name: ".",
@@ -122,6 +132,7 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
NextOff: 2,
},
}
+ d.fs.renameMu.RUnlock()
off := uint64(0)
const count = 64 * 1024 // for consistency with the vfs1 client
d.handleMu.RLock()
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index cd744bf5e..43e863c61 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -116,6 +116,8 @@ func putDentrySlice(ds *[]*dentry) {
// Preconditions: fs.renameMu must be locked. d.dirMu must be locked.
// !rp.Done(). If fs.opts.interop == InteropModeShared, then d's cached
// metadata must be up to date.
+//
+// Postconditions: The returned dentry's cached metadata is up to date.
func (fs *filesystem) stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry, ds **[]*dentry) (*dentry, error) {
if !d.isDir() {
return nil, syserror.ENOTDIR
@@ -130,39 +132,42 @@ afterSymlink:
return d, nil
}
if name == ".." {
- parentVFSD, err := rp.ResolveParent(&d.vfsd)
- if err != nil {
+ if isRoot, err := rp.CheckRoot(&d.vfsd); err != nil {
+ return nil, err
+ } else if isRoot || d.parent == nil {
+ rp.Advance()
+ return d, nil
+ }
+ // We must assume that d.parent is correct, because if d has been moved
+ // elsewhere in the remote filesystem so that its parent has changed,
+ // we have no way of determining its new parent's location in the
+ // filesystem.
+ //
+ // Call rp.CheckMount() before updating d.parent's metadata, since if
+ // we traverse to another mount then d.parent's metadata is irrelevant.
+ if err := rp.CheckMount(&d.parent.vfsd); err != nil {
return nil, err
}
- parent := parentVFSD.Impl().(*dentry)
- if fs.opts.interop == InteropModeShared {
- // We must assume that parentVFSD is correct, because if d has been
- // moved elsewhere in the remote filesystem so that its parent has
- // changed, we have no way of determining its new parent's location
- // in the filesystem. Get updated metadata for parentVFSD.
- _, attrMask, attr, err := parent.file.getAttr(ctx, dentryAttrMask())
+ if fs.opts.interop == InteropModeShared && d != d.parent {
+ _, attrMask, attr, err := d.parent.file.getAttr(ctx, dentryAttrMask())
if err != nil {
return nil, err
}
- parent.updateFromP9Attrs(attrMask, &attr)
+ d.parent.updateFromP9Attrs(attrMask, &attr)
}
rp.Advance()
- return parent, nil
+ return d.parent, nil
}
- childVFSD, err := rp.ResolveChild(&d.vfsd, name)
- if err != nil {
- return nil, err
- }
- // FIXME(jamieliu): Linux performs revalidation before mount lookup
- // (fs/namei.c:lookup_fast() => __d_lookup_rcu(), d_revalidate(),
- // __follow_mount_rcu()).
- child, err := fs.revalidateChildLocked(ctx, rp.VirtualFilesystem(), d, name, childVFSD, ds)
+ child, err := fs.getChildLocked(ctx, rp.VirtualFilesystem(), d, name, ds)
if err != nil {
return nil, err
}
if child == nil {
return nil, syserror.ENOENT
}
+ if err := rp.CheckMount(&child.vfsd); err != nil {
+ return nil, err
+ }
if child.isSymlink() && rp.ShouldFollowSymlink() {
target, err := child.readlink(ctx, rp.Mount())
if err != nil {
@@ -177,38 +182,37 @@ afterSymlink:
return child, nil
}
-// revalidateChildLocked must be called after a call to parent.vfsd.Child(name)
-// or vfs.ResolvingPath.ResolveChild(name) returns childVFSD (which may be
-// nil) to verify that the returned child (or lack thereof) is correct. If no file
-// exists at name, revalidateChildLocked returns (nil, nil).
+// getChildLocked returns a dentry representing the child of parent with the
+// given name. If no such child exists, getChildLocked returns (nil, nil).
//
// Preconditions: fs.renameMu must be locked. parent.dirMu must be locked.
// parent.isDir(). name is not "." or "..".
//
-// Postconditions: If revalidateChildLocked returns a non-nil dentry, its
-// cached metadata is up to date.
-func (fs *filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.VirtualFilesystem, parent *dentry, name string, childVFSD *vfs.Dentry, ds **[]*dentry) (*dentry, error) {
- if childVFSD != nil && fs.opts.interop != InteropModeShared {
- // We have a cached dentry that is assumed to be correct.
- return childVFSD.Impl().(*dentry), nil
- }
- // We either don't have a cached dentry or need to verify that it's still
- // correct, either of which requires a remote lookup. Check if this name is
- // valid before performing the lookup.
+// Postconditions: If getChildLocked returns a non-nil dentry, its cached
+// metadata is up to date.
+func (fs *filesystem) getChildLocked(ctx context.Context, vfsObj *vfs.VirtualFilesystem, parent *dentry, name string, ds **[]*dentry) (*dentry, error) {
if len(name) > maxFilenameLen {
return nil, syserror.ENAMETOOLONG
}
- // Check if we've already cached this lookup with a negative result.
- if _, ok := parent.negativeChildren[name]; ok {
- return nil, nil
+ child, ok := parent.children[name]
+ if ok && fs.opts.interop != InteropModeShared {
+ // Whether child is nil or not, it is cached information that is
+ // assumed to be correct.
+ return child, nil
}
- // Perform the remote lookup.
+ // We either don't have cached information or need to verify that it's
+ // still correct, either of which requires a remote lookup. Check if this
+ // name is valid before performing the lookup.
+ return fs.revalidateChildLocked(ctx, vfsObj, parent, name, child, ds)
+}
+
+// Preconditions: As for getChildLocked.
+func (fs *filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.VirtualFilesystem, parent *dentry, name string, child *dentry, ds **[]*dentry) (*dentry, error) {
qid, file, attrMask, attr, err := parent.file.walkGetAttrOne(ctx, name)
if err != nil && err != syserror.ENOENT {
return nil, err
}
- if childVFSD != nil {
- child := childVFSD.Impl().(*dentry)
+ if child != nil {
if !file.isNil() && qid.Path == child.ino {
// The file at this path hasn't changed. Just update cached
// metadata.
@@ -219,9 +223,8 @@ func (fs *filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir
// The file at this path has changed or no longer exists. Remove
// the stale dentry from the tree, and re-evaluate its caching
// status (i.e. if it has 0 references, drop it).
- vfsObj.ForceDeleteDentry(childVFSD)
+ vfsObj.InvalidateDentry(&child.vfsd)
*ds = appendDentry(*ds, child)
- childVFSD = nil
}
if file.isNil() {
// No file exists at this path now. Cache the negative lookup if
@@ -232,13 +235,12 @@ func (fs *filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir
return nil, nil
}
// Create a new dentry representing the file.
- child, err := fs.newDentry(ctx, file, qid, attrMask, &attr)
+ child, err = fs.newDentry(ctx, file, qid, attrMask, &attr)
if err != nil {
file.close(ctx)
return nil, err
}
- parent.IncRef() // reference held by child on its parent
- parent.vfsd.InsertChild(&child.vfsd, name)
+ parent.cacheNewChildLocked(child, name)
// For now, child has 0 references, so our caller should call
// child.checkCachingLocked().
*ds = appendDentry(*ds, child)
@@ -318,9 +320,6 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if err := parent.checkPermissions(rp.Credentials(), vfs.MayWrite|vfs.MayExec); err != nil {
return err
}
- if parent.isDeleted() {
- return syserror.ENOENT
- }
name := rp.Component()
if name == "." || name == ".." {
return syserror.EEXIST
@@ -331,6 +330,9 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if !dir && rp.MustBeDir() {
return syserror.ENOENT
}
+ if parent.isDeleted() {
+ return syserror.ENOENT
+ }
mnt := rp.Mount()
if err := mnt.CheckBeginWrite(); err != nil {
return err
@@ -348,7 +350,7 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
// it's used.
return create(parent, name)
}
- if parent.vfsd.Child(name) != nil {
+ if child := parent.children[name]; child != nil {
return syserror.EEXIST
}
// No cached dentry exists; however, there might still be an existing file
@@ -356,10 +358,11 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir
if err := create(parent, name); err != nil {
return err
}
- if fs.opts.interop != InteropModeShared {
- parent.touchCMtime()
- }
- delete(parent.negativeChildren, name)
+ parent.touchCMtime()
+ // Either parent.children[name] doesn't exist (in which case this is a
+ // no-op) or is nil (in which case this erases the now-stale information
+ // that the file doesn't exist).
+ delete(parent.children, name)
parent.dirents = nil
return nil
}
@@ -407,56 +410,55 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b
defer mntns.DecRef()
parent.dirMu.Lock()
defer parent.dirMu.Unlock()
- childVFSD := parent.vfsd.Child(name)
- var child *dentry
+ child, ok := parent.children[name]
+ if ok && child == nil {
+ return syserror.ENOENT
+ }
// We only need a dentry representing the file at name if it can be a mount
- // point. If childVFSD is nil, then it can't be a mount point. If childVFSD
- // is non-nil but stale, the actual file can't be a mount point either; we
+ // point. If child is nil, then it can't be a mount point. If child is
+ // non-nil but stale, the actual file can't be a mount point either; we
// detect this case by just speculatively calling PrepareDeleteDentry and
// only revalidating the dentry if that fails (indicating that the existing
// dentry is a mount point).
- if childVFSD != nil {
- child = childVFSD.Impl().(*dentry)
- if err := vfsObj.PrepareDeleteDentry(mntns, childVFSD); err != nil {
- child, err = fs.revalidateChildLocked(ctx, vfsObj, parent, name, childVFSD, &ds)
+ if child != nil {
+ if err := vfsObj.PrepareDeleteDentry(mntns, &child.vfsd); err != nil {
+ if fs.opts.interop != InteropModeShared {
+ return err
+ }
+ child, err = fs.revalidateChildLocked(ctx, vfsObj, parent, name, child, &ds)
if err != nil {
return err
}
if child != nil {
- childVFSD = &child.vfsd
- if err := vfsObj.PrepareDeleteDentry(mntns, childVFSD); err != nil {
+ if err := vfsObj.PrepareDeleteDentry(mntns, &child.vfsd); err != nil {
return err
}
- } else {
- childVFSD = nil
}
}
- } else if _, ok := parent.negativeChildren[name]; ok {
- return syserror.ENOENT
}
flags := uint32(0)
if dir {
if child != nil && !child.isDir() {
- vfsObj.AbortDeleteDentry(childVFSD)
+ vfsObj.AbortDeleteDentry(&child.vfsd)
return syserror.ENOTDIR
}
flags = linux.AT_REMOVEDIR
} else {
if child != nil && child.isDir() {
- vfsObj.AbortDeleteDentry(childVFSD)
+ vfsObj.AbortDeleteDentry(&child.vfsd)
return syserror.EISDIR
}
if rp.MustBeDir() {
- if childVFSD != nil {
- vfsObj.AbortDeleteDentry(childVFSD)
+ if child != nil {
+ vfsObj.AbortDeleteDentry(&child.vfsd)
}
return syserror.ENOTDIR
}
}
err = parent.file.unlinkAt(ctx, name, flags)
if err != nil {
- if childVFSD != nil {
- vfsObj.AbortDeleteDentry(childVFSD)
+ if child != nil {
+ vfsObj.AbortDeleteDentry(&child.vfsd)
}
return err
}
@@ -467,10 +469,12 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b
}
parent.cacheNegativeChildLocked(name)
parent.dirents = nil
+ } else {
+ delete(parent.children, name)
}
if child != nil {
child.setDeleted()
- vfsObj.CommitDeleteDentry(childVFSD)
+ vfsObj.CommitDeleteDentry(&child.vfsd)
ds = appendDentry(ds, child)
}
return nil
@@ -806,16 +810,14 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
// eligible for caching yet, so we don't need to append to a dentry slice.)
child.refs = 1
// Insert the dentry into the tree.
- d.IncRef() // reference held by child on its parent d
- d.vfsd.InsertChild(&child.vfsd, name)
+ d.cacheNewChildLocked(child, name)
if d.fs.opts.interop != InteropModeShared {
- delete(d.negativeChildren, name)
+ d.touchCMtime()
d.dirents = nil
}
// Finally, construct a file description representing the created file.
var childVFSFD *vfs.FileDescription
- mnt.IncRef()
if useRegularFileFD {
fd := &regularFileFD{}
if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &child.vfsd, &vfs.FileDescriptionOptions{
@@ -840,9 +842,6 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
}
childVFSFD = &fd.vfsfd
}
- if d.fs.opts.interop != InteropModeShared {
- d.touchCMtime()
- }
return childVFSFD, nil
}
@@ -902,7 +901,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
// directory, we need to check for write permission on it.
oldParent.dirMu.Lock()
defer oldParent.dirMu.Unlock()
- renamed, err := fs.revalidateChildLocked(ctx, vfsObj, oldParent, oldName, oldParent.vfsd.Child(oldName), &ds)
+ renamed, err := fs.getChildLocked(ctx, vfsObj, oldParent, oldName, &ds)
if err != nil {
return err
}
@@ -910,7 +909,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
return syserror.ENOENT
}
if renamed.isDir() {
- if renamed == newParent || renamed.vfsd.IsAncestorOf(&newParent.vfsd) {
+ if renamed == newParent || genericIsAncestorDentry(renamed, newParent) {
return syserror.EINVAL
}
if oldParent != newParent {
@@ -934,16 +933,17 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if newParent.isDeleted() {
return syserror.ENOENT
}
- replacedVFSD := newParent.vfsd.Child(newName)
- var replaced *dentry
+ replaced := newParent.children[newName]
// This is similar to unlinkAt, except:
//
- // - We revalidate the replaced dentry unconditionally for simplicity.
+ // - If a dentry exists for the file to be replaced, we revalidate it
+ // unconditionally (instead of only if PrepareRenameDentry fails) for
+ // simplicity.
//
// - If rp.MustBeDir(), then we need a dentry representing the replaced
// file regardless to confirm that it's a directory.
- if replacedVFSD != nil || rp.MustBeDir() {
- replaced, err = fs.revalidateChildLocked(ctx, vfsObj, newParent, newName, replacedVFSD, &ds)
+ if replaced != nil || rp.MustBeDir() {
+ replaced, err = fs.getChildLocked(ctx, rp.VirtualFilesystem(), newParent, newName, &ds)
if err != nil {
return err
}
@@ -957,11 +957,12 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
return syserror.ENOTDIR
}
}
- replacedVFSD = &replaced.vfsd
- } else {
- replacedVFSD = nil
}
}
+ var replacedVFSD *vfs.Dentry
+ if replaced != nil {
+ replacedVFSD = &replaced.vfsd
+ }
if oldParent == newParent && oldName == newName {
return nil
@@ -978,7 +979,6 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
if fs.opts.interop != InteropModeShared {
oldParent.cacheNegativeChildLocked(oldName)
oldParent.dirents = nil
- delete(newParent.negativeChildren, newName)
newParent.dirents = nil
if renamed.isDir() {
oldParent.decLinks()
@@ -987,8 +987,24 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
oldParent.touchCMtime()
newParent.touchCMtime()
renamed.touchCtime()
+ } else {
+ delete(oldParent.children, oldName)
+ }
+ if oldParent != newParent {
+ appendDentry(ds, oldParent)
+ newParent.IncRef()
+ }
+ renamed.parent = newParent
+ renamed.name = newName
+ if newParent.children == nil {
+ newParent.children = make(map[string]*dentry)
+ }
+ newParent.children[newName] = renamed
+ if replaced != nil {
+ replaced.setDeleted()
+ appendDentry(ds, replaced)
}
- vfsObj.CommitRenameReplaceDentry(&renamed.vfsd, &newParent.vfsd, newName, replacedVFSD)
+ vfsObj.CommitRenameReplaceDentry(&renamed.vfsd, replacedVFSD)
return nil
}
@@ -1131,5 +1147,5 @@ func (fs *filesystem) RemovexattrAt(ctx context.Context, rp *vfs.ResolvingPath,
func (fs *filesystem) PrependPath(ctx context.Context, vfsroot, vd vfs.VirtualDentry, b *fspath.Builder) error {
fs.renameMu.RLock()
defer fs.renameMu.RUnlock()
- return vfs.GenericPrependPath(vfsroot, vd, b)
+ return genericPrependPath(vfsroot, vd.Mount(), vd.Dentry().Impl().(*dentry), b)
}
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 2485cdb53..293df2545 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -452,6 +452,16 @@ type dentry struct {
// fs is the owning filesystem. fs is immutable.
fs *filesystem
+ // parent is this dentry's parent directory. Each dentry holds a reference
+ // on its parent. If this dentry is a filesystem root, parent is nil.
+ // parent is protected by filesystem.renameMu.
+ parent *dentry
+
+ // name is the name of this dentry in its parent. If this dentry is a
+ // filesystem root, name is the empty string. name is protected by
+ // filesystem.renameMu.
+ name string
+
// We don't support hard links, so each dentry maps 1:1 to an inode.
// file is the unopened p9.File that backs this dentry. file is immutable.
@@ -469,10 +479,15 @@ type dentry struct {
dirMu sync.Mutex
- // If this dentry represents a directory, and InteropModeShared is not in
- // effect, negativeChildren is a set of child names in this directory that
- // are known not to exist. negativeChildren is protected by dirMu.
- negativeChildren map[string]struct{}
+ // If this dentry represents a directory, children contains:
+ //
+ // - Mappings of child filenames to dentries representing those children.
+ //
+ // - Mappings of child filenames that are known not to exist to nil
+ // dentries (only if InteropModeShared is not in effect).
+ //
+ // children is protected by dirMu.
+ children map[string]*dentry
// If this dentry represents a directory, InteropModeShared is not in
// effect, and dirents is not nil, it is a cache of all entries in the
@@ -910,9 +925,9 @@ func (d *dentry) checkCachingLocked() {
// Dentry has already been destroyed.
return
}
- // Non-child dentries with zero references are no longer reachable by path
- // resolution and should be dropped immediately.
- if d.vfsd.Parent() == nil || d.vfsd.IsDisowned() {
+ // Deleted and invalidated dentries with zero references are no longer
+ // reachable by path resolution and should be dropped immediately.
+ if d.vfsd.IsDead() {
if d.cached {
d.fs.cachedDentries.Remove(d)
d.fs.cachedDentriesLen--
@@ -937,28 +952,26 @@ func (d *dentry) checkCachingLocked() {
d.fs.cachedDentries.Remove(victim)
d.fs.cachedDentriesLen--
victim.cached = false
- // victim.refs may have become non-zero from an earlier path
- // resolution since it was inserted into fs.cachedDentries; see
- // dentry.incRefLocked(). Either way, we brought
- // fs.cachedDentriesLen back down to fs.opts.maxCachedDentries, so
- // we don't loop.
+ // victim.refs may have become non-zero from an earlier path resolution
+ // since it was inserted into fs.cachedDentries.
if atomic.LoadInt64(&victim.refs) == 0 {
- if victimParentVFSD := victim.vfsd.Parent(); victimParentVFSD != nil {
- victimParent := victimParentVFSD.Impl().(*dentry)
- victimParent.dirMu.Lock()
- if !victim.vfsd.IsDisowned() {
- // victim can't be a mount point (in any mount
- // namespace), since VFS holds references on mount
- // points.
- d.fs.vfsfs.VirtualFilesystem().ForceDeleteDentry(&victim.vfsd)
+ if victim.parent != nil {
+ victim.parent.dirMu.Lock()
+ if !victim.vfsd.IsDead() {
+ // Note that victim can't be a mount point (in any mount
+ // namespace), since VFS holds references on mount points.
+ d.fs.vfsfs.VirtualFilesystem().InvalidateDentry(&victim.vfsd)
+ delete(victim.parent.children, victim.name)
// We're only deleting the dentry, not the file it
// represents, so we don't need to update
// victimParent.dirents etc.
}
- victimParent.dirMu.Unlock()
+ victim.parent.dirMu.Unlock()
}
victim.destroyLocked()
}
+ // Whether or not victim was destroyed, we brought fs.cachedDentriesLen
+ // back down to fs.opts.maxCachedDentries, so we don't loop.
}
}
@@ -1005,12 +1018,11 @@ func (d *dentry) destroyLocked() {
d.fs.syncMu.Lock()
delete(d.fs.dentries, d)
d.fs.syncMu.Unlock()
- // Drop the reference held by d on its parent.
- if parentVFSD := d.vfsd.Parent(); parentVFSD != nil {
- parent := parentVFSD.Impl().(*dentry)
- // This is parent.DecRef() without recursive locking of d.fs.renameMu.
- if refs := atomic.AddInt64(&parent.refs, -1); refs == 0 {
- parent.checkCachingLocked()
+ // Drop the reference held by d on its parent without recursively locking
+ // d.fs.renameMu.
+ if d.parent != nil {
+ if refs := atomic.AddInt64(&d.parent.refs, -1); refs == 0 {
+ d.parent.checkCachingLocked()
} else if refs < 0 {
panic("gofer.dentry.DecRef() called without holding a reference")
}
diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go
index 82bc239db..4041fb252 100644
--- a/pkg/sentry/fsimpl/gofer/gofer_test.go
+++ b/pkg/sentry/fsimpl/gofer/gofer_test.go
@@ -48,8 +48,7 @@ func TestDestroyIdempotent(t *testing.T) {
if err != nil {
t.Fatalf("fs.newDentry(): %v", err)
}
- parent.IncRef() // reference held by child on its parent.
- parent.vfsd.InsertChild(&child.vfsd, "child")
+ parent.cacheNewChildLocked(child, "child")
child.checkCachingLocked()
if got := atomic.LoadInt64(&child.refs); got != -1 {