[CVE-2018-8130] [CVE-2018-0946] move allocators to ServerScriptContext, add missing marshalling code

Author: MikeHolman

lib/Backend/NativeCodeGenerator.cpp

@@ -3257,8 +3257,7 @@ NativeCodeGenerator::FreeNativeCodeGenAllocation(void* codeAddress)
 #if PDATA_ENABLED && defined(_WIN32)
         DelayDeletingFunctionTable::Clear();
 #endif
-        ThreadContext * context = this->scriptContext->GetThreadContext();
-        HRESULT hr = JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)codeAddress);
+        HRESULT hr = JITManager::GetJITManager()->FreeAllocation(this->scriptContext->GetRemoteScriptAddr(), (intptr_t)codeAddress);
         JITManager::HandleServerCallResult(hr, RemoteCallType::MemFree);
     }
     else if(this->backgroundAllocators)

lib/Backend/ServerScriptContext.cpp

@@ -26,13 +26,19 @@ ServerScriptContext::ServerScriptContext(ScriptContextDataIDL * contextData, Ser
     m_asmJsInterpreterThunkBufferManager(&m_sourceCodeArena, threadContextInfo->GetThunkPageAllocators(), nullptr, threadContextInfo, _u("Asm.js interpreter thunk buffer"), GetThreadContext()->GetProcessHandle()),
     m_domFastPathHelperMap(nullptr),
     m_moduleRecords(&HeapAllocator::Instance),
+    m_codeGenAlloc(nullptr, nullptr, threadContextInfo, threadContextInfo->GetCodePageAllocators(), threadContextInfo->GetProcessHandle()),
     m_globalThisAddr(0),
 #ifdef PROFILE_EXEC
     m_codeGenProfiler(nullptr),
 #endif
     m_refCount(0),
     m_isClosed(false)
 {
+
+#if !TARGET_64 && _CONTROL_FLOW_GUARD
+    m_codeGenAlloc.canCreatePreReservedSegment = threadContextInfo->CanCreatePreReservedSegment();
+#endif
+
 #ifdef PROFILE_EXEC
     if (Js::Configuration::Global.flags.IsEnabled(Js::ProfileFlag))
     {
@@ -357,7 +363,9 @@ ServerScriptContext::Close()
 {
     Assert(!IsClosed());
     m_isClosed = true;
-    
+
+    m_codeGenAlloc.emitBufferManager.Decommit();
+
 #ifdef STACK_BACK_TRACE
     ServerContextManager::RecordCloseContext(this);
 #endif
@@ -381,6 +389,12 @@ ServerScriptContext::Release()
     }
 }
 
+OOPCodeGenAllocators *
+ServerScriptContext::GetCodeGenAllocators()
+{
+    return &m_codeGenAlloc;
+}
+
 Field(Js::Var)*
 ServerScriptContext::GetModuleExportSlotArrayAddress(uint moduleIndex, uint slotIndex)
 {

lib/Backend/ServerScriptContext.h

@@ -87,6 +87,7 @@ class ServerScriptContext : public ScriptContextInfo
     Js::ScriptContextProfiler *  GetCodeGenProfiler() const;
     ServerThreadContext* GetThreadContext() { return threadContextHolder.threadContextInfo; }
 
+    OOPCodeGenAllocators * GetCodeGenAllocators();
     ArenaAllocator * GetSourceCodeArena();
     void Close();
     void AddRef();
@@ -107,6 +108,8 @@ class ServerScriptContext : public ScriptContextInfo
 
     uint m_refCount;
 
+    OOPCodeGenAllocators m_codeGenAlloc;
+
     bool m_isPRNGSeeded;
     bool m_isClosed;
 #endif

lib/Backend/ServerThreadContext.cpp

@@ -14,24 +14,21 @@ ServerThreadContext::ServerThreadContext(ThreadContextDataIDL* data, ProcessCont
     m_numericPropertyBV(nullptr),
     m_preReservedSectionAllocator(processContext->processHandle),
     m_sectionAllocator(processContext->processHandle),
-    m_thunkPageAllocators(nullptr, /* allocXData */ false, &m_sectionAllocator, nullptr, processContext->processHandle),
     m_codePageAllocators(nullptr, ALLOC_XDATA, &m_sectionAllocator, &m_preReservedSectionAllocator, processContext->processHandle),
+    m_thunkPageAllocators(nullptr, /* allocXData */ false, &m_sectionAllocator, nullptr, processContext->processHandle),
 #if defined(_CONTROL_FLOW_GUARD) && !defined(_M_ARM)
     m_jitThunkEmitter(this, &m_sectionAllocator, processContext->processHandle),
 #endif
-    m_codeGenAlloc(nullptr, nullptr, this, &m_codePageAllocators, processContext->processHandle),
     m_pageAlloc(nullptr, Js::Configuration::Global.flags, PageAllocatorType_BGJIT,
         AutoSystemInfo::Data.IsLowMemoryProcess() ?
         PageAllocator::DefaultLowMaxFreePageCount :
         PageAllocator::DefaultMaxFreePageCount
     ),
-    processContext(processContext)
+    processContext(processContext),
+    m_canCreatePreReservedSegment(data->allowPrereserveAlloc != FALSE)
 {
     m_pid = GetProcessId(processContext->processHandle);
 
-#if !TARGET_64 && _CONTROL_FLOW_GUARD
-    m_codeGenAlloc.canCreatePreReservedSegment = data->allowPrereserveAlloc != FALSE;
-#endif
     m_numericPropertyBV = HeapNew(BVSparse<HeapAllocator>, &HeapAllocator::Instance);
 }
 
@@ -121,22 +118,16 @@ ServerThreadContext::GetThunkPageAllocators()
     return &m_thunkPageAllocators;
 }
 
-CustomHeap::OOPCodePageAllocators *
-ServerThreadContext::GetCodePageAllocators()
-{
-    return &m_codePageAllocators;
-}
-
 SectionAllocWrapper *
 ServerThreadContext::GetSectionAllocator()
 {
     return &m_sectionAllocator;
 }
 
-OOPCodeGenAllocators *
-ServerThreadContext::GetCodeGenAllocators()
+CustomHeap::OOPCodePageAllocators *
+ServerThreadContext::GetCodePageAllocators()
 {
-    return &m_codeGenAlloc;
+    return &m_codePageAllocators;
 }
 
 #if defined(_CONTROL_FLOW_GUARD) && !defined(_M_ARM)
@@ -172,6 +163,12 @@ ServerThreadContext::GetForegroundPageAllocator()
     return &m_pageAlloc;
 }
 
+bool
+ServerThreadContext::CanCreatePreReservedSegment() const
+{
+    return m_canCreatePreReservedSegment;
+}
+
 bool
 ServerThreadContext::IsNumericProperty(Js::PropertyId propertyId)
 {

lib/Backend/ServerThreadContext.h

@@ -55,7 +55,6 @@ class ServerThreadContext : public ThreadContextInfo
     virtual ptrdiff_t GetChakraBaseAddressDifference() const override;
     virtual ptrdiff_t GetCRTBaseAddressDifference() const override;
 
-    OOPCodeGenAllocators * GetCodeGenAllocators();
 #if defined(_CONTROL_FLOW_GUARD) && !defined(_M_ARM)
     OOPJITThunkEmitter * GetJITThunkEmitter();
 #endif
@@ -72,6 +71,7 @@ class ServerThreadContext : public ThreadContextInfo
 
     intptr_t GetRuntimeChakraBaseAddress() const;
     intptr_t GetRuntimeCRTBaseAddress() const;
+    bool CanCreatePreReservedSegment() const;
 
     static intptr_t GetJITCRTBaseAddress();
 
@@ -84,7 +84,6 @@ class ServerThreadContext : public ThreadContextInfo
     SectionAllocWrapper m_sectionAllocator;
     CustomHeap::OOPCodePageAllocators m_thunkPageAllocators;
     CustomHeap::OOPCodePageAllocators  m_codePageAllocators;
-    OOPCodeGenAllocators m_codeGenAlloc;
 #if defined(_CONTROL_FLOW_GUARD) && !defined(_M_ARM)
     OOPJITThunkEmitter m_jitThunkEmitter;
 #endif
@@ -96,5 +95,6 @@ class ServerThreadContext : public ThreadContextInfo
 
     CriticalSection m_cs;
     uint m_refCount;
+    bool m_canCreatePreReservedSegment;
 };
 #endif

lib/JITClient/JITManager.cpp

@@ -586,15 +586,15 @@ JITManager::CloseScriptContext(
 
 HRESULT
 JITManager::FreeAllocation(
-    __in PTHREADCONTEXT_HANDLE threadContextInfoAddress,
+    __in PSCRIPTCONTEXT_HANDLE scriptContextInfoAddress,
     __in intptr_t codeAddress)
 {
     Assert(IsOOPJITEnabled());
 
     HRESULT hr = E_FAIL;
     RpcTryExcept
     {
-        hr = ClientFreeAllocation(m_rpcBindingHandle, threadContextInfoAddress, codeAddress);
+        hr = ClientFreeAllocation(m_rpcBindingHandle, scriptContextInfoAddress, codeAddress);
     }
     RpcExcept(RpcExceptionFilter(RpcExceptionCode()))
     {

lib/JITClient/JITManager.h

@@ -79,7 +79,7 @@ class JITManager
         __in PSCRIPTCONTEXT_HANDLE scriptContextInfoAddress);
 
     HRESULT FreeAllocation(
-        __in PTHREADCONTEXT_HANDLE threadContextInfoAddress,
+        __in PSCRIPTCONTEXT_HANDLE scriptContextInfoAddress,
         __in intptr_t codeAddress);
 
     HRESULT SetIsPRNGSeeded(
@@ -213,7 +213,7 @@ class JITManager
         { Assert(false); return E_FAIL; }
 
     HRESULT FreeAllocation(
-        __in PTHREADCONTEXT_HANDLE threadContextInfoAddress,
+        __in PSCRIPTCONTEXT_HANDLE scriptContextInfoAddress,
         __in intptr_t codeAddress)
         { Assert(false); return E_FAIL; }
 

lib/JITIDL/ChakraJIT.idl

@@ -85,7 +85,7 @@ interface IChakraJIT
 
     HRESULT FreeAllocation(
         [in] handle_t binding,
-        [in] PTHREADCONTEXT_HANDLE threadContextInfoAddress,
+        [in] PSCRIPTCONTEXT_HANDLE scriptContextInfoAddress,
         [in] CHAKRA_PTR codeAddress);
 
     HRESULT NewInterpreterThunkBlock(

lib/JITServer/JITServer.cpp

@@ -676,10 +676,10 @@ ServerIsInterpreterThunkAddr(
 HRESULT
 ServerFreeAllocation(
     /* [in] */ handle_t binding,
-    /* [in] */ __RPC__in PTHREADCONTEXT_HANDLE threadContextInfo,
+    /* [in] */ __RPC__in PSCRIPTCONTEXT_HANDLE scriptContextInfo,
     /* [in] */ intptr_t codeAddress)
 {
-    ServerThreadContext * context = (ServerThreadContext*)DecodePointer(threadContextInfo);
+    ServerScriptContext* context = (ServerScriptContext*)DecodePointer(scriptContextInfo);
 
     if (context == nullptr)
     {
@@ -709,7 +709,7 @@ ServerIsNativeAddr(
 
     *result = false;
 
-    ServerThreadContext * context = (ServerThreadContext*)DecodePointer(threadContextInfo);
+    ServerThreadContext* context = (ServerThreadContext*)DecodePointer(threadContextInfo);
     if (context == nullptr)
     {
         Assert(false);
@@ -850,7 +850,7 @@ ServerRemoteCodeGen(
             nullptr,
             nullptr,
             jitWorkItem->GetPolymorphicInlineCacheInfo(),
-            threadContextInfo->GetCodeGenAllocators(),
+            scriptContextInfo->GetCodeGenAllocators(),
 #if !FLOATVAR
             nullptr, // number allocator
 #endif

lib/Runtime/Base/ScriptContext.cpp

@@ -701,6 +701,13 @@ namespace Js
         }
 #endif
 
+#if ENABLE_NATIVE_CODEGEN
+        if (m_remoteScriptContextAddr)
+        {
+            JITManager::GetJITManager()->CloseScriptContext(m_remoteScriptContextAddr);
+        }
+#endif
+
 #ifdef ENABLE_SCRIPT_PROFILING
         // Stop profiling if present
         DeRegisterProfileProbe(S_OK, nullptr);
@@ -1452,12 +1459,6 @@ namespace Js
 #endif
             }
 
-#if ENABLE_NATIVE_CODEGEN
-            if (m_remoteScriptContextAddr)
-            {
-                JITManager::GetJITManager()->CloseScriptContext(m_remoteScriptContextAddr);
-            }
-#endif
             this->PrintStats();
         }
     }