diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 899de8377..78d54f65f 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -505,14 +505,14 @@ func (fs *filesystem) restoreRoot(ctx context.Context, opts *vfs.CompleteRestore func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { switch dt := d.impl.(type) { case *lisafsDentry: - inode, err := d.parent.impl.(*lisafsDentry).controlFD.Walk(ctx, d.name) + inode, err := d.parent.Load().impl.(*lisafsDentry).controlFD.Walk(ctx, d.name) if err != nil { return err } return dt.restoreFile(ctx, &inode, opts) case *directfsDentry: childFD, err := tryOpen(func(flags int) (int, error) { - return unix.Openat(d.parent.impl.(*directfsDentry).controlFD, d.name, flags, 0) + return unix.Openat(d.parent.Load().impl.(*directfsDentry).controlFD, d.name, flags, 0) }) if err != nil { return err diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index a9429cc85..3deac2609 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -148,7 +148,8 @@ func (fs *filesystem) newDirectfsDentry(controlFD int) (*dentry, error) { // Precondition: fs.renameMu is locked. func (d *directfsDentry) openHandle(ctx context.Context, flags uint32) (handle, error) { - if d.parent == nil { + parent := d.parent.Load() + if parent == nil { // This is a mount point. We don't have parent. Fallback to using lisafs. if !d.controlFDLisa.Ok() { panic("directfsDentry.controlFDLisa is not set for mount point dentry") @@ -168,7 +169,7 @@ func (d *directfsDentry) openHandle(ctx context.Context, flags uint32) (handle, // The only way to re-open an FD with different flags is via procfs or // openat(2) from the parent. Procfs does not exist here. So use parent. flags |= hostOpenFlags - openFD, err := unix.Openat(d.parent.impl.(*directfsDentry).controlFD, d.name, int(flags), 0) + openFD, err := unix.Openat(parent.impl.(*directfsDentry).controlFD, d.name, int(flags), 0) if err != nil { return noHandle, err } @@ -185,9 +186,9 @@ func (d *directfsDentry) ensureLisafsControlFD(ctx context.Context) error { var names []string root := d - for root.parent != nil { + for root.parent.Load() != nil { names = append(names, root.name) - root = root.parent.impl.(*directfsDentry) + root = root.parent.Load().impl.(*directfsDentry) } if !root.controlFDLisa.Ok() { panic("controlFDLisa is not set for mount point dentry") @@ -270,10 +271,10 @@ func (d *directfsDentry) chmod(ctx context.Context, mode uint16) error { // fchmod(2) on socket files created via bind(2) fails. We need to // fchmodat(2) it from its parent. - if d.parent != nil { + if parent := d.parent.Load(); parent != nil { // We have parent FD, just use that. Note that AT_SYMLINK_NOFOLLOW flag is // currently not supported. So we don't use it. - return unix.Fchmodat(d.parent.impl.(*directfsDentry).controlFD, d.name, uint32(mode), 0 /* flags */) + return unix.Fchmodat(parent.impl.(*directfsDentry).controlFD, d.name, uint32(mode), 0 /* flags */) } // This is a mount point socket. We don't have a parent FD. Fallback to using @@ -320,8 +321,8 @@ func (d *directfsDentry) utimensat(ctx context.Context, stat *linux.Statx) error // utimensat operates different that other syscalls. To operate on a // symlink it *requires* AT_SYMLINK_NOFOLLOW with dirFD and a non-empty // name. - if d.parent != nil { - return fsutil.Utimensat(d.parent.impl.(*directfsDentry).controlFD, d.name, utimes, unix.AT_SYMLINK_NOFOLLOW) + if parent := d.parent.Load(); parent != nil { + return fsutil.Utimensat(parent.impl.(*directfsDentry).controlFD, d.name, utimes, unix.AT_SYMLINK_NOFOLLOW) } // This is a mount point symlink. We don't have a parent FD. Fallback to @@ -521,7 +522,7 @@ func (d *directfsDentry) link(target *directfsDentry, name string) (*dentry, err // using olddirfd to call linkat(2). // Also note that d and target are from the same mount. Given target is a // non-directory and d is a directory, target.parent must exist. - if err := unix.Linkat(target.parent.impl.(*directfsDentry).controlFD, target.name, d.controlFD, name, 0); err != nil { + if err := unix.Linkat(target.parent.Load().impl.(*directfsDentry).controlFD, target.name, d.controlFD, name, 0); err != nil { return nil, err } // Note that we don't need to set uid/gid for the new child. This is a hard diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index acfa9f19a..1531fe3a0 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -51,7 +51,7 @@ func (d *dentry) isDir() bool { // +checklocks:d.childrenMu func (d *dentry) cacheNewChildLocked(child *dentry, name string) { d.IncRef() // reference held by child on its parent - child.parent = d + child.parent.Store(d) child.name = name if d.children == nil { d.children = make(map[string]*dentry) diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 4a82efcea..d0d4f94a7 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -193,15 +193,15 @@ func (fs *filesystem) stepLocked(ctx context.Context, rp resolvingPath, d *dentr if name == ".." { if isRoot, err := rp.CheckRoot(ctx, &d.vfsd); err != nil { return nil, false, err - } else if isRoot || d.parent == nil { + } else if isRoot || d.parent.Load() == nil { rp.Advance() return d, false, nil } - if err := rp.CheckMount(ctx, &d.parent.vfsd); err != nil { + if err := rp.CheckMount(ctx, &d.parent.Load().vfsd); err != nil { return nil, false, err } rp.Advance() - return d.parent, false, nil + return d.parent.Load(), false, nil } child, err := fs.getChildAndWalkPathLocked(ctx, d, rp, ds) if err != nil { @@ -1591,7 +1591,7 @@ func (fs *filesystem) StatFSAt(ctx context.Context, rp *vfs.ResolvingPath) (linu } // If d is synthetic, invoke statfs on the first ancestor of d that isn't. for d.isSynthetic() { - d = d.parent + d = d.parent.Load() } statfs, err := d.statfs(ctx) if err != nil { @@ -1786,3 +1786,8 @@ func (fs *filesystem) MountOptions() string { } return strings.Join(opts, ",") } + +// IsDescendant implements vfs.FilesystemImpl.IsDescendant. +func (fs *filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool { + return genericIsDescendant(vfsroot.Dentry(), vd.Dentry().Impl().(*dentry)) +} diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 679019aee..7aa0120ad 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -44,6 +44,7 @@ import ( "path" "strconv" "strings" + "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" @@ -788,7 +789,7 @@ type dentry struct { // 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 + parent atomic.Pointer[dentry] `state:".(*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 @@ -1516,8 +1517,8 @@ func (d *dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, e d.fs.renameMu.RLock() // The ordering below is important, Linux always notifies the parent first. - if d.parent != nil { - d.parent.watches.Notify(ctx, d.name, events, cookie, et, d.isDeleted()) + if parent := d.parent.Load(); parent != nil { + parent.watches.Notify(ctx, d.name, events, cookie, et, d.isDeleted()) } d.watches.Notify(ctx, "", events, cookie, et, d.isDeleted()) d.fs.renameMu.RUnlock() @@ -1623,10 +1624,10 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo d.fs.renameMu.Lock() defer d.fs.renameMu.Unlock() } - if d.parent != nil { - d.parent.childrenMu.Lock() - delete(d.parent.children, d.name) - d.parent.childrenMu.Unlock() + if parent := d.parent.Load(); parent != nil { + parent.childrenMu.Lock() + delete(parent.children, d.name) + parent.childrenMu.Unlock() } d.destroyLocked(ctx) // +checklocksforce: see above. return @@ -1730,8 +1731,8 @@ func (d *dentry) evictLocked(ctx context.Context) { d.cachingMu.Unlock() return } - if d.parent != nil { - d.parent.opMu.Lock() + if parent := d.parent.Load(); parent != nil { + parent.opMu.Lock() if !d.vfsd.IsDead() { // Note that d can't be a mount point (in any mount namespace), since VFS // holds references on mount points. @@ -1740,15 +1741,15 @@ func (d *dentry) evictLocked(ctx context.Context) { rc.DecRef(ctx) } - d.parent.childrenMu.Lock() - delete(d.parent.children, d.name) - d.parent.childrenMu.Unlock() + parent.childrenMu.Lock() + delete(parent.children, d.name) + parent.childrenMu.Unlock() // We're only deleting the dentry, not the file it // represents, so we don't need to update // victim parent.dirents etc. } - d.parent.opMu.Unlock() + parent.opMu.Unlock() } // Safe to unlock cachingMu now that d.vfsd.IsDead(). Henceforth any // concurrent caching attempts on d will attempt to destroy it and so will @@ -1848,8 +1849,9 @@ func (d *dentry) destroyLocked(ctx context.Context) { // Drop the reference held by d on its parent without recursively locking // d.fs.renameMu. - if d.parent != nil && d.parent.decRefNoCaching() == 0 { - d.parent.checkCachingLocked(ctx, true /* renameMuWriteLocked */) + + if parent := d.parent.Load(); parent != nil && parent.decRefNoCaching() == 0 { + parent.checkCachingLocked(ctx, true /* renameMuWriteLocked */) } } diff --git a/pkg/sentry/fsimpl/gofer/revalidate.go b/pkg/sentry/fsimpl/gofer/revalidate.go index 69d80799b..0dea3bc3c 100644 --- a/pkg/sentry/fsimpl/gofer/revalidate.go +++ b/pkg/sentry/fsimpl/gofer/revalidate.go @@ -153,7 +153,7 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *d // can only be acquired from parent to child to avoid deadlocks. if isRoot, err := rp.CheckRoot(ctx, &d.vfsd); err != nil { return nil, errRevalidationStepDone{} - } else if isRoot || d.parent == nil { + } else if isRoot || d.parent.Load() == nil { rp.Advance() return d, errPartialRevalidation{} } @@ -164,11 +164,11 @@ func (fs *filesystem) revalidateStep(ctx context.Context, rp resolvingPath, d *d // // 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(ctx, &d.parent.vfsd); err != nil { + if err := rp.CheckMount(ctx, &d.parent.Load().vfsd); err != nil { return nil, errRevalidationStepDone{} } rp.Advance() - return d.parent, errPartialRevalidation{} + return d.parent.Load(), errPartialRevalidation{} default: d.childrenMu.Lock() @@ -212,10 +212,11 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem, // The dentry will be reloaded next time it's accessed. *ds = appendDentry(*ds, d) - d.parent.opMu.RLock() - defer d.parent.opMu.RUnlock() - d.parent.childrenMu.Lock() - defer d.parent.childrenMu.Unlock() + parent := d.parent.Load() + parent.opMu.RLock() + defer parent.opMu.RUnlock() + parent.childrenMu.Lock() + defer parent.childrenMu.Unlock() if d.isSynthetic() { // Normally we don't mark invalidated dentries as deleted since @@ -227,16 +228,16 @@ func (d *dentry) invalidate(ctx context.Context, vfsObj *vfs.VirtualFilesystem, d.decRefNoCaching() *ds = appendDentry(*ds, d) - d.parent.syntheticChildren-- - d.parent.clearDirentsLocked() + parent.syntheticChildren-- + parent.clearDirentsLocked() } // Since the opMu was just reacquired above, re-check that the // parent's child with this name is still the same. Do not touch it if // it has been replaced with a different one. - if child := d.parent.children[d.name]; child == d { + if child := parent.children[d.name]; child == d { // Invalidate dentry so it gets reloaded next time it's accessed. - delete(d.parent.children, d.name) + delete(parent.children, d.name) } } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index bf91ad442..1ae976d4a 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -168,6 +168,16 @@ func (fd *specialFileFD) afterLoad() { } } +// saveParent is called by stateify. +func (d *dentry) saveParent() *dentry { + return d.parent.Load() +} + +// loadParent is called by stateify. +func (d *dentry) loadParent(parent *dentry) { + d.parent.Store(parent) +} + // CompleteRestore implements // vfs.FilesystemImplSaveRestoreExtension.CompleteRestore. func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRestoreOptions) error { diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 4b3e67a82..622a85ece 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -58,15 +58,15 @@ func (fs *Filesystem) stepExistingLocked(ctx context.Context, rp *vfs.ResolvingP if name == ".." { if isRoot, err := rp.CheckRoot(ctx, d.VFSDentry()); err != nil { return nil, false, err - } else if isRoot || d.parent == nil { + } else if isRoot || d.parent.Load() == nil { rp.Advance() return d, false, nil } - if err := rp.CheckMount(ctx, d.parent.VFSDentry()); err != nil { + if err := rp.CheckMount(ctx, d.Parent().VFSDentry()); err != nil { return nil, false, err } rp.Advance() - return d.parent, false, nil + return d.parent.Load(), false, nil } if len(name) > linux.NAME_MAX { return nil, false, linuxerr.ENAMETOOLONG @@ -231,7 +231,7 @@ func checkCreateLocked(ctx context.Context, creds *auth.Credentials, name string // // Preconditions: Filesystem.mu must be locked for at least reading. func checkDeleteLocked(ctx context.Context, rp *vfs.ResolvingPath, d *Dentry) error { - parent := d.parent + parent := d.parent.Load() if parent == nil { return linuxerr.EBUSY } @@ -772,7 +772,7 @@ func (fs *Filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa fs.deferDecRef(srcDir) // child (src) drops ref on old parent. dstDir.IncRef() // child (src) takes a ref on the new parent. } - src.parent = dstDir + src.parent.Store(dstDir) src.name = newName if dstDir.children == nil { dstDir.children = make(map[string]*Dentry) @@ -971,7 +971,7 @@ func (fs *Filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error return linuxerr.EISDIR } virtfs := rp.VirtualFilesystem() - parentDentry := d.parent + parentDentry := d.parent.Load() parentDentry.dirMu.Lock() defer parentDentry.dirMu.Unlock() mntns := vfs.MountNamespaceFromContext(ctx) @@ -1082,3 +1082,8 @@ func (fs *Filesystem) deferDecRefVD(ctx context.Context, vd vfs.VirtualDentry) { vd.DecRef(ctx) } } + +// IsDescendant implements vfs.FilesystemImpl.IsDescendant. +func (fs *Filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool { + return genericIsDescendant(vfsroot.Dentry(), vd.Dentry().Impl().(*Dentry)) +} diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index a54015a47..8cb2e11ed 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -60,6 +60,7 @@ package kernfs import ( "fmt" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -226,8 +227,9 @@ type Dentry struct { // dflags* consts above. flags atomicbitops.Uint32 - parent *Dentry - name string + parent atomic.Pointer[Dentry] `state:".(*Dentry)"` + + name string // If cached is true, dentryEntry links dentry into // Filesystem.cachedDentries. cached and dentryEntry are protected by @@ -342,7 +344,7 @@ func (d *Dentry) cacheLocked(ctx context.Context) { // because it has zero references. // Note that a dentry may not always have a parent; for example magic links // as described in Inode.Getlink. - if isDead := d.VFSDentry().IsDead(); isDead || d.parent == nil { + if isDead, parent := d.VFSDentry().IsDead(), d.parent.Load(); isDead || parent == nil { if !isDead { rcs := d.fs.vfsfs.VirtualFilesystem().InvalidateDentry(ctx, d.VFSDentry()) for _, rc := range rcs { @@ -358,8 +360,8 @@ func (d *Dentry) cacheLocked(ctx context.Context) { d.inode.Watches().HandleDeletion(ctx) } d.destroy(ctx) - if d.parent != nil { - d.parent.decRefLocked(ctx) + if parent != nil { + parent.decRefLocked(ctx) } return } @@ -409,19 +411,20 @@ func (d *Dentry) evictLocked(ctx context.Context) { // after it was inserted into fs.cachedDentries. if d.refs.Load() == 0 { if !d.vfsd.IsDead() { - d.parent.dirMu.Lock() + parent := d.parent.Load() + parent.dirMu.Lock() // Note that victim can't be a mount point (in any mount // namespace), since VFS holds references on mount points. rcs := d.fs.vfsfs.VirtualFilesystem().InvalidateDentry(ctx, d.VFSDentry()) for _, rc := range rcs { d.fs.deferDecRef(rc) } - delete(d.parent.children, d.name) - d.parent.dirMu.Unlock() + delete(parent.children, d.name) + parent.dirMu.Unlock() } d.destroy(ctx) - if d.parent != nil { - d.parent.decRefLocked(ctx) + if parent := d.parent.Load(); parent != nil { + parent.decRefLocked(ctx) } } } @@ -535,8 +538,8 @@ func (d *Dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, e // won't have one. if !d.inode.Anonymous() { d.fs.mu.RLock() - if d.parent != nil { - d.parent.inode.Watches().Notify(ctx, d.name, events, cookie, et, d.isDeleted()) + if parent := d.parent.Load(); parent != nil { + parent.inode.Watches().Notify(ctx, d.name, events, cookie, et, d.isDeleted()) } d.fs.mu.RUnlock() } @@ -577,7 +580,7 @@ func (d *Dentry) insertChildLocked(name string, child *Dentry) { panic(fmt.Sprintf("insertChildLocked called on non-directory Dentry: %+v.", d)) } d.IncRef() // DecRef in child's Dentry.destroy. - child.parent = d + child.parent.Store(d) child.name = name if d.children == nil { d.children = make(map[string]*Dentry) @@ -632,7 +635,7 @@ func (d *Dentry) WalkDentryTree(ctx context.Context, vfsObj *vfs.VirtualFilesyst // Don't let .. traverse above the start point of the walk. continue } - target = target.parent + target = target.parent.Load() // Parent doesn't need revalidation since we revalidated it on the // way to the child, and we're still holding fs.mu. default: @@ -660,7 +663,7 @@ func (d *Dentry) WalkDentryTree(ctx context.Context, vfsObj *vfs.VirtualFilesyst // filesystem may concurrently move d elsewhere. The caller is responsible for // ensuring the returned result remains valid while it is used. func (d *Dentry) Parent() *Dentry { - return d.parent + return d.parent.Load() } // The Inode interface maps filesystem-level operations that operate on paths to diff --git a/pkg/sentry/fsimpl/kernfs/save_restore.go b/pkg/sentry/fsimpl/kernfs/save_restore.go index 1b2245955..9d4130165 100644 --- a/pkg/sentry/fsimpl/kernfs/save_restore.go +++ b/pkg/sentry/fsimpl/kernfs/save_restore.go @@ -32,3 +32,13 @@ func (i *inodePlatformFile) afterLoad() { i.fileMapperInitOnce.Do(func() {}) } } + +// saveParent is called by stateify. +func (d *Dentry) saveParent() *Dentry { + return d.parent.Load() +} + +// loadParent is called by stateify. +func (d *Dentry) loadParent(parent *Dentry) { + d.parent.Store(parent) +} diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go index e6c12e0c3..6e0f45d30 100644 --- a/pkg/sentry/fsimpl/overlay/copy_up.go +++ b/pkg/sentry/fsimpl/overlay/copy_up.go @@ -65,11 +65,12 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } // Ensure that our parent directory is copied-up. - if d.parent == nil { + parent := d.parent.Load() + if parent == nil { // d is a filesystem root with no upper layer. return linuxerr.EROFS } - if err := d.parent.copyUpMaybeSyntheticMountpointLocked(ctx, forSyntheticMountpoint); err != nil { + if err := parent.copyUpMaybeSyntheticMountpointLocked(ctx, forSyntheticMountpoint); err != nil { return err } @@ -101,8 +102,8 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy // Perform copy-up. ftype := d.mode.Load() & linux.S_IFMT newpop := vfs.PathOperation{ - Root: d.parent.upperVD, - Start: d.parent.upperVD, + Root: parent.upperVD, + Start: parent.upperVD, Path: fspath.Parse(d.name), } // Used during copy-up of memory-mapped regular files. diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 493515dd4..aa440b40a 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -151,15 +151,16 @@ func (fs *filesystem) stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d * if name == ".." { if isRoot, err := rp.CheckRoot(ctx, &d.vfsd); err != nil { return nil, lookupLayerNone, false, err - } else if isRoot || d.parent == nil { + } else if isRoot || d.parent.Load() == nil { rp.Advance() return d, d.topLookupLayer(), false, nil } - if err := rp.CheckMount(ctx, &d.parent.vfsd); err != nil { + if err := rp.CheckMount(ctx, &d.parent.Load().vfsd); err != nil { return nil, lookupLayerNone, false, err } rp.Advance() - return d.parent, d.parent.topLookupLayer(), false, nil + parent := d.parent.Load() + return parent, parent.topLookupLayer(), false, nil } if uint64(len(name)) > fs.maxFilenameLen { return nil, lookupLayerNone, false, linuxerr.ENAMETOOLONG @@ -344,7 +345,7 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str } parent.IncRef() - child.parent = parent + child.parent.Store(parent) child.name = name return child, topLookupLayer, nil } @@ -1335,7 +1336,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa oldParent.DecRef(ctx) ds = appendDentry(ds, oldParent) newParent.IncRef() - renamed.parent = newParent + renamed.parent.Store(newParent) } renamed.name = newName if newParent.children == nil { diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index fed5c13a2..d4a0a787e 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -36,6 +36,7 @@ package overlay import ( "fmt" "strings" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -473,6 +474,11 @@ func (fs *filesystem) getLowerDevMinor(layerMajor, layerMinor uint32) (uint32, e return minor, nil } +// IsDescendant implements vfs.FilesystemImpl.IsDescendant. +func (fs *filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool { + return genericIsDescendant(vfsroot.Dentry(), vd.Dentry().Impl().(*dentry)) +} + // dentry implements vfs.DentryImpl. // // +stateify savable @@ -500,7 +506,7 @@ type dentry struct { // name is this dentry's name in parent. If this dentry is a filesystem // root, parent is nil and name is the empty string. parent and name are // protected by fs.renameMu. - parent *dentry + parent atomic.Pointer[dentry] `state:".(*dentry)"` name string // If this dentry represents a directory, children maps the names of @@ -693,15 +699,15 @@ func (d *dentry) destroyLocked(ctx context.Context) { d.watches.HandleDeletion(ctx) - if d.parent != nil { - d.parent.dirMu.Lock() + if parent := d.parent.Load(); parent != nil { + parent.dirMu.Lock() if !d.vfsd.IsDead() { - delete(d.parent.children, d.name) + delete(parent.children, d.name) } - d.parent.dirMu.Unlock() + parent.dirMu.Unlock() // Drop the reference held by d on its parent without recursively // locking d.fs.renameMu. - d.parent.decRefLocked(ctx) + parent.decRefLocked(ctx) } refs.Unregister(d) } @@ -736,8 +742,8 @@ func (d *dentry) InotifyWithParent(ctx context.Context, events uint32, cookie ui d.fs.renameMu.RLock() // The ordering below is important, Linux always notifies the parent first. - if d.parent != nil { - d.parent.watches.Notify(ctx, d.name, events, cookie, et, deleted) + if parent := d.parent.Load(); parent != nil { + parent.watches.Notify(ctx, d.name, events, cookie, et, deleted) } d.watches.Notify(ctx, "", events, cookie, et, deleted) d.fs.renameMu.RUnlock() diff --git a/pkg/sentry/fsimpl/overlay/save_restore.go b/pkg/sentry/fsimpl/overlay/save_restore.go index 94fb439e6..fe78f153a 100644 --- a/pkg/sentry/fsimpl/overlay/save_restore.go +++ b/pkg/sentry/fsimpl/overlay/save_restore.go @@ -23,3 +23,13 @@ func (d *dentry) afterLoad() { refs.Register(d) } } + +// saveParent is called by stateify. +func (d *dentry) saveParent() *dentry { + return d.parent.Load() +} + +// loadParent is called by stateify. +func (d *dentry) loadParent(parent *dentry) { + d.parent.Store(parent) +} diff --git a/pkg/sentry/fsimpl/tmpfs/directory.go b/pkg/sentry/fsimpl/tmpfs/directory.go index 1383a1043..c2ae5ebda 100644 --- a/pkg/sentry/fsimpl/tmpfs/directory.go +++ b/pkg/sentry/fsimpl/tmpfs/directory.go @@ -60,7 +60,7 @@ func (fs *filesystem) newDirectory(kuid auth.KUID, kgid auth.KGID, mode linux.Fi // - filesystem.mu must be locked for writing. // - dir must not already contain a child with the given name. func (dir *directory) insertChildLocked(child *dentry, name string) { - child.parent = &dir.dentry + child.parent.Store(&dir.dentry) child.name = name if dir.childMap == nil { dir.childMap = make(map[string]*dentry) diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index b2d7aaa12..6958412b5 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -68,15 +68,15 @@ func stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d *dentry) (*dentry, if name == ".." { if isRoot, err := rp.CheckRoot(ctx, &d.vfsd); err != nil { return nil, false, err - } else if isRoot || d.parent == nil { + } else if isRoot || d.parent.Load() == nil { rp.Advance() return d, false, nil } - if err := rp.CheckMount(ctx, &d.parent.vfsd); err != nil { + if err := rp.CheckMount(ctx, &d.parent.Load().vfsd); err != nil { return nil, false, err } rp.Advance() - return d.parent, false, nil + return d.parent.Load(), false, nil } if len(name) > d.inode.fs.maxFilenameLen { return nil, false, linuxerr.ENAMETOOLONG @@ -947,7 +947,8 @@ func (fs *filesystem) PrependPath(ctx context.Context, vfsroot, vd vfs.VirtualDe if mnt != nil && &d.vfsd == mnt.Root() { return nil } - if d.parent == nil { + parent := d.parent.Load() + if parent == nil { if d.name != "" { // This file must have been created by // newUnlinkedRegularFileDescription(). In Linux, @@ -964,7 +965,7 @@ func (fs *filesystem) PrependPath(ctx context.Context, vfsroot, vd vfs.VirtualDe return vfs.PrependPathAtNonMountRootError{} } b.PrependComponent(d.name) - d = d.parent + d = parent } } @@ -973,6 +974,11 @@ func (fs *filesystem) MountOptions() string { return fs.mopts } +// IsDescendant implements vfs.FilesystemImpl.IsDescendant. +func (fs *filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool { + return genericIsDescendant(vfsroot.Dentry(), vd.Dentry().Impl().(*dentry)) +} + // adjustPageAcct adjusts the accounting done against filesystem size limit in // case there is any discrepency between the number of pages reserved vs the // number of pages actually allocated. diff --git a/pkg/sentry/fsimpl/tmpfs/save_restore.go b/pkg/sentry/fsimpl/tmpfs/save_restore.go index d08975d82..b6e8f033f 100644 --- a/pkg/sentry/fsimpl/tmpfs/save_restore.go +++ b/pkg/sentry/fsimpl/tmpfs/save_restore.go @@ -22,3 +22,13 @@ func (fs *filesystem) afterLoad() { } fs.mf = fs.mfp.MemoryFile() } + +// saveParent is called by stateify. +func (d *dentry) saveParent() *dentry { + return d.parent.Load() +} + +// saveParent is called by stateify. +func (d *dentry) loadParent(parent *dentry) { + d.parent.Store(parent) +} diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index f534b7e96..29ce64050 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -33,6 +33,7 @@ import ( "math" "strconv" "strings" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -411,7 +412,7 @@ type dentry struct { // parent is this dentry's parent directory. Each referenced dentry holds a // reference on parent.dentry. If this dentry is a filesystem root, parent // is nil. parent is protected by filesystem.mu. - parent *dentry + parent atomic.Pointer[dentry] `state:".(*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 @@ -468,8 +469,9 @@ func (d *dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, e d.inode.fs.mu.RLock() // The ordering below is important, Linux always notifies the parent first. - if d.parent != nil { - d.parent.inode.watches.Notify(ctx, d.name, events, cookie, et, deleted) + parent := d.parent.Load() + if parent != nil { + parent.inode.watches.Notify(ctx, d.name, events, cookie, et, deleted) } d.inode.watches.Notify(ctx, "", events, cookie, et, deleted) d.inode.fs.mu.RUnlock() diff --git a/pkg/sentry/vfs/anonfs.go b/pkg/sentry/vfs/anonfs.go index e8e7e2e0b..187770b2a 100644 --- a/pkg/sentry/vfs/anonfs.go +++ b/pkg/sentry/vfs/anonfs.go @@ -301,6 +301,9 @@ func (fs *anonFilesystem) MountOptions() string { return "" } +// IsDescendant implements FilesystemImpl.IsDescendant. +func (fs *anonFilesystem) IsDescendant(vfsroot, vd VirtualDentry) bool { return vfsroot == vd } + // IncRef implements DentryImpl.IncRef. func (d *anonDentry) IncRef() { // no-op diff --git a/pkg/sentry/vfs/filesystem.go b/pkg/sentry/vfs/filesystem.go index 252c691c3..f2647ea1c 100644 --- a/pkg/sentry/vfs/filesystem.go +++ b/pkg/sentry/vfs/filesystem.go @@ -511,6 +511,15 @@ type FilesystemImpl interface { // If the implementation has no filesystem-specific options, it should // return the empty string. MountOptions() string + + // IsDescendant returns true if vd is a descendant of vfsroot or if vd and + // vfsroot are the same dentry. The method does not take filesystem locks when + // accessing the parents of each dentry, so it's possible for parents to be + // mutated concurrently during a call to IsDescendant. Callers should take + // appropriate caution when using this method. + // + // Preconditions: vd.Mount().Filesystem().Impl() == this FilesystemImpl. + IsDescendant(vfsroot, vd VirtualDentry) bool } // PrependPathAtVFSRootError is returned by implementations of diff --git a/pkg/sentry/vfs/genericfstree/genericfstree.go b/pkg/sentry/vfs/genericfstree/genericfstree.go index ba6e6ed49..b4dd7c088 100644 --- a/pkg/sentry/vfs/genericfstree/genericfstree.go +++ b/pkg/sentry/vfs/genericfstree/genericfstree.go @@ -26,6 +26,13 @@ import ( "gvisor.dev/gvisor/pkg/sentry/vfs" ) +// We need to define an interface instead of using atomic.Pointer because +// the Dentry type gets removed during code generation and the compiler +// complains about the unused sync/atomic type. +type atomicptr interface { + Load() *Dentry +} + // Dentry is a required type parameter that is a struct with the given fields. // // +stateify savable @@ -35,7 +42,7 @@ type Dentry struct { // parent is the parent of this Dentry in the filesystem's tree. If this // Dentry is a filesystem root, parent is nil. - parent *Dentry + parent atomicptr // name is the name of this Dentry in its parent. If this Dentry is a // filesystem root, name is unspecified. @@ -46,21 +53,31 @@ type Dentry struct { // either d2's parent or an ancestor of d2's parent. func IsAncestorDentry(d, d2 *Dentry) bool { for d2 != nil { // Stop at root, where d2.parent == nil. - if d2.parent == d { + parent := d2.parent.Load() + if parent == d { return true } - if d2.parent == d2 { + if parent == d2 { return false } - d2 = d2.parent + d2 = parent } return false } +// IsDescendant returns true if vd is a descendant of vfsroot or if vd and +// vfsroot are the same dentry. +func IsDescendant(vfsroot *vfs.Dentry, d *Dentry) bool { + for d != nil && &d.vfsd != vfsroot { + d = d.parent.Load() + } + return d != nil +} + // ParentOrSelf returns d.parent. If d.parent is nil, ParentOrSelf returns d. func ParentOrSelf(d *Dentry) *Dentry { - if d.parent != nil { - return d.parent + if parent := d.parent.Load(); parent != nil { + return parent } return d } @@ -74,11 +91,12 @@ func PrependPath(vfsroot vfs.VirtualDentry, mnt *vfs.Mount, d *Dentry, b *fspath if mnt != nil && &d.vfsd == mnt.Root() { return nil } - if d.parent == nil { + parent := d.parent.Load() + if parent == nil { return vfs.PrependPathAtNonMountRootError{} } b.PrependComponent(d.name) - d = d.parent + d = parent } } diff --git a/pkg/sentry/vfs/vfs.go b/pkg/sentry/vfs/vfs.go index a457a9132..219e3886c 100644 --- a/pkg/sentry/vfs/vfs.go +++ b/pkg/sentry/vfs/vfs.go @@ -18,7 +18,7 @@ // // EpollInstance.interestMu // FileDescription.epollMu -// Locks acquired by FilesystemImpl/FileDescriptionImpl methods +// Locks acquired by FilesystemImpl/FileDescriptionImpl methods (except IsDescendant) // VirtualFilesystem.mountMu // Dentry.mu // Locks acquired by FilesystemImpls between Prepare{Delete,Rename}Dentry and Commit{Delete,Rename*}Dentry @@ -33,6 +33,11 @@ // Locking Dentry.mu in multiple Dentries requires holding // VirtualFilesystem.mountMu. Locking EpollInstance.interestMu in multiple // EpollInstances requires holding epollCycleMu. +// +// FilesystemImpl locks are not held during calls to FilesystemImpl.IsDescendant +// since it's called under mountMu. It's possible for concurrent mutation +// to dentry ancestors during calls IsDescendant. Callers should take +// appropriate caution when using this method. package vfs import (