diff options
author | Dean Deng <deandeng@google.com> | 2020-11-09 08:30:29 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-11-09 08:33:17 -0800 |
commit | 0fb5353e45f166460d5846576c20479072207a06 (patch) | |
tree | f88e4472c07f56e4d34b14743e1a3afaafab62e1 /pkg/sentry/fsimpl | |
parent | 78cce3a46b953cab00731f8afacf7e9e7f4dc751 (diff) |
Initialize references with a value of 1.
This lets us avoid treating a value of 0 as one reference. All references
using the refsvfs2 template must call InitRefs() before the reference is
incremented/decremented, or else a panic will occur. Therefore, it should be
pretty easy to identify missing InitRef calls during testing.
Updates #1486.
PiperOrigin-RevId: 341411151
Diffstat (limited to 'pkg/sentry/fsimpl')
-rw-r--r-- | pkg/sentry/fsimpl/devpts/devpts.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/fuse/fusefs.go | 8 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/fuse/utils_test.go | 1 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/gofer/gofer.go | 12 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/host/host.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/host/socket.go | 6 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/kernfs/inode_impl_util.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/kernfs/kernfs.go | 16 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/kernfs/kernfs_test.go | 4 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/overlay/overlay.go | 16 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/proc/subtasks.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/proc/task.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/proc/task_fds.go | 4 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/proc/tasks.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/sys/sys.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/tmpfs/tmpfs.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/verity/verity.go | 16 |
17 files changed, 63 insertions, 36 deletions
diff --git a/pkg/sentry/fsimpl/devpts/devpts.go b/pkg/sentry/fsimpl/devpts/devpts.go index 346cca558..d8c237753 100644 --- a/pkg/sentry/fsimpl/devpts/devpts.go +++ b/pkg/sentry/fsimpl/devpts/devpts.go @@ -110,7 +110,7 @@ func (fstype *FilesystemType) newFilesystem(ctx context.Context, vfsObj *vfs.Vir } root.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, devMinor, 1, linux.ModeDirectory|0555) root.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - root.EnableLeakCheck() + root.InitRefs() var rootD kernfs.Dentry rootD.InitRoot(&fs.Filesystem, root) diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 6de416da0..cd0eb56e5 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -219,16 +219,12 @@ func newFUSEFilesystem(ctx context.Context, devMinor uint32, opts *filesystemOpt } fuseFD := device.Impl().(*DeviceFD) - fs := &filesystem{ devMinor: devMinor, opts: opts, conn: conn, } - - fs.VFSFilesystem().IncRef() fuseFD.fs = fs - return fs, nil } @@ -288,7 +284,7 @@ func (fs *filesystem) newRoot(ctx context.Context, creds *auth.Credentials, mode i := &inode{fs: fs, nodeID: 1} i.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, fs.devMinor, 1, linux.ModeDirectory|0755) i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - i.EnableLeakCheck() + i.InitRefs() var d kernfs.Dentry d.InitRoot(&fs.Filesystem, i) @@ -301,7 +297,7 @@ func (fs *filesystem) newInode(ctx context.Context, nodeID uint64, attr linux.FU i.InodeAttrs.Init(ctx, &creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.FileMode(attr.Mode)) atomic.StoreUint64(&i.size, attr.Size) i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - i.EnableLeakCheck() + i.InitRefs() return i } diff --git a/pkg/sentry/fsimpl/fuse/utils_test.go b/pkg/sentry/fsimpl/fuse/utils_test.go index e1d9e3365..b2f4276b8 100644 --- a/pkg/sentry/fsimpl/fuse/utils_test.go +++ b/pkg/sentry/fsimpl/fuse/utils_test.go @@ -72,6 +72,7 @@ func newTestConnection(system *testutil.System, k *kernel.Kernel, maxActiveReque if err != nil { return nil, nil, err } + fs.VFSFilesystem().Init(vfsObj, nil, fs) return fs.conn, &fuseDev.vfsfd, nil } diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index b59f62652..53bcc9986 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -1236,7 +1236,9 @@ func (d *dentry) IncRef() { // d.refs may be 0 if d.fs.renameMu is locked, which serializes against // d.checkCachingLocked(). r := atomic.AddInt64(&d.refs, 1) - refsvfs2.LogIncRef(d, r) + if d.LogRefs() { + refsvfs2.LogIncRef(d, r) + } } // TryIncRef implements vfs.DentryImpl.TryIncRef. @@ -1247,7 +1249,9 @@ func (d *dentry) TryIncRef() bool { return false } if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { - refsvfs2.LogTryIncRef(d, r+1) + if d.LogRefs() { + refsvfs2.LogTryIncRef(d, r+1) + } return true } } @@ -1267,7 +1271,9 @@ func (d *dentry) DecRef(ctx context.Context) { // responsible for ensuring that d.checkCachingLocked will be called later. func (d *dentry) decRefNoCaching() int64 { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r < 0 { panic("gofer.dentry.decRefNoCaching() called without holding a reference") } diff --git a/pkg/sentry/fsimpl/host/host.go b/pkg/sentry/fsimpl/host/host.go index 39b902a3e..435a21d77 100644 --- a/pkg/sentry/fsimpl/host/host.go +++ b/pkg/sentry/fsimpl/host/host.go @@ -126,8 +126,8 @@ func newInode(ctx context.Context, fs *filesystem, hostFD int, savable bool, fil isTTY: isTTY, savable: savable, } + i.InitRefs() i.CachedMappable.Init(hostFD) - i.EnableLeakCheck() // If the hostFD can return EWOULDBLOCK when set to non-blocking, do so and // handle blocking behavior in the sentry. diff --git a/pkg/sentry/fsimpl/host/socket.go b/pkg/sentry/fsimpl/host/socket.go index 8a447e29f..60acc367f 100644 --- a/pkg/sentry/fsimpl/host/socket.go +++ b/pkg/sentry/fsimpl/host/socket.go @@ -84,6 +84,8 @@ type ConnectedEndpoint struct { // init performs initialization required for creating new ConnectedEndpoints and // for restoring them. func (c *ConnectedEndpoint) init() *syserr.Error { + c.InitRefs() + family, err := syscall.GetsockoptInt(c.fd, syscall.SOL_SOCKET, syscall.SO_DOMAIN) if err != nil { return syserr.FromError(err) @@ -132,7 +134,6 @@ func NewConnectedEndpoint(ctx context.Context, hostFD int, addr string, saveable // ConnectedEndpointRefs start off with a single reference. We need two. e.IncRef() - e.EnableLeakCheck() return &e, nil } @@ -376,8 +377,7 @@ func NewSCMEndpoint(ctx context.Context, hostFD int, queue *waiter.Queue, addr s return nil, err } - // ConnectedEndpointRefs start off with a single reference. We need two. + // e starts off with a single reference. We need two. e.IncRef() - e.EnableLeakCheck() return &e, nil } diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index d83c17f83..eac578f25 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -660,7 +660,7 @@ var _ Inode = (*StaticDirectory)(nil) func NewStaticDir(ctx context.Context, creds *auth.Credentials, devMajor, devMinor uint32, ino uint64, perm linux.FileMode, children map[string]Inode, fdOpts GenericDirectoryFDOptions) Inode { inode := &StaticDirectory{} inode.Init(ctx, creds, devMajor, devMinor, ino, perm, fdOpts) - inode.EnableLeakCheck() + inode.InitRefs() inode.OrderedChildren.Init(OrderedChildrenOptions{}) links := inode.OrderedChildren.Populate(children) diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index abb477c7d..c14abcff4 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -222,7 +222,9 @@ func (d *Dentry) IncRef() { // d.refs may be 0 if d.fs.mu is locked, which serializes against // d.cacheLocked(). r := atomic.AddInt64(&d.refs, 1) - refsvfs2.LogIncRef(d, r) + if d.LogRefs() { + refsvfs2.LogIncRef(d, r) + } } // TryIncRef implements vfs.DentryImpl.TryIncRef. @@ -233,7 +235,9 @@ func (d *Dentry) TryIncRef() bool { return false } if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { - refsvfs2.LogTryIncRef(d, r+1) + if d.LogRefs() { + refsvfs2.LogTryIncRef(d, r+1) + } return true } } @@ -242,7 +246,9 @@ func (d *Dentry) TryIncRef() bool { // DecRef implements vfs.DentryImpl.DecRef. func (d *Dentry) DecRef(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.fs.mu.Lock() d.cacheLocked(ctx) @@ -254,7 +260,9 @@ func (d *Dentry) DecRef(ctx context.Context) { func (d *Dentry) decRefLocked(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.cacheLocked(ctx) } else if r < 0 { diff --git a/pkg/sentry/fsimpl/kernfs/kernfs_test.go b/pkg/sentry/fsimpl/kernfs/kernfs_test.go index 2418eec44..e63588e33 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs_test.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs_test.go @@ -109,7 +109,7 @@ func (fs *filesystem) newReadonlyDir(ctx context.Context, creds *auth.Credential dir := &readonlyDir{} dir.attrs.Init(ctx, creds, 0 /* devMajor */, 0 /* devMinor */, fs.NextIno(), linux.ModeDirectory|mode) dir.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - dir.EnableLeakCheck() + dir.InitRefs() dir.IncLinks(dir.OrderedChildren.Populate(contents)) return dir } @@ -147,7 +147,7 @@ func (fs *filesystem) newDir(ctx context.Context, creds *auth.Credentials, mode dir.fs = fs dir.attrs.Init(ctx, creds, 0 /* devMajor */, 0 /* devMinor */, fs.NextIno(), linux.ModeDirectory|mode) dir.OrderedChildren.Init(kernfs.OrderedChildrenOptions{Writable: true}) - dir.EnableLeakCheck() + dir.InitRefs() dir.IncLinks(dir.OrderedChildren.Populate(contents)) return dir diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 73130bc8d..3492409b2 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -514,7 +514,9 @@ func (d *dentry) IncRef() { // d.refs may be 0 if d.fs.renameMu is locked, which serializes against // d.checkDropLocked(). r := atomic.AddInt64(&d.refs, 1) - refsvfs2.LogIncRef(d, r) + if d.LogRefs() { + refsvfs2.LogIncRef(d, r) + } } // TryIncRef implements vfs.DentryImpl.TryIncRef. @@ -525,7 +527,9 @@ func (d *dentry) TryIncRef() bool { return false } if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { - refsvfs2.LogTryIncRef(d, r+1) + if d.LogRefs() { + refsvfs2.LogTryIncRef(d, r+1) + } return true } } @@ -534,7 +538,9 @@ func (d *dentry) TryIncRef() bool { // DecRef implements vfs.DentryImpl.DecRef. func (d *dentry) DecRef(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.fs.renameMu.Lock() d.checkDropLocked(ctx) @@ -546,7 +552,9 @@ func (d *dentry) DecRef(ctx context.Context) { func (d *dentry) decRefLocked(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.checkDropLocked(ctx) } else if r < 0 { diff --git a/pkg/sentry/fsimpl/proc/subtasks.go b/pkg/sentry/fsimpl/proc/subtasks.go index cb3c5e0fd..e001d5032 100644 --- a/pkg/sentry/fsimpl/proc/subtasks.go +++ b/pkg/sentry/fsimpl/proc/subtasks.go @@ -60,7 +60,7 @@ func (fs *filesystem) newSubtasks(task *kernel.Task, pidns *kernel.PIDNamespace, // Note: credentials are overridden by taskOwnedInode. subInode.InodeAttrs.Init(task, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555) subInode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - subInode.EnableLeakCheck() + subInode.InitRefs() inode := &taskOwnedInode{Inode: subInode, owner: task} return inode diff --git a/pkg/sentry/fsimpl/proc/task.go b/pkg/sentry/fsimpl/proc/task.go index 19011b010..dc46a09bc 100644 --- a/pkg/sentry/fsimpl/proc/task.go +++ b/pkg/sentry/fsimpl/proc/task.go @@ -91,7 +91,7 @@ func (fs *filesystem) newTaskInode(task *kernel.Task, pidns *kernel.PIDNamespace taskInode := &taskInode{task: task} // Note: credentials are overridden by taskOwnedInode. taskInode.InodeAttrs.Init(task, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555) - taskInode.EnableLeakCheck() + taskInode.InitRefs() inode := &taskOwnedInode{Inode: taskInode, owner: task} diff --git a/pkg/sentry/fsimpl/proc/task_fds.go b/pkg/sentry/fsimpl/proc/task_fds.go index d268b44be..3ec4471f5 100644 --- a/pkg/sentry/fsimpl/proc/task_fds.go +++ b/pkg/sentry/fsimpl/proc/task_fds.go @@ -128,7 +128,7 @@ func (fs *filesystem) newFDDirInode(task *kernel.Task) kernfs.Inode { }, } inode.InodeAttrs.Init(task, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555) - inode.EnableLeakCheck() + inode.InitRefs() inode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) return inode } @@ -265,7 +265,7 @@ func (fs *filesystem) newFDInfoDirInode(task *kernel.Task) kernfs.Inode { }, } inode.InodeAttrs.Init(task, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555) - inode.EnableLeakCheck() + inode.InitRefs() inode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) return inode } diff --git a/pkg/sentry/fsimpl/proc/tasks.go b/pkg/sentry/fsimpl/proc/tasks.go index b81ea14bf..151d1f10d 100644 --- a/pkg/sentry/fsimpl/proc/tasks.go +++ b/pkg/sentry/fsimpl/proc/tasks.go @@ -83,7 +83,7 @@ func (fs *filesystem) newTasksInode(ctx context.Context, k *kernel.Kernel, pidns cgroupControllers: cgroupControllers, } inode.InodeAttrs.Init(ctx, root, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555) - inode.EnableLeakCheck() + inode.InitRefs() inode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) links := inode.OrderedChildren.Populate(contents) diff --git a/pkg/sentry/fsimpl/sys/sys.go b/pkg/sentry/fsimpl/sys/sys.go index 506a2a0f0..79bc3fe88 100644 --- a/pkg/sentry/fsimpl/sys/sys.go +++ b/pkg/sentry/fsimpl/sys/sys.go @@ -160,7 +160,7 @@ func (fs *filesystem) newDir(ctx context.Context, creds *auth.Credentials, mode d := &dir{} d.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0755) d.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) - d.EnableLeakCheck() + d.InitRefs() d.IncLinks(d.OrderedChildren.Populate(contents)) return d } diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 4ce859d57..85a3dfe20 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -402,7 +402,7 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth i.mtime = now // i.nlink initialized by caller i.impl = impl - i.refs.EnableLeakCheck() + i.refs.InitRefs() } // incLinksLocked increments i's link count. diff --git a/pkg/sentry/fsimpl/verity/verity.go b/pkg/sentry/fsimpl/verity/verity.go index d24c839bb..faa862c55 100644 --- a/pkg/sentry/fsimpl/verity/verity.go +++ b/pkg/sentry/fsimpl/verity/verity.go @@ -380,7 +380,9 @@ func (fs *filesystem) newDentry() *dentry { // IncRef implements vfs.DentryImpl.IncRef. func (d *dentry) IncRef() { r := atomic.AddInt64(&d.refs, 1) - refsvfs2.LogIncRef(d, r) + if d.LogRefs() { + refsvfs2.LogIncRef(d, r) + } } // TryIncRef implements vfs.DentryImpl.TryIncRef. @@ -391,7 +393,9 @@ func (d *dentry) TryIncRef() bool { return false } if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { - refsvfs2.LogTryIncRef(d, r+1) + if d.LogRefs() { + refsvfs2.LogTryIncRef(d, r+1) + } return true } } @@ -400,7 +404,9 @@ func (d *dentry) TryIncRef() bool { // DecRef implements vfs.DentryImpl.DecRef. func (d *dentry) DecRef(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.fs.renameMu.Lock() d.checkDropLocked(ctx) @@ -412,7 +418,9 @@ func (d *dentry) DecRef(ctx context.Context) { func (d *dentry) decRefLocked(ctx context.Context) { r := atomic.AddInt64(&d.refs, -1) - refsvfs2.LogDecRef(d, r) + if d.LogRefs() { + refsvfs2.LogDecRef(d, r) + } if r == 0 { d.checkDropLocked(ctx) } else if r < 0 { |