Fix race condition caused by updating interface map with exact target (#55308)

- Change interface map reading to reliably perform a volatile load (without barrier)
- When performing scan for exact interface match/match trhough SpecialMarkerTypeForGenericCasting, be careful to only read the map once and use that value for all comparisons
- All other uses of InterfaceMapIterator have been audited, and look to be safe

4 files changed, 37 insertions, 6 deletions

diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp
index 2fc25cf8133..834d3cfc8c4 100644
--- a/src/coreclr/vm/methodtable.cpp
+++ b/src/coreclr/vm/methodtable.cpp
@@ -713,8 +713,10 @@ PTR_MethodTable InterfaceInfo_t::GetApproxMethodTable(Module * pContainingModule
 
         return pItfMT;
     }
-#endif
     MethodTable * pItfMT = m_pMethodTable.GetValue();
+#else
+    MethodTable * pItfMT = GetMethodTable();
+#endif
     ClassLoader::EnsureLoaded(TypeHandle(pItfMT), CLASS_LOAD_APPROXPARENTS);
     return pItfMT;
 }
@@ -9269,7 +9271,7 @@ MethodTable::ResolveVirtualStaticMethod(MethodTable* pInterfaceType, MethodDesc*
                     {
                         if (it.CurrentInterfaceMatches(this, pInterfaceType))
                         {
-                            // This is the variant interface check logic, skip the 
+                            // This is the variant interface check logic, skip the exact matches as they were handled above
                             continue;
                         }
 
diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h
index 476a1cd3faa..4063e50b7b6 100644
--- a/src/coreclr/vm/methodtable.h
+++ b/src/coreclr/vm/methodtable.h
@@ -105,24 +105,36 @@ struct InterfaceInfo_t
 #endif
 
     // Method table of the interface
+#ifdef FEATURE_PREJIT
 #if defined(FEATURE_NGEN_RELOCS_OPTIMIZATIONS)
     RelativeFixupPointer<PTR_MethodTable> m_pMethodTable;
 #else
     FixupPointer<PTR_MethodTable> m_pMethodTable;
 #endif
+#else
+    PTR_MethodTable m_pMethodTable;
+#endif
 
 public:
     FORCEINLINE PTR_MethodTable GetMethodTable()
     {
         LIMITED_METHOD_CONTRACT;
+#ifdef FEATURE_PREJIT
         return ReadPointerMaybeNull(this, &InterfaceInfo_t::m_pMethodTable);
+#else
+        return VolatileLoadWithoutBarrier(&m_pMethodTable);
+#endif
     }
 
 #ifndef DACCESS_COMPILE
     void SetMethodTable(MethodTable * pMT)
     {
         LIMITED_METHOD_CONTRACT;
+#ifdef FEATURE_PREJIT
         m_pMethodTable.SetValueMaybeNull(pMT);
+#else
+        return VolatileStoreWithoutBarrier(&m_pMethodTable, pMT);
+#endif
     }
 
     // Get approximate method table. This is used by the type loader before the type is fully loaded.
@@ -132,7 +144,11 @@ public:
 #ifndef DACCESS_COMPILE
     InterfaceInfo_t(InterfaceInfo_t &right)
     {
+#ifdef FEATURE_PREJIT
         m_pMethodTable.SetValueMaybeNull(right.m_pMethodTable.GetValueMaybeNull());
+#else
+        VolatileStoreWithoutBarrier(&m_pMethodTable, VolatileLoadWithoutBarrier(&right.m_pMethodTable));
+#endif
     }
 #else // !DACCESS_COMPILE
 private:
@@ -2222,12 +2238,14 @@ public:
             }
             CONTRACT_END;
 
-            bool exactMatch = m_pMap->GetMethodTable() == pMT;
+            MethodTable *pCurrentMethodTable = m_pMap->GetMethodTable();
+
+            bool exactMatch = pCurrentMethodTable == pMT;
             if (!exactMatch)
             {
-                if (m_pMap->GetMethodTable()->HasSameTypeDefAs(pMT) && 
+                if (pCurrentMethodTable->HasSameTypeDefAs(pMT) && 
                     pMT->HasInstantiation() && 
-                    m_pMap->GetMethodTable()->IsSpecialMarkerTypeForGenericCasting() && 
+                    pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() && 
                     pMT->GetInstantiation().ContainsAllOneType(pMTOwner))
                 {
                     exactMatch = true;
diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl
index 688fc2630bf..a3adc702cbe 100644
--- a/src/coreclr/vm/methodtable.inl
+++ b/src/coreclr/vm/methodtable.inl
@@ -1578,7 +1578,16 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface)
 
         do
         {
-            if (pInfo->GetMethodTable()->HasSameTypeDefAs(pInterface) && pInfo->GetMethodTable()->IsSpecialMarkerTypeForGenericCasting())
+            MethodTable *pInterfaceInMap = pInfo->GetMethodTable();
+            if (pInterfaceInMap == pInterface)
+            {
+                // Since there is no locking on updating the interface with an exact match
+                // It is possible to reach here for a match which would ideally have been handled above
+                // GetMethodTable uses a VolatileLoadWithoutBarrier to prevent compiler optimizations
+                // from interfering with this check
+                return TRUE;
+            }
+            if (pInterfaceInMap->HasSameTypeDefAs(pInterface) && pInterfaceInMap->IsSpecialMarkerTypeForGenericCasting())
             {
                 // Extensible RCW's need to be handled specially because they can have interfaces
                 // in their map that are added at runtime. These interfaces will have a start offset
diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp
index 2d0889c5324..df7315fc879 100644
--- a/src/coreclr/vm/siginfo.cpp
+++ b/src/coreclr/vm/siginfo.cpp
@@ -1417,11 +1417,13 @@ TypeHandle SigPointer::GetTypeHandleThrowing(
 
             Instantiation genericLoadInst(thisinst, ntypars);
 
+#ifndef FEATURE_PREJIT // PREJIT doesn't support the volatile update semantics in the interface map that this requires
             if (pMTInterfaceMapOwner != NULL && genericLoadInst.ContainsAllOneType(pMTInterfaceMapOwner))
             {
                 thRet = ClassLoader::LoadTypeDefThrowing(pGenericTypeModule, tkGenericType, ClassLoader::ThrowIfNotFound, ClassLoader::PermitUninstDefOrRef, 0, level);
             }
             else
+#endif // FEATURE_PREJIT
             {
                 // Group together the current signature type context and substitution chain, which
                 // we may later use to instantiate constraints of type arguments that turn out to be