From 7eec8dcf9a0135f9449d6a88763b66b71910acb7 Mon Sep 17 00:00:00 2001
From: Rahat Mahmood <rahat@google.com>
Date: Thu, 12 May 2022 16:05:56 -0700
Subject: [PATCH] mm: Protect mm.dumpability with atomic access instead of
 mm.metadataMu.

Previously, this was a lock order violation, as mm.metadataMutex ->
mm.mappingRWMutex -> kernel.taskSetRWMutex is required when forking
the task image, and ptrace aquired kernel.taskSetRWMutex ->
mm.metadataMutex to check dumpability.

Ptrace doesn't require a critical section around the use of the
dumpability value.

PiperOrigin-RevId: 448360804
---
 pkg/sentry/mm/lifecycle.go |  4 ++--
 pkg/sentry/mm/metadata.go  |  8 ++------
 pkg/sentry/mm/mm.go        | 11 +++++------
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go
index 32bad1172..3c1f4d033 100644
--- a/pkg/sentry/mm/lifecycle.go
+++ b/pkg/sentry/mm/lifecycle.go
@@ -36,7 +36,7 @@ func NewMemoryManager(p platform.Platform, mfp pgalloc.MemoryFileProvider, sleep
 		privateRefs:        &privateRefs{},
 		users:              atomicbitops.FromInt32(1),
 		auxv:               arch.Auxv{},
-		dumpability:        UserDumpable,
+		dumpability:        atomicbitops.FromInt32(int32(UserDumpable)),
 		aioManager:         aioManager{contexts: make(map[uint64]*AIOContext)},
 		sleepForActivation: sleepForActivation,
 	}
@@ -92,7 +92,7 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
 		auxv:                 append(arch.Auxv(nil), mm.auxv...),
 		// IncRef'd below, once we know that there isn't an error.
 		executable:         mm.executable,
-		dumpability:        mm.dumpability,
+		dumpability:        atomicbitops.FromInt32(mm.dumpability.Load()),
 		aioManager:         aioManager{contexts: make(map[uint64]*AIOContext)},
 		sleepForActivation: mm.sleepForActivation,
 		vdsoSigReturnAddr:  mm.vdsoSigReturnAddr,
diff --git a/pkg/sentry/mm/metadata.go b/pkg/sentry/mm/metadata.go
index 28c5fead9..4ddbb2490 100644
--- a/pkg/sentry/mm/metadata.go
+++ b/pkg/sentry/mm/metadata.go
@@ -39,16 +39,12 @@ const (
 
 // Dumpability returns the dumpability.
 func (mm *MemoryManager) Dumpability() Dumpability {
-	mm.metadataMu.Lock()
-	defer mm.metadataMu.Unlock()
-	return mm.dumpability
+	return Dumpability(mm.dumpability.Load())
 }
 
 // SetDumpability sets the dumpability.
 func (mm *MemoryManager) SetDumpability(d Dumpability) {
-	mm.metadataMu.Lock()
-	defer mm.metadataMu.Unlock()
-	mm.dumpability = d
+	mm.dumpability.Store(int32(d))
 }
 
 // ArgvStart returns the start of the application argument vector.
diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go
index aec322558..16f6e58e4 100644
--- a/pkg/sentry/mm/mm.go
+++ b/pkg/sentry/mm/mm.go
@@ -193,6 +193,11 @@ type MemoryManager struct {
 	captureInvalidations  bool             `state:"zerovalue"`
 	capturedInvalidations []invalidateArgs `state:"nosave"`
 
+	// dumpability describes if and how this MemoryManager may be dumped to
+	// userspace. This is read under kernel.TaskSet.mu, so it can't be protected
+	// by metadataMu.
+	dumpability atomicbitops.Int32
+
 	metadataMu sync.Mutex `state:"nosave"`
 
 	// argv is the application argv. This is set up by the loader and may be
@@ -220,12 +225,6 @@ type MemoryManager struct {
 	// executable is protected by metadataMu.
 	executable fsbridge.File
 
-	// dumpability describes if and how this MemoryManager may be dumped to
-	// userspace.
-	//
-	// dumpability is protected by metadataMu.
-	dumpability Dumpability
-
 	// aioManager keeps track of AIOContexts used for async IOs. AIOManager
 	// must be cloned when CLONE_VM is used.
 	aioManager aioManager