From 810c4bcb0a29fb5f273a0c2b26b37e4ee431d05c Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 13 Mar 2018 16:07:55 -0700 Subject: [PATCH] Fix two AVs with InProcess (#656) --- src/AspNetCore/Src/applicationinfo.cpp | 42 ++++++------------- src/AspNetCore/Src/dllmain.cpp | 2 +- src/RequestHandler/dllmain.cxx | 3 ++ .../inprocess/inprocessapplication.cpp | 19 +++++---- src/RequestHandler/precomp.hxx | 1 + 5 files changed, 28 insertions(+), 39 deletions(-) diff --git a/src/AspNetCore/Src/applicationinfo.cpp b/src/AspNetCore/Src/applicationinfo.cpp index 18bd99d2d5..71a431add6 100644 --- a/src/AspNetCore/Src/applicationinfo.cpp +++ b/src/AspNetCore/Src/applicationinfo.cpp @@ -183,42 +183,12 @@ APPLICATION_INFO::EnsureApplicationCreated() APPLICATION* pApplication = NULL; STACK_STRU(struFileName, 300); // >MAX_PATH STRU struHostFxrDllLocation; - PWSTR* pwzArgv; - DWORD dwArgCount; if (m_pApplication != NULL) { goto Finished; } - if (m_pConfiguration->QueryHostingModel() == APP_HOSTING_MODEL::HOSTING_IN_PROCESS) - { - if (FAILED(hr = HOSTFXR_UTILITY::GetHostFxrParameters( - g_hEventLog, - m_pConfiguration->QueryProcessPath()->QueryStr(), - m_pConfiguration->QueryApplicationPhysicalPath()->QueryStr(), - m_pConfiguration->QueryArguments()->QueryStr(), - &struHostFxrDllLocation, - &dwArgCount, - &pwzArgv))) - { - goto Finished; - } - - if (FAILED(hr = m_pConfiguration->SetHostFxrFullPath(struHostFxrDllLocation.QueryStr()))) - { - goto Finished; - } - - m_pConfiguration->SetHostFxrArguments(dwArgCount, pwzArgv); - } - - hr = FindRequestHandlerAssembly(); - if (FAILED(hr)) - { - goto Finished; - } - if (m_pApplication == NULL) { AcquireSRWLockExclusive(&m_srwLock); @@ -233,6 +203,18 @@ APPLICATION_INFO::EnsureApplicationCreated() // if (!m_fAppOfflineFound) { + + // Move the request handler check inside of the lock + // such that only one request finds and loads it. + // FindRequestHandlerAssembly obtains a global lock, but after releasing the lock, + // there is a period where we could call + + hr = FindRequestHandlerAssembly(); + if (FAILED(hr)) + { + goto Finished; + } + if (m_pfnAspNetCoreCreateApplication == NULL) { hr = HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION); diff --git a/src/AspNetCore/Src/dllmain.cpp b/src/AspNetCore/Src/dllmain.cpp index 2b08d45b8e..41a4b4bd7c 100644 --- a/src/AspNetCore/Src/dllmain.cpp +++ b/src/AspNetCore/Src/dllmain.cpp @@ -48,7 +48,7 @@ BOOL WINAPI DllMain(HMODULE hModule, DisableThreadLibraryCalls(hModule); break; case DLL_PROCESS_DETACH: - // IIS can cause dll detatch to occur before we receive global notifications + // IIS can cause dll detach to occur before we receive global notifications // For example, when we switch the bitness of the worker process, // this is a bug in IIS. To try to avoid AVs, we will set a global flag g_fInShutdown = TRUE; diff --git a/src/RequestHandler/dllmain.cxx b/src/RequestHandler/dllmain.cxx index 3036600948..3cfdef0798 100644 --- a/src/RequestHandler/dllmain.cxx +++ b/src/RequestHandler/dllmain.cxx @@ -10,6 +10,7 @@ BOOL g_fGlobalInitialize = FALSE; BOOL g_fOutOfProcessInitialize = FALSE; BOOL g_fOutOfProcessInitializeError = FALSE; BOOL g_fWinHttpNonBlockingCallbackAvailable = FALSE; +BOOL g_fProcessDetach = FALSE; DWORD g_OptionalWinHttpFlags = 0; DWORD g_dwAspNetCoreDebugFlags = 0; DWORD g_dwDebugFlags = 0; @@ -263,6 +264,8 @@ BOOL APIENTRY DllMain(HMODULE hModule, DisableThreadLibraryCalls(hModule); InitializeSRWLock(&g_srwLockRH); break; + case DLL_PROCESS_DETACH: + g_fProcessDetach = TRUE; default: break; } diff --git a/src/RequestHandler/inprocess/inprocessapplication.cpp b/src/RequestHandler/inprocess/inprocessapplication.cpp index 0127591ae1..c3fca31b87 100644 --- a/src/RequestHandler/inprocess/inprocessapplication.cpp +++ b/src/RequestHandler/inprocess/inprocessapplication.cpp @@ -77,10 +77,14 @@ IN_PROCESS_APPLICATION::ShutDown() if (!m_fShutdownCalledFromManaged) { - // Initiate a recycle such that another worker process is created to replace this one. - - m_ShutdownHandler(m_ShutdownHandlerContext); - m_ShutdownHandler = NULL; + // We cannot call into managed if the dll is detaching from the process. + // Calling into managed code when the dll is detaching is strictly a bad idea, + // and usually results in an AV saying "The string binding is invalid" + if (!g_fProcessDetach) + { + m_ShutdownHandler(m_ShutdownHandlerContext); + m_ShutdownHandler = NULL; + } ReleaseSRWLockExclusive(&m_srwLock); fLocked = FALSE; @@ -608,7 +612,6 @@ IN_PROCESS_APPLICATION::LoadManagedApplication goto Finished; } - m_hThread = CreateThread( NULL, // default security attributes 0, // default stack size @@ -648,7 +651,7 @@ IN_PROCESS_APPLICATION::LoadManagedApplication // Wait on either the thread to complete or the event to be set dwResult = WaitForMultipleObjects(2, pHandles, FALSE, dwTimeout); - + // It all timed out if (dwResult == WAIT_TIMEOUT) { @@ -759,8 +762,8 @@ IN_PROCESS_APPLICATION::ExecuteApplication( HMODULE hModule; hostfxr_main_fn pProc; - // should be a redudant call here, but we will be safe and call it twice. - // TODO AV here on m_pHostFxrParameters being null + DBG_ASSERT(m_status == APPLICATION_STATUS::STARTING); + hModule = LoadLibraryW(m_pConfig->QueryHostFxrFullPath()); if (hModule == NULL) diff --git a/src/RequestHandler/precomp.hxx b/src/RequestHandler/precomp.hxx index 9e70f88e79..2fa43aac17 100644 --- a/src/RequestHandler/precomp.hxx +++ b/src/RequestHandler/precomp.hxx @@ -111,6 +111,7 @@ extern BOOL g_fWinHttpNonBlockingCallbackAvailable; extern BOOL g_fWebSocketSupported; extern BOOL g_fNsiApiNotSupported; extern BOOL g_fEnableReferenceCountTracing; +extern BOOL g_fProcessDetach; extern DWORD g_dwActiveServerProcesses; extern DWORD g_OptionalWinHttpFlags; extern SRWLOCK g_srwLockRH;