Skip to content

Commit

Permalink
proc: invalidate task inodes when a task is destroyed
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 699233555
  • Loading branch information
avagin authored and gvisor-bot committed Nov 23, 2024
1 parent c6c3c2d commit 04c4b9f
Show file tree
Hide file tree
Showing 30 changed files with 220 additions and 45 deletions.
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/cgroupfs/cgroupfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ type dir struct {
kernfs.InodeNotSymlink
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned
implStatFS

locks vfs.FileLocks
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/devpts/devpts.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ type rootInode struct {
kernfs.InodeTemporary // This holds no meaning as this inode can't be Looked up and is always valid.
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned
rootInodeRefs

locks vfs.FileLocks
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/devpts/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type masterInode struct {
kernfs.InodeNotAnonymous
kernfs.InodeNotDirectory
kernfs.InodeNotSymlink
kernfs.InodeFSOwned
kernfs.InodeWatches

locks vfs.FileLocks
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/devpts/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type replicaInode struct {
kernfs.InodeNotDirectory
kernfs.InodeNotSymlink
kernfs.InodeWatches
kernfs.InodeFSOwned

locks vfs.FileLocks

Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/fuse/inode.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type inode struct {
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.CachedMappable
kernfs.InodeFSOwned

// the owning filesystem. fs is immutable.
fs *filesystem
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ type inode struct {
kernfs.InodeNotSymlink
kernfs.InodeTemporary // This holds no meaning as this inode can't be Looked up and is always valid.
kernfs.InodeWatches
kernfs.InodeFSOwned

locks vfs.FileLocks

Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type DynamicBytesFile struct {
InodeNotDirectory
InodeNotSymlink
InodeWatches
InodeFSOwned

locks vfs.FileLocks
// data can additionally implement vfs.WritableDynamicBytesSource to support
Expand Down
43 changes: 22 additions & 21 deletions pkg/sentry/fsimpl/kernfs/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,27 @@ func (fs *Filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir
parent.dirMu.Lock()
defer parent.dirMu.Unlock() // may be temporarily unlocked and re-locked below
child := parent.children[name]
for child != nil {
for {
if child == nil {
// Dentry isn't cached; it either doesn't exist or failed revalidation.
// Attempt to resolve it via Lookup.
childInode, err := parent.inode.Lookup(ctx, name)
if err != nil {
return nil, err
}
var newChild Dentry
newChild.Init(fs, childInode) // childInode's ref is transferred to newChild.
parent.insertChildLocked(name, &newChild)
child = &newChild

// Drop the ref on newChild. This will cause the dentry to get pruned
// from the dentry tree by the end of current filesystem operation
// (before returning to the VFS layer) if another ref is not picked on
// this dentry.
if !childInode.Keep() {
fs.deferDecRef(&newChild)
}
}
// Cached dentry exists, revalidate.
if child.inode.Valid(ctx, parent, name) {
break
Expand All @@ -120,26 +140,7 @@ func (fs *Filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir
parent.dirMu.Lock()
// Check for concurrent insertion of a new cached dentry.
child = parent.children[name]
}
if child == nil {
// Dentry isn't cached; it either doesn't exist or failed revalidation.
// Attempt to resolve it via Lookup.
childInode, err := parent.inode.Lookup(ctx, name)
if err != nil {
return nil, err
}
var newChild Dentry
newChild.Init(fs, childInode) // childInode's ref is transferred to newChild.
parent.insertChildLocked(name, &newChild)
child = &newChild

// Drop the ref on newChild. This will cause the dentry to get pruned
// from the dentry tree by the end of current filesystem operation
// (before returning to the VFS layer) if another ref is not picked on
// this dentry.
if !childInode.Keep() {
fs.deferDecRef(&newChild)
}

}
return child, nil
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/sentry/fsimpl/kernfs/inode_impl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ type StaticDirectory struct {
InodeWatches
OrderedChildren
StaticDirectoryRefs
InodeFSOwned

locks vfs.FileLocks
fdOpts GenericDirectoryFDOptions
Expand Down Expand Up @@ -845,3 +846,15 @@ type InodeNotAnonymous struct{}
func (*InodeNotAnonymous) Anonymous() bool {
return false
}

// InodeFSOwned represents inodes whose lifecycle is entirely managed by the
// filesystem.
//
// +stateify savable
type InodeFSOwned struct{}

// AddInvalidateCallback implements Inode.AddInvalidateCallback.
func (*InodeFSOwned) AddInvalidateCallback(d *Dentry) {}

// RemoveInvalidateCallback implements Remove.AddInvalidateCallback.
func (*InodeFSOwned) RemoveInvalidateCallback(d *Dentry) {}
50 changes: 50 additions & 0 deletions pkg/sentry/fsimpl/kernfs/kernfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,42 @@ func (d *Dentry) decRefLocked(ctx context.Context) {
}
}

// Invalidate invalidates the dentry and its children.
func (d *Dentry) Invalidate(ctx context.Context) {
d.fs.mu.Lock()
defer d.fs.mu.Unlock()
if d.vfsd.IsDead() {
return
}
parent := d.parent.Load()
if parent == nil {
return
}
parent.dirMu.Lock()
if parent.vfsd.IsDead() {
parent.dirMu.Unlock()
return
}
child := parent.children[d.name]
if child != d {
parent.dirMu.Unlock()
return
}
delete(parent.children, d.name)
parent.dirMu.Unlock()

children := []*Dentry{d}
for i := 0; i < len(children); i++ {
for _, c := range children[i].children {
children = append(children, c)
}
}
d.fs.invalidateRemovedChildLocked(ctx, d.fs.vfsfs.VirtualFilesystem(), child)
for _, c := range children {
c.evictLocked(ctx)
}
}

// cacheLocked should be called after d's reference count becomes 0. The ref
// count check may happen before acquiring d.fs.mu so there might be a race
// condition where the ref count is increased again by the time the caller
Expand Down Expand Up @@ -392,6 +428,15 @@ func (d *Dentry) cacheLocked(ctx context.Context) {
// back down to fs.opts.maxCachedDentries, so we don't loop.
}

// DropDentryCache cleans up the dentry cache.
func (fs *Filesystem) DropDentryCache(ctx context.Context) {
fs.mu.Lock()
defer fs.mu.Unlock()
for !fs.cachedDentries.Empty() {
fs.cachedDentries.Back().evictLocked(ctx)
}
}

// Preconditions:
// - fs.mu must be locked for writing.
func (fs *Filesystem) evictCachedDentryLocked(ctx context.Context) {
Expand Down Expand Up @@ -451,6 +496,7 @@ func (d *Dentry) destroy(ctx context.Context) {
panic("dentry.destroy() called with references on the dentry")
}

d.inode.RemoveInvalidateCallback(d)
d.inode.DecRef(ctx) // IncRef from Init.

refs.Unregister(d)
Expand Down Expand Up @@ -505,6 +551,7 @@ func (d *Dentry) Init(fs *Filesystem, inode Inode) {
d.flags = atomicbitops.FromUint32(d.flags.RacyLoad() | dflagsIsSymlink)
}
refs.Register(d)
inode.AddInvalidateCallback(d)
}

// VFSDentry returns the generic vfs dentry for this kernfs dentry.
Expand Down Expand Up @@ -732,6 +779,9 @@ type Inode interface {
// Anonymous indicates that the Inode is anonymous. It will never have
// a name or parent.
Anonymous() bool

AddInvalidateCallback(d *Dentry)
RemoveInvalidateCallback(d *Dentry)
}

type inodeRefs interface {
Expand Down
2 changes: 2 additions & 0 deletions pkg/sentry/fsimpl/kernfs/kernfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ type readonlyDir struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned

locks vfs.FileLocks
}
Expand Down Expand Up @@ -146,6 +147,7 @@ type dir struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned

locks vfs.FileLocks

Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/kernfs/symlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type StaticSymlink struct {
InodeSymlink
InodeNoStatFS
InodeWatches
InodeFSOwned

target string
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/kernfs/synthetic_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type syntheticDirectory struct {
InodeWatches
OrderedChildren
syntheticDirectoryRefs
InodeFSOwned

locks vfs.FileLocks
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/mqfs/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type rootInode struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned

locks vfs.FileLocks
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/nsfs/nsfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type Inode struct {
kernfs.InodeNotDirectory
kernfs.InodeNotSymlink
kernfs.InodeWatches
kernfs.InodeFSOwned
inodeRefs

locks vfs.FileLocks
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/pipefs/pipefs.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ type inode struct {
kernfs.InodeNotSymlink
kernfs.InodeNoopRefCount
kernfs.InodeWatches
kernfs.InodeFSOwned

locks vfs.FileLocks
pipe *pipe.VFSPipe
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/proc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ go_library(
"fd_dir_inode_refs.go",
"fd_info_dir_inode_refs.go",
"filesystem.go",
"filesystem_state.go",
"proc_impl.go",
"subtasks.go",
"subtasks_inode_refs.go",
Expand Down
23 changes: 23 additions & 0 deletions pkg/sentry/fsimpl/proc/filesystem_state.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2024 The gVisor Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package proc

import (
"gvisor.dev/gvisor/pkg/context"
)

func (fs *filesystem) beforeSave() {
fs.DropDentryCache(context.Background())
}
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/proc/subtasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type subtasksInode struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned
subtasksInodeRefs

locks vfs.FileLocks
Expand Down
10 changes: 10 additions & 0 deletions pkg/sentry/fsimpl/proc/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,16 @@ func (fs *filesystem) newTaskInode(ctx context.Context, task *kernel.Task, pidns
return inode, nil
}

// AddInvalidateCallback implements kernfs.Inode.AddInvalidateCallback.
func (i *taskInode) AddInvalidateCallback(d *kernfs.Dentry) {
i.task.RegisterOnDestroyAction(d, d.Invalidate)
}

// RemoveInvalidateCallback implements kernfs.Inode.AddInvalidateCallback.
func (i *taskInode) RemoveInvalidateCallback(d *kernfs.Dentry) {
i.task.UnregisterOnDestroyAction(d)
}

// Valid implements kernfs.Inode.Valid. This inode remains valid as long
// as the task is still running. When it's dead, another tasks with the same
// PID could replace it.
Expand Down
3 changes: 3 additions & 0 deletions pkg/sentry/fsimpl/proc/task_fds.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ type fdDirInode struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned
}

var _ kernfs.Inode = (*fdDirInode)(nil)
Expand Down Expand Up @@ -202,6 +203,7 @@ type fdSymlink struct {
kernfs.InodeNotAnonymous
kernfs.InodeSymlink
kernfs.InodeWatches
kernfs.InodeFSOwned

fs *filesystem
task *kernel.Task
Expand Down Expand Up @@ -264,6 +266,7 @@ type fdInfoDirInode struct {
kernfs.InodeTemporary
kernfs.InodeWatches
kernfs.OrderedChildren
kernfs.InodeFSOwned
}

var _ kernfs.Inode = (*fdInfoDirInode)(nil)
Expand Down
Loading

0 comments on commit 04c4b9f

Please sign in to comment.