Merge release-20201030.0-53-g0fb5353e4 (automated)

6 files changed, 75 insertions, 41 deletions

diff --git a/pkg/sentry/vfs/file_description.go b/pkg/sentry/vfs/file_description.go
index 546e445aa..936f9fc71 100644
--- a/pkg/sentry/vfs/file_description.go
+++ b/pkg/sentry/vfs/file_description.go
@@ -133,7 +133,7 @@ func (fd *FileDescription) Init(impl FileDescriptionImpl, flags uint32, mnt *Mou
 		}
 	}
 
-	fd.EnableLeakCheck()
+	fd.InitRefs()
 
 	// Remove "file creation flags" to mirror the behavior from file.f_flags in
 	// fs/open.c:do_dentry_open.
diff --git a/pkg/sentry/vfs/file_description_refs.go b/pkg/sentry/vfs/file_description_refs.go
index 5cc5fe104..1e6d3f5af 100644
--- a/pkg/sentry/vfs/file_description_refs.go
+++ b/pkg/sentry/vfs/file_description_refs.go
@@ -20,9 +20,6 @@ var FileDescriptionobj *FileDescription
 // 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 FileDescriptionRefs struct {
 	// refCount is composed of two fields:
@@ -35,6 +32,13 @@ type FileDescriptionRefs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *FileDescriptionRefs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *FileDescriptionRefs) RefType() string {
 	return fmt.Sprintf("%T", FileDescriptionobj)[1:]
@@ -58,8 +62,7 @@ func (r *FileDescriptionRefs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *FileDescriptionRefs) ReadRefs() int64 {
-
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -67,8 +70,10 @@ func (r *FileDescriptionRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *FileDescriptionRefs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if FileDescriptionenableLogging {
+		refsvfs2.LogIncRef(r, v)
+	}
+	if v <= 1 {
 		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
@@ -82,14 +87,16 @@ func (r *FileDescriptionRefs) IncRef() {
 //go:nosplit
 func (r *FileDescriptionRefs) 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 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
 	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
-	refsvfs2.LogTryIncRef(r, v+1)
+	if FileDescriptionenableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -107,12 +114,14 @@ func (r *FileDescriptionRefs) TryIncRef() bool {
 //go:nosplit
 func (r *FileDescriptionRefs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if FileDescriptionenableLogging {
+		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)
 
 		if destroy != nil {
diff --git a/pkg/sentry/vfs/filesystem.go b/pkg/sentry/vfs/filesystem.go
index c93d94634..2c4b81e78 100644
--- a/pkg/sentry/vfs/filesystem.go
+++ b/pkg/sentry/vfs/filesystem.go
@@ -48,7 +48,7 @@ type Filesystem struct {
 
 // Init must be called before first use of fs.
 func (fs *Filesystem) Init(vfsObj *VirtualFilesystem, fsType FilesystemType, impl FilesystemImpl) {
-	fs.EnableLeakCheck()
+	fs.InitRefs()
 	fs.vfs = vfsObj
 	fs.fsType = fsType
 	fs.impl = impl
diff --git a/pkg/sentry/vfs/filesystem_refs.go b/pkg/sentry/vfs/filesystem_refs.go
index 6b403c04a..75da47bef 100644
--- a/pkg/sentry/vfs/filesystem_refs.go
+++ b/pkg/sentry/vfs/filesystem_refs.go
@@ -20,9 +20,6 @@ var Filesystemobj *Filesystem
 // 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 FilesystemRefs struct {
 	// refCount is composed of two fields:
@@ -35,6 +32,13 @@ type FilesystemRefs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *FilesystemRefs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *FilesystemRefs) RefType() string {
 	return fmt.Sprintf("%T", Filesystemobj)[1:]
@@ -58,8 +62,7 @@ func (r *FilesystemRefs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *FilesystemRefs) ReadRefs() int64 {
-
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -67,8 +70,10 @@ func (r *FilesystemRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *FilesystemRefs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if FilesystemenableLogging {
+		refsvfs2.LogIncRef(r, v)
+	}
+	if v <= 1 {
 		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
@@ -82,14 +87,16 @@ func (r *FilesystemRefs) IncRef() {
 //go:nosplit
 func (r *FilesystemRefs) 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 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
 	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
-	refsvfs2.LogTryIncRef(r, v+1)
+	if FilesystemenableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -107,12 +114,14 @@ func (r *FilesystemRefs) TryIncRef() bool {
 //go:nosplit
 func (r *FilesystemRefs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if FilesystemenableLogging {
+		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)
 
 		if destroy != nil {
diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go
index 3ea981ad4..d865fd603 100644
--- a/pkg/sentry/vfs/mount.go
+++ b/pkg/sentry/vfs/mount.go
@@ -169,7 +169,7 @@ func (vfs *VirtualFilesystem) NewMountNamespace(ctx context.Context, creds *auth
 		Owner:       creds.UserNamespace,
 		mountpoints: make(map[*Dentry]uint32),
 	}
-	mntns.EnableLeakCheck()
+	mntns.InitRefs()
 	mntns.root = newMount(vfs, fs, root, mntns, opts)
 	return mntns, nil
 }
@@ -477,7 +477,9 @@ func (mnt *Mount) tryIncMountedRef() bool {
 			return false
 		}
 		if atomic.CompareAndSwapInt64(&mnt.refs, r, r+1) {
-			refsvfs2.LogTryIncRef(mnt, r+1)
+			if mnt.LogRefs() {
+				refsvfs2.LogTryIncRef(mnt, r+1)
+			}
 			return true
 		}
 	}
@@ -488,12 +490,17 @@ func (mnt *Mount) IncRef() {
 	// In general, negative values for mnt.refs are valid because the MSB is
 	// the eager-unmount bit.
 	r := atomic.AddInt64(&mnt.refs, 1)
-	refsvfs2.LogIncRef(mnt, r)
+	if mnt.LogRefs() {
+		refsvfs2.LogIncRef(mnt, r)
+	}
 }
 
 // DecRef decrements mnt's reference count.
 func (mnt *Mount) DecRef(ctx context.Context) {
 	r := atomic.AddInt64(&mnt.refs, -1)
+	if mnt.LogRefs() {
+		refsvfs2.LogDecRef(mnt, r)
+	}
 	if r&^math.MinInt64 == 0 { // mask out MSB
 		refsvfs2.Unregister(mnt)
 		mnt.destroy(ctx)
diff --git a/pkg/sentry/vfs/mount_namespace_refs.go b/pkg/sentry/vfs/mount_namespace_refs.go
index 7c39d445f..bd79fb8a7 100644
--- a/pkg/sentry/vfs/mount_namespace_refs.go
+++ b/pkg/sentry/vfs/mount_namespace_refs.go
@@ -20,9 +20,6 @@ var MountNamespaceobj *MountNamespace
 // 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 MountNamespaceRefs struct {
 	// refCount is composed of two fields:
@@ -35,6 +32,13 @@ type MountNamespaceRefs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *MountNamespaceRefs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *MountNamespaceRefs) RefType() string {
 	return fmt.Sprintf("%T", MountNamespaceobj)[1:]
@@ -58,8 +62,7 @@ func (r *MountNamespaceRefs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *MountNamespaceRefs) ReadRefs() int64 {
-
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -67,8 +70,10 @@ func (r *MountNamespaceRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *MountNamespaceRefs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if MountNamespaceenableLogging {
+		refsvfs2.LogIncRef(r, v)
+	}
+	if v <= 1 {
 		panic(fmt.Sprintf("Incrementing non-positive count %p on %s", r, r.RefType()))
 	}
 }
@@ -82,14 +87,16 @@ func (r *MountNamespaceRefs) IncRef() {
 //go:nosplit
 func (r *MountNamespaceRefs) 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 {
 
 		atomic.AddInt64(&r.refCount, -speculativeRef)
 		return false
 	}
 
 	v := atomic.AddInt64(&r.refCount, -speculativeRef+1)
-	refsvfs2.LogTryIncRef(r, v+1)
+	if MountNamespaceenableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -107,12 +114,14 @@ func (r *MountNamespaceRefs) TryIncRef() bool {
 //go:nosplit
 func (r *MountNamespaceRefs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if MountNamespaceenableLogging {
+		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)
 
 		if destroy != nil {