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
This commit is contained in:
Rahat Mahmood
2022-05-12 16:08:33 -07:00
committed by gVisor bot
parent 14c5686d50
commit 7eec8dcf9a
3 changed files with 9 additions and 14 deletions
+2 -2
View File
@@ -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,
+2 -6
View File
@@ -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.
+5 -6
View File
@@ -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