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

pkg/refsvfs2/refs_template.go

@@ -13,10 +13,7 @@
 // limitations under the License.
 
 // Package refs_template defines a template that can be used by reference
-// counted objects. The "owner" template parameter is used in log messages to
-// indicate the type of reference-counted object that exhibited a reference
-// leak. As a result, structs that are embedded in other structs should not use
-// this template, since it will make tracking down leaks more difficult.
+// counted objects.
 package refs_template
 
 import (
@@ -43,9 +40,6 @@ var obj *T
 // Refs implements refs.RefCounter. It keeps a reference count using atomic
 // operations and calls the destructor when the count reaches zero.
 //
-// Note that the number of references is actually refCount + 1 so that a default
-// zero-value Refs object contains one reference.
-//
 // +stateify savable
 type Refs struct {
 	// refCount is composed of two fields:
@@ -58,6 +52,13 @@ type Refs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *Refs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *Refs) RefType() string {
 	return fmt.Sprintf("%T", obj)[1:]
@@ -81,8 +82,7 @@ func (r *Refs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *Refs) ReadRefs() int64 {
-	// Account for the internal -1 offset on refcounts.
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -90,8 +90,10 @@ func (r *Refs) ReadRefs() int64 {
 //go:nosplit
 func (r *Refs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if enableLogging {
+		refsvfs2.LogIncRef(r, v)
+	}
+	if v <= 1 {
 		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
@@ -105,7 +107,7 @@ func (r *Refs) IncRef() {
 //go:nosplit
 func (r *Refs) TryIncRef() bool {
 	const speculativeRef = 1 << 32
-	if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) < 0 {
+	if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) == 0 {
 		// This object has already been freed.
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
@@ -113,7 +115,9 @@ func (r *Refs) TryIncRef() bool {
 
 	// Turn into a real reference.
 	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
-	refsvfs2.LogTryIncRef(r, v+1)
+	if enableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -131,12 +135,14 @@ func (r *Refs) TryIncRef() bool {
 //go:nosplit
 func (r *Refs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if enableLogging {
+		refsvfs2.LogDecRef(r, v+1)
+	}
 	switch {
-	case v < -1:
+	case v < 0:
 		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %s", r, r.RefType()))
 
-	case v == -1:
+	case v == 0:
 		refsvfs2.Unregister(r)
 		// Call the destructor.
 		if destroy != nil {

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)

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
 }
 

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
 }

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")
 	}

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.

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
 }

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)

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 {

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

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 {

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

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}
 

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
 }

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)

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
 }

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.

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 {

pkg/sentry/kernel/fd_table_unsafe.go

@@ -43,7 +43,7 @@ func (f *FDTable) initNoLeakCheck() {
 // init initializes the table with leak checking.
 func (f *FDTable) init() {
 	f.initNoLeakCheck()
-	f.EnableLeakCheck()
+	f.InitRefs()
 }
 
 // get gets a file entry.

pkg/sentry/kernel/fs_context.go

@@ -63,7 +63,7 @@ func newFSContext(root, cwd *fs.Dirent, umask uint) *FSContext {
 		cwd:   cwd,
 		umask: umask,
 	}
-	f.EnableLeakCheck()
+	f.InitRefs()
 	return &f
 }
 
@@ -76,7 +76,7 @@ func NewFSContextVFS2(root, cwd vfs.VirtualDentry, umask uint) *FSContext {
 		cwdVFS2:  cwd,
 		umask:    umask,
 	}
-	f.EnableLeakCheck()
+	f.InitRefs()
 	return &f
 }
 
@@ -137,7 +137,7 @@ func (f *FSContext) Fork() *FSContext {
 		rootVFS2: f.rootVFS2,
 		umask:    f.umask,
 	}
-	ctx.EnableLeakCheck()
+	ctx.InitRefs()
 	return ctx
 }