Fix OOM edge case in interleaved LoaderHeap (#81602)
authorJan Vorlicek <janvorli@microsoft.com>
Fri, 3 Feb 2023 19:20:55 +0000 (20:20 +0100)
committerGitHub <noreply@github.com>
Fri, 3 Feb 2023 19:20:55 +0000 (20:20 +0100)
When the `CommitPages` in `GetMoreCommittedPages` fails, which can
happen due to OOM, the `m_pPtrToEndOfCommittedRegion` was already
updated from the end of the code page to the end of the data page.
So if another allocation request comes to the heap, we think there is
one extra page of space left and end up allocating region that crosses
the code / data page boundary. And later we crash when initializing
precodes that were allocated by this request.

This issue was discovered by a Roslyn CI test that loaded assemblies
into non-collectible AssemblyLoadContext instances and ended up jitting
so many methods that the maximum number of mappings on Linux (65535 by
default) got exceeded and mmap failed.

This change fixes it by moving the `m_pPtrToEndOfCommittedRegion` only
in the success case.

src/coreclr/utilcode/loaderheap.cpp

index 56da111..25fd436 100644 (file)
@@ -1285,11 +1285,12 @@ BOOL UnlockedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize)
 
         size_t unusedRemainder = (size_t)((BYTE*)m_pPtrToEndOfCommittedRegion - m_pAllocPtr);
 
+        PTR_BYTE pCommitBaseAddress = m_pPtrToEndOfCommittedRegion;
         if (IsInterleaved())
         {
             // The end of committed region for interleaved heaps points to the end of the executable
             // page and the data pages goes right after that. So we skip the data page here.
-            m_pPtrToEndOfCommittedRegion += GetOsPageSize();
+            pCommitBaseAddress += GetOsPageSize();
         }
         else
         {
@@ -1307,7 +1308,7 @@ BOOL UnlockedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize)
             dwSizeToCommitPart /= 2;
         }
 
-        if (!CommitPages(m_pPtrToEndOfCommittedRegion, dwSizeToCommitPart))
+        if (!CommitPages(pCommitBaseAddress, dwSizeToCommitPart))
         {
             return FALSE;
         }
@@ -1328,10 +1329,10 @@ BOOL UnlockedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize)
 
             // For interleaved heaps, further allocations will start from the newly committed page as they cannot
             // cross page boundary.
-            m_pAllocPtr = (BYTE*)m_pPtrToEndOfCommittedRegion;
+            m_pAllocPtr = (BYTE*)pCommitBaseAddress;
         }
 
-        m_pPtrToEndOfCommittedRegion += dwSizeToCommitPart;
+        m_pPtrToEndOfCommittedRegion += dwSizeToCommit;
         m_dwTotalAlloc += dwSizeToCommit;
 
         return TRUE;