[scudo] Lock/unlock MutexTSDs in disable/enable.

[cferris1000]

It is possible that a fork could occur while MutexTSDs is being held and then cause a deadlock in a forked process when something attempts to lock it again. Instead add it to the enable/disable list of mutexes.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Christopher Ferris (cferris1000)

Changes

It is possible that a fork could occur while MutexTSDs is being held and then cause a deadlock in a forked process when something attempts to lock it again. Instead add it to the enable/disable list of mutexes.

---

Full diff: https://github.com/llvm/llvm-project/pull/169440.diff

1 Files Affected:

• (modified) compiler-rt/lib/scudo/standalone/tsd_shared.h (+2)

diff --git a/compiler-rt/lib/scudo/standalone/tsd_shared.h b/compiler-rt/lib/scudo/standalone/tsd_shared.h
index 8b570a770b503..404e984e1f5e9 100644
--- a/compiler-rt/lib/scudo/standalone/tsd_shared.h
+++ b/compiler-rt/lib/scudo/standalone/tsd_shared.h
@@ -93,6 +93,7 @@ struct TSDRegistrySharedT {
 
   void disable() NO_THREAD_SAFETY_ANALYSIS {
     Mutex.lock();
+    MutexTSDs.lock();
     for (u32 I = 0; I < TSDsArraySize; I++)
       TSDs[I].lock();
   }
@@ -100,6 +101,7 @@ struct TSDRegistrySharedT {
   void enable() NO_THREAD_SAFETY_ANALYSIS {
     for (s32 I = static_cast<s32>(TSDsArraySize - 1); I >= 0; I--)
       TSDs[I].unlock();
+    MutexTSDs.unlock();
     Mutex.unlock();
   }
 

[cferris1000]

Chia-hung, since you did a lot of the work on the locking code, I wanted you to look at this. I think this is fine and I ran some allocation stress testing and didn't show any problems.

[ChiaHungDuan]

Can we also add ACQUIRED_BEFORE(MutexTSDs) on Mutex? Even though there's no thread safety analysis for enable()/disable() but I guess in init() this can be useful.

[cferris1000]

I feel like this might confuse people since Mutex and MutexTSDs are not really related. Looking at this more closely, I'm not sure we really need both Mutexes since one is only used for initialization and the other used for the TSD access. I don't want to make that change in this pull request though. But I'm going to look at a follow-on that removes Mutex completely. If there is a reason to keep them separate, let me know.