Merge release-20201030.0-53-g0fb5353e4 (automated)

4 files changed, 46 insertions, 28 deletions

diff --git a/pkg/sentry/fsimpl/host/connected_endpoint_refs.go b/pkg/sentry/fsimpl/host/connected_endpoint_refs.go
index faa9d2fd2..3f5f4ebc3 100644
--- a/pkg/sentry/fsimpl/host/connected_endpoint_refs.go
+++ b/pkg/sentry/fsimpl/host/connected_endpoint_refs.go
@@ -20,9 +20,6 @@ var ConnectedEndpointobj *ConnectedEndpoint
 // 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 ConnectedEndpointRefs struct {
 	// refCount is composed of two fields:
@@ -35,6 +32,13 @@ type ConnectedEndpointRefs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *ConnectedEndpointRefs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *ConnectedEndpointRefs) RefType() string {
 	return fmt.Sprintf("%T", ConnectedEndpointobj)[1:]
@@ -58,8 +62,7 @@ func (r *ConnectedEndpointRefs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *ConnectedEndpointRefs) ReadRefs() int64 {
-
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -67,8 +70,10 @@ func (r *ConnectedEndpointRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *ConnectedEndpointRefs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if ConnectedEndpointenableLogging {
+		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 *ConnectedEndpointRefs) IncRef() {
 //go:nosplit
 func (r *ConnectedEndpointRefs) 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 ConnectedEndpointenableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -107,12 +114,14 @@ func (r *ConnectedEndpointRefs) TryIncRef() bool {
 //go:nosplit
 func (r *ConnectedEndpointRefs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if ConnectedEndpointenableLogging {
+		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/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/inode_refs.go b/pkg/sentry/fsimpl/host/inode_refs.go
index ef2f56522..4c850a7ac 100644
--- a/pkg/sentry/fsimpl/host/inode_refs.go
+++ b/pkg/sentry/fsimpl/host/inode_refs.go
@@ -20,9 +20,6 @@ var inodeobj *inode
 // 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 inodeRefs struct {
 	// refCount is composed of two fields:
@@ -35,6 +32,13 @@ type inodeRefs struct {
 	refCount int64
 }
 
+// InitRefs initializes r with one reference and, if enabled, activates leak
+// checking.
+func (r *inodeRefs) InitRefs() {
+	atomic.StoreInt64(&r.refCount, 1)
+	refsvfs2.Register(r)
+}
+
 // RefType implements refsvfs2.CheckedObject.RefType.
 func (r *inodeRefs) RefType() string {
 	return fmt.Sprintf("%T", inodeobj)[1:]
@@ -58,8 +62,7 @@ func (r *inodeRefs) EnableLeakCheck() {
 // ReadRefs returns the current number of references. The returned count is
 // inherently racy and is unsafe to use without external synchronization.
 func (r *inodeRefs) ReadRefs() int64 {
-
-	return atomic.LoadInt64(&r.refCount) + 1
+	return atomic.LoadInt64(&r.refCount)
 }
 
 // IncRef implements refs.RefCounter.IncRef.
@@ -67,8 +70,10 @@ func (r *inodeRefs) ReadRefs() int64 {
 //go:nosplit
 func (r *inodeRefs) IncRef() {
 	v := atomic.AddInt64(&r.refCount, 1)
-	refsvfs2.LogIncRef(r, v+1)
-	if v <= 0 {
+	if inodeenableLogging {
+		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 *inodeRefs) IncRef() {
 //go:nosplit
 func (r *inodeRefs) 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 inodeenableLogging {
+		refsvfs2.LogTryIncRef(r, v)
+	}
 	return true
 }
 
@@ -107,12 +114,14 @@ func (r *inodeRefs) TryIncRef() bool {
 //go:nosplit
 func (r *inodeRefs) DecRef(destroy func()) {
 	v := atomic.AddInt64(&r.refCount, -1)
-	refsvfs2.LogDecRef(r, v+1)
+	if inodeenableLogging {
+		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/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
 }