CVE-2018-19333: Insufficient Information (NVD-CWE-noinfo)
value: pkg/sentry/kernel/shm/shm.go in Google gVisor before 2018-11-01 allows attackers to overwrite memory locations in processes running as root (but not escape the sandbox) via vectors involving IPC_RMID shmctl calls, because reference counting is mishandled.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 9.8
Published: 2018-11-17T17:29Z
Repository: https://github.com/google/gvisor
Language: Go
Files changed: pkg/sentry/kernel/shm/shm.go, pkg/sentry/syscalls/linux/sys_shm.go
Commit date: 2018-11-01 15:54:14 -0700
Commit message: Prevent premature destruction of shm segments.

Shm segments can be marked for lazy destruction via shmctl(IPC_RMID),
which destroys a segment once it is no longer attached to any
processes. We were unconditionally decrementing the segment refcount
on shmctl(IPC_RMID) which allowed a user to force a segment to be
destroyed by repeatedly calling shmctl(IPC_RMID), with outstanding
memory maps to the segment.

This is problematic because the memory released by a segment destroyed
this way can be reused by a different process while remaining
accessible by the process with outstanding maps to the segment.

PiperOrigin-RevId: 219713660
Change-Id: I443ab838322b4fb418ed87b2722c3413ead21845

Diff: diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go
index 2feffe61..f760f5f7 100644
--- a/pkg/sentry/kernel/shm/shm.go
+++ b/pkg/sentry/kernel/shm/shm.go
@@ -573,14 +573,23 @@ func (s *Shm) destroy() {
 // MarkDestroyed marks a shm for destruction. The shm is actually destroyed once
 // it has no references. See shmctl(IPC_RMID).
 func (s *Shm) MarkDestroyed() {
 	s.mu.Lock()
 	defer s.mu.Unlock()
+
 	// Prevent the segment from being found in the registry.
 	s.key = linux.IPC_PRIVATE
-	s.pendingDestruction = true
-	s.DecRef()
+
+	// Only drop the segment's self-reference once, when destruction is
+	// requested. Otherwise, repeated calls shmctl(IPC_RMID) would force a
+	// segment to be destroyed prematurely, potentially with active maps to the
+	// segment's address range. Remaining references are dropped when the
+	// segment is detached or unmaped.
+	if !s.pendingDestruction {
+		s.pendingDestruction = true
+		s.DecRef()
+	}
 }
 
 // checkOwnership verifies whether a segment may be accessed by ctx as an
 // owner. See ipc/util.c:ipcctl_pre_down_nolock() in Linux.
 //
diff --git a/pkg/sentry/syscalls/linux/sys_shm.go b/pkg/sentry/syscalls/linux/sys_shm.go
index 5f887523..8753c2e5 100644
--- a/pkg/sentry/syscalls/linux/sys_shm.go
+++ b/pkg/sentry/syscalls/linux/sys_shm.go
@@ -142,11 +142,11 @@ func Shmctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal
 	case linux.IPC_RMID:
 		segment.MarkDestroyed()
 		return 0, nil, nil
 
 	case linux.SHM_LOCK, linux.SHM_UNLOCK:
-		// We currently do not support memmory locking anywhere.
+		// We currently do not support memory locking anywhere.
 		// mlock(2)/munlock(2) are currently stubbed out as no-ops so do the
 		// same here.
 		t.Kernel().EmitUnimplementedEvent(t)
 		return 0, nil, nil
 

Vulnerable code: 	s.pendingDestruction = true
	s.DecRef()
		// We currently do not support memmory locking anywhere.
Fixed code: 

	// Only drop the segment's self-reference once, when destruction is
	// requested. Otherwise, repeated calls shmctl(IPC_RMID) would force a
	// segment to be destroyed prematurely, potentially with active maps to the
	// segment's address range. Remaining references are dropped when the
	// segment is detached or unmaped.
	if !s.pendingDestruction {
		s.pendingDestruction = true
		s.DecRef()
	}
		// We currently do not support memory locking anywhere.