Merge release-20201019.0-95-g3b4674ffe (automated)

5 files changed, 131 insertions, 72 deletions

diff --git a/pkg/sentry/vfs/file_description_refs.go b/pkg/sentry/vfs/file_description_refs.go
index 951cfbd4d..5cc5fe104 100644
--- a/pkg/sentry/vfs/file_description_refs.go
+++ b/pkg/sentry/vfs/file_description_refs.go
@@ -7,9 +7,15 @@ import (
 	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
-// ownerType is used to customize logging. Note that we use a pointer to T so
-// that we do not copy the entire object when passed as a format parameter.
-var FileDescriptionownerType *FileDescription
+// enableLogging indicates whether reference-related events should be logged (with
+// stack traces). This is false by default and should only be set to true for
+// debugging purposes, as it can generate an extremely large amount of output
+// and drastically degrade performance.
+const FileDescriptionenableLogging = false
+
+// obj is used to customize logging. Note that we use a pointer to T so that
+// we do not copy the entire object when passed as a format parameter.
+var FileDescriptionobj *FileDescription
 
 // Refs implements refs.RefCounter. It keeps a reference count using atomic
 // operations and calls the destructor when the count reaches zero.
@@ -29,16 +35,24 @@ type FileDescriptionRefs struct {
 	refCount int64
 }
 
-// EnableLeakCheck enables reference leak checking on r.
-func (r *FileDescriptionRefs) EnableLeakCheck() {
-	if refsvfs2.LeakCheckEnabled() {
-		refsvfs2.Register(r, fmt.Sprintf("%T", FileDescriptionownerType))
-	}
+// RefType implements refsvfs2.CheckedObject.RefType.
+func (r *FileDescriptionRefs) RefType() string {
+	return fmt.Sprintf("%T", FileDescriptionobj)[1:]
 }
 
 // LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
 func (r *FileDescriptionRefs) LeakMessage() string {
-	return fmt.Sprintf("%T %p: reference count of %d instead of 0", FileDescriptionownerType, r, r.ReadRefs())
+	return fmt.Sprintf("[%s %p] reference count of %d instead of 0", r.RefType(), r, r.ReadRefs())
+}
+
+// LogRefs implements refsvfs2.CheckedObject.LogRefs.
+func (r *FileDescriptionRefs) LogRefs() bool {
+	return FileDescriptionenableLogging
+}
+
+// EnableLeakCheck enables reference leak checking on r.
+func (r *FileDescriptionRefs) EnableLeakCheck() {
+	refsvfs2.Register(r)
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -52,8 +66,10 @@ func (r *FileDescriptionRefs) ReadRefs() int64 {
 //
 //go:nosplit
 func (r *FileDescriptionRefs) IncRef() {
-	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FileDescriptionownerType))
+	v := atomic.AddInt64(&r.refCount, 1)
+	refsvfs2.LogIncRef(r, v+1)
+	if v <= 0 {
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
 
@@ -66,14 +82,14 @@ func (r *FileDescriptionRefs) IncRef() {
 //go:nosplit
 func (r *FileDescriptionRefs) TryIncRef() bool {
 	const speculativeRef = 1 << 32
-	v := atomic.AddInt64(&r.refCount, speculativeRef)
-	if int32(v) < 0 {
+	if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) < 0 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
-	atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	refsvfs2.LogTryIncRef(r, v+1)
 	return true
 }
 
@@ -90,14 +106,14 @@ func (r *FileDescriptionRefs) TryIncRef() bool {
 //
 //go:nosplit
 func (r *FileDescriptionRefs) DecRef(destroy func()) {
-	switch v := atomic.AddInt64(&r.refCount, -1); {
+	v := atomic.AddInt64(&r.refCount, -1)
+	refsvfs2.LogDecRef(r, v+1)
+	switch {
 	case v < -1:
-		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FileDescriptionownerType))
+		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %s", r, r.RefType()))
 
 	case v == -1:
-		if refsvfs2.LeakCheckEnabled() {
-			refsvfs2.Unregister(r, fmt.Sprintf("%T", FileDescriptionownerType))
-		}
+		refsvfs2.Unregister(r)
 
 		if destroy != nil {
 			destroy()
@@ -106,7 +122,7 @@ func (r *FileDescriptionRefs) DecRef(destroy func()) {
 }
 
 func (r *FileDescriptionRefs) afterLoad() {
-	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+	if r.ReadRefs() > 0 {
 		r.EnableLeakCheck()
 	}
 }
diff --git a/pkg/sentry/vfs/filesystem_refs.go b/pkg/sentry/vfs/filesystem_refs.go
index f1abc120d..6b403c04a 100644
--- a/pkg/sentry/vfs/filesystem_refs.go
+++ b/pkg/sentry/vfs/filesystem_refs.go
@@ -7,9 +7,15 @@ import (
 	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
-// ownerType is used to customize logging. Note that we use a pointer to T so
-// that we do not copy the entire object when passed as a format parameter.
-var FilesystemownerType *Filesystem
+// enableLogging indicates whether reference-related events should be logged (with
+// stack traces). This is false by default and should only be set to true for
+// debugging purposes, as it can generate an extremely large amount of output
+// and drastically degrade performance.
+const FilesystemenableLogging = false
+
+// obj is used to customize logging. Note that we use a pointer to T so that
+// we do not copy the entire object when passed as a format parameter.
+var Filesystemobj *Filesystem
 
 // Refs implements refs.RefCounter. It keeps a reference count using atomic
 // operations and calls the destructor when the count reaches zero.
@@ -29,16 +35,24 @@ type FilesystemRefs struct {
 	refCount int64
 }
 
-// EnableLeakCheck enables reference leak checking on r.
-func (r *FilesystemRefs) EnableLeakCheck() {
-	if refsvfs2.LeakCheckEnabled() {
-		refsvfs2.Register(r, fmt.Sprintf("%T", FilesystemownerType))
-	}
+// RefType implements refsvfs2.CheckedObject.RefType.
+func (r *FilesystemRefs) RefType() string {
+	return fmt.Sprintf("%T", Filesystemobj)[1:]
 }
 
 // LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
 func (r *FilesystemRefs) LeakMessage() string {
-	return fmt.Sprintf("%T %p: reference count of %d instead of 0", FilesystemownerType, r, r.ReadRefs())
+	return fmt.Sprintf("[%s %p] reference count of %d instead of 0", r.RefType(), r, r.ReadRefs())
+}
+
+// LogRefs implements refsvfs2.CheckedObject.LogRefs.
+func (r *FilesystemRefs) LogRefs() bool {
+	return FilesystemenableLogging
+}
+
+// EnableLeakCheck enables reference leak checking on r.
+func (r *FilesystemRefs) EnableLeakCheck() {
+	refsvfs2.Register(r)
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -52,8 +66,10 @@ func (r *FilesystemRefs) ReadRefs() int64 {
 //
 //go:nosplit
 func (r *FilesystemRefs) IncRef() {
-	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, FilesystemownerType))
+	v := atomic.AddInt64(&r.refCount, 1)
+	refsvfs2.LogIncRef(r, v+1)
+	if v <= 0 {
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
 
@@ -66,14 +82,14 @@ func (r *FilesystemRefs) IncRef() {
 //go:nosplit
 func (r *FilesystemRefs) TryIncRef() bool {
 	const speculativeRef = 1 << 32
-	v := atomic.AddInt64(&r.refCount, speculativeRef)
-	if int32(v) < 0 {
+	if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) < 0 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
-	atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	refsvfs2.LogTryIncRef(r, v+1)
 	return true
 }
 
@@ -90,14 +106,14 @@ func (r *FilesystemRefs) TryIncRef() bool {
 //
 //go:nosplit
 func (r *FilesystemRefs) DecRef(destroy func()) {
-	switch v := atomic.AddInt64(&r.refCount, -1); {
+	v := atomic.AddInt64(&r.refCount, -1)
+	refsvfs2.LogDecRef(r, v+1)
+	switch {
 	case v < -1:
-		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, FilesystemownerType))
+		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %s", r, r.RefType()))
 
 	case v == -1:
-		if refsvfs2.LeakCheckEnabled() {
-			refsvfs2.Unregister(r, fmt.Sprintf("%T", FilesystemownerType))
-		}
+		refsvfs2.Unregister(r)
 
 		if destroy != nil {
 			destroy()
@@ -106,7 +122,7 @@ func (r *FilesystemRefs) DecRef(destroy func()) {
 }
 
 func (r *FilesystemRefs) afterLoad() {
-	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+	if r.ReadRefs() > 0 {
 		r.EnableLeakCheck()
 	}
 }
diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go
index d452d2cda..3ea981ad4 100644
--- a/pkg/sentry/vfs/mount.go
+++ b/pkg/sentry/vfs/mount.go
@@ -107,9 +107,7 @@ func newMount(vfs *VirtualFilesystem, fs *Filesystem, root *Dentry, mntns *Mount
 	if opts.ReadOnly {
 		mnt.setReadOnlyLocked(true)
 	}
-	if refsvfs2.LeakCheckEnabled() {
-		refsvfs2.Register(mnt, "vfs.Mount")
-	}
+	refsvfs2.Register(mnt)
 	return mnt
 }
 
@@ -474,11 +472,12 @@ func (vfs *VirtualFilesystem) disconnectLocked(mnt *Mount) VirtualDentry {
 // tryIncMountedRef does not require that a reference is held on mnt.
 func (mnt *Mount) tryIncMountedRef() bool {
 	for {
-		refs := atomic.LoadInt64(&mnt.refs)
-		if refs <= 0 { // refs < 0 => MSB set => eagerly unmounted
+		r := atomic.LoadInt64(&mnt.refs)
+		if r <= 0 { // r < 0 => MSB set => eagerly unmounted
 			return false
 		}
-		if atomic.CompareAndSwapInt64(&mnt.refs, refs, refs+1) {
+		if atomic.CompareAndSwapInt64(&mnt.refs, r, r+1) {
+			refsvfs2.LogTryIncRef(mnt, r+1)
 			return true
 		}
 	}
@@ -488,16 +487,15 @@ func (mnt *Mount) tryIncMountedRef() bool {
 func (mnt *Mount) IncRef() {
 	// In general, negative values for mnt.refs are valid because the MSB is
 	// the eager-unmount bit.
-	atomic.AddInt64(&mnt.refs, 1)
+	r := atomic.AddInt64(&mnt.refs, 1)
+	refsvfs2.LogIncRef(mnt, r)
 }
 
 // DecRef decrements mnt's reference count.
 func (mnt *Mount) DecRef(ctx context.Context) {
 	r := atomic.AddInt64(&mnt.refs, -1)
 	if r&^math.MinInt64 == 0 { // mask out MSB
-		if refsvfs2.LeakCheckEnabled() {
-			refsvfs2.Unregister(mnt, "vfs.Mount")
-		}
+		refsvfs2.Unregister(mnt)
 		mnt.destroy(ctx)
 	}
 }
@@ -520,11 +518,24 @@ func (mnt *Mount) destroy(ctx context.Context) {
 	}
 }
 
+// RefType implements refsvfs2.CheckedObject.Type.
+func (mnt *Mount) RefType() string {
+	return "vfs.Mount"
+}
+
 // LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
 func (mnt *Mount) LeakMessage() string {
 	return fmt.Sprintf("[vfs.Mount %p] reference count of %d instead of 0", mnt, atomic.LoadInt64(&mnt.refs))
 }
 
+// LogRefs implements refsvfs2.CheckedObject.LogRefs.
+//
+// This should only be set to true for debugging purposes, as it can generate an
+// extremely large amount of output and drastically degrade performance.
+func (mnt *Mount) LogRefs() bool {
+	return false
+}
+
 // DecRef decrements mntns' reference count.
 func (mntns *MountNamespace) DecRef(ctx context.Context) {
 	vfs := mntns.root.fs.VirtualFilesystem()
diff --git a/pkg/sentry/vfs/mount_namespace_refs.go b/pkg/sentry/vfs/mount_namespace_refs.go
index 32e28ebf8..7c39d445f 100644
--- a/pkg/sentry/vfs/mount_namespace_refs.go
+++ b/pkg/sentry/vfs/mount_namespace_refs.go
@@ -7,9 +7,15 @@ import (
 	"gvisor.dev/gvisor/pkg/refsvfs2"
 )
 
-// ownerType is used to customize logging. Note that we use a pointer to T so
-// that we do not copy the entire object when passed as a format parameter.
-var MountNamespaceownerType *MountNamespace
+// enableLogging indicates whether reference-related events should be logged (with
+// stack traces). This is false by default and should only be set to true for
+// debugging purposes, as it can generate an extremely large amount of output
+// and drastically degrade performance.
+const MountNamespaceenableLogging = false
+
+// obj is used to customize logging. Note that we use a pointer to T so that
+// we do not copy the entire object when passed as a format parameter.
+var MountNamespaceobj *MountNamespace
 
 // Refs implements refs.RefCounter. It keeps a reference count using atomic
 // operations and calls the destructor when the count reaches zero.
@@ -29,16 +35,24 @@ type MountNamespaceRefs struct {
 	refCount int64
 }
 
-// EnableLeakCheck enables reference leak checking on r.
-func (r *MountNamespaceRefs) EnableLeakCheck() {
-	if refsvfs2.LeakCheckEnabled() {
-		refsvfs2.Register(r, fmt.Sprintf("%T", MountNamespaceownerType))
-	}
+// RefType implements refsvfs2.CheckedObject.RefType.
+func (r *MountNamespaceRefs) RefType() string {
+	return fmt.Sprintf("%T", MountNamespaceobj)[1:]
 }
 
 // LeakMessage implements refsvfs2.CheckedObject.LeakMessage.
 func (r *MountNamespaceRefs) LeakMessage() string {
-	return fmt.Sprintf("%T %p: reference count of %d instead of 0", MountNamespaceownerType, r, r.ReadRefs())
+	return fmt.Sprintf("[%s %p] reference count of %d instead of 0", r.RefType(), r, r.ReadRefs())
+}
+
+// LogRefs implements refsvfs2.CheckedObject.LogRefs.
+func (r *MountNamespaceRefs) LogRefs() bool {
+	return MountNamespaceenableLogging
+}
+
+// EnableLeakCheck enables reference leak checking on r.
+func (r *MountNamespaceRefs) EnableLeakCheck() {
+	refsvfs2.Register(r)
 }
 
 // ReadRefs returns the current number of references. The returned count is
@@ -52,8 +66,10 @@ func (r *MountNamespaceRefs) ReadRefs() int64 {
 //
 //go:nosplit
 func (r *MountNamespaceRefs) IncRef() {
-	if v := atomic.AddInt64(&r.refCount, 1); v <= 0 {
-		panic(fmt.Sprintf("Incrementing non-positive count %p on %T", r, MountNamespaceownerType))
+	v := atomic.AddInt64(&r.refCount, 1)
+	refsvfs2.LogIncRef(r, v+1)
+	if v <= 0 {
+		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
 
@@ -66,14 +82,14 @@ func (r *MountNamespaceRefs) IncRef() {
 //go:nosplit
 func (r *MountNamespaceRefs) TryIncRef() bool {
 	const speculativeRef = 1 << 32
-	v := atomic.AddInt64(&r.refCount, speculativeRef)
-	if int32(v) < 0 {
+	if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) < 0 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
-	atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
+	refsvfs2.LogTryIncRef(r, v+1)
 	return true
 }
 
@@ -90,14 +106,14 @@ func (r *MountNamespaceRefs) TryIncRef() bool {
 //
 //go:nosplit
 func (r *MountNamespaceRefs) DecRef(destroy func()) {
-	switch v := atomic.AddInt64(&r.refCount, -1); {
+	v := atomic.AddInt64(&r.refCount, -1)
+	refsvfs2.LogDecRef(r, v+1)
+	switch {
 	case v < -1:
-		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %T", r, MountNamespaceownerType))
+		panic(fmt.Sprintf("Decrementing non-positive ref count %p, owned by %s", r, r.RefType()))
 
 	case v == -1:
-		if refsvfs2.LeakCheckEnabled() {
-			refsvfs2.Unregister(r, fmt.Sprintf("%T", MountNamespaceownerType))
-		}
+		refsvfs2.Unregister(r)
 
 		if destroy != nil {
 			destroy()
@@ -106,7 +122,7 @@ func (r *MountNamespaceRefs) DecRef(destroy func()) {
 }
 
 func (r *MountNamespaceRefs) afterLoad() {
-	if refsvfs2.LeakCheckEnabled() && r.ReadRefs() > 0 {
+	if r.ReadRefs() > 0 {
 		r.EnableLeakCheck()
 	}
 }
diff --git a/pkg/sentry/vfs/save_restore.go b/pkg/sentry/vfs/save_restore.go
index 46e50d55d..7723ed643 100644
--- a/pkg/sentry/vfs/save_restore.go
+++ b/pkg/sentry/vfs/save_restore.go
@@ -111,8 +111,8 @@ func (vfs *VirtualFilesystem) loadMounts(mounts []*Mount) {
 }
 
 func (mnt *Mount) afterLoad() {
-	if refsvfs2.LeakCheckEnabled() && atomic.LoadInt64(&mnt.refs) != 0 {
-		refsvfs2.Register(mnt, "vfs.Mount")
+	if atomic.LoadInt64(&mnt.refs) != 0 {
+		refsvfs2.Register(mnt)
 	}
 }