From: Jan Vorlicek <janvorli@microsoft.com>
Date: Tue, 10 Dec 2019 11:40:49 +0000 (+0100)
Subject: Fix DynamicMethodDesc::Destroy vs code heap enumeration race (#713)
X-Git-Tag: submit/tizen/20210909.063632~10734
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=bd920acbd6fedb86cc91376c9fc39aae9243a370;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

Fix DynamicMethodDesc::Destroy vs code heap enumeration race (#713)

There is a race between DynamicMethodDesc::Destroy called from
the finalizer thread and the MethodDescs enumeration called from
ETW::MethodLog::SendEventsForJitMethods at process exit.
DynamicMethodDesc::Destroy cleanos up its members m_pSig and
m_pszMethodName and then it calls GetLCGMethodResolver()->Destroy();
That calls EEJitManager::FreeCodeMemory, which tries to take the
m_CodeHeapCritSec lock. But this lock is already held by
the ETW::MethodLog::SendEventsForJitMethods.
So the iterator can see half-destroyed DynamicMethodDesc and
a crash happens when trying to get the dynamic method name
from the m_pszMethodName for the ETW event purposes.

The fix is to call the GetLCGMethodResolver()->Destroy() before
destroying the m_pSig and m_pszMethodName.
---

diff --git a/src/coreclr/src/vm/dynamicmethod.cpp b/src/coreclr/src/vm/dynamicmethod.cpp
index a9d350c..b25dfb7 100644
--- a/src/coreclr/src/vm/dynamicmethod.cpp
+++ b/src/coreclr/src/vm/dynamicmethod.cpp
@@ -849,22 +849,27 @@ void DynamicMethodDesc::Destroy()
 
     _ASSERTE(IsDynamicMethod());
     LoaderAllocator *pLoaderAllocator = GetLoaderAllocator();
-
     LOG((LF_BCL, LL_INFO1000, "Level3 - Destroying DynamicMethod {0x%p}\n", this));
-    if (!m_pSig.IsNull())
+
+    // The m_pSig and m_pszMethodName need to be destroyed after the GetLCGMethodResolver()->Destroy() call
+    // otherwise the EEJitManager::CodeHeapIterator could return DynamicMethodDesc with these members NULLed, but
+    // the nibble map for the corresponding code memory indicating that this DynamicMethodDesc is still alive.
+    PCODE pSig = m_pSig.GetValue();
+    PTR_CUTF8 pszMethodName = m_pszMethodName.GetValue();
+
+    GetLCGMethodResolver()->Destroy();
+    // The current DynamicMethodDesc storage is destroyed at this point
+
+    if (pszMethodName != NULL)
     {
-        delete[] (BYTE*)m_pSig.GetValue();
-        m_pSig.SetValueMaybeNull(NULL);
+        delete[] pszMethodName;
     }
-    m_cSig = 0;
-    if (!m_pszMethodName.IsNull())
+
+    if (pSig != NULL)
     {
-        delete[] m_pszMethodName.GetValue();
-        m_pszMethodName.SetValueMaybeNull(NULL);
+        delete[] (BYTE*)pSig;
     }
 
-    GetLCGMethodResolver()->Destroy();
-
     if (pLoaderAllocator->IsCollectible())
     {
         if (pLoaderAllocator->Release())