Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Bug 699134 - Protect against reentrancy in the LdrLoadDll hook, with …

…less XPCOM stringery and more locking. a=bsmedberg ON A CLOSED TREE

--HG--
branch : GECKO80_2011110416_RELBRANCH
  • Loading branch information...
commit 601cfe66c412c861a780ce4dc26b3e76d06633c0 1 parent 2d77d28
Benjamin Smedberg authored
Showing with 70 additions and 0 deletions.
  1. +70 −0 toolkit/xre/nsWindowsDllBlocklist.cpp
View
70 toolkit/xre/nsWindowsDllBlocklist.cpp
@@ -39,6 +39,9 @@
#include <winternl.h>
#include <stdio.h>
+#include <string.h>
+
+#include <map>
#ifdef XRE_WANT_DLL_BLOCKLIST
#define XRE_SetupDllBlocklist SetupDllBlocklist
@@ -62,10 +65,68 @@
// define this for very verbose dll load debug spew
#undef DEBUG_very_verbose
+namespace {
+
typedef NTSTATUS (NTAPI *LdrLoadDll_func) (PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileName, PHANDLE handle);
static LdrLoadDll_func stub_LdrLoadDll = 0;
+/**
+ * Some versions of Windows call LoadLibraryEx to get the version information
+ * for a DLL, which causes our patched LdrLoadDll implementation to re-enter
+ * itself and cause infinite recursion and a stack-exhaustion crash. We protect
+ * against reentrancy by allowing recursive loads of the same DLL.
+ *
+ * Note that we don't use __declspec(thread) because that doesn't work in DLLs
+ * loaded via LoadLibrary and there can be a limited number of TLS slots, so
+ * we roll our own.
+ */
+class ReentrancySentinel
+{
+public:
+ explicit ReentrancySentinel(const char* dllName)
+ {
+ DWORD currentThreadId = GetCurrentThreadId();
+ EnterCriticalSection(&sLock);
+ mPreviousDllName = (*sThreadMap)[currentThreadId];
+
+ // If there is a DLL currently being loaded and it has the same name
+ // as the current attempt, we're re-entering.
+ mReentered = mPreviousDllName && !stricmp(mPreviousDllName, dllName);
+ (*sThreadMap)[currentThreadId] = dllName;
+ LeaveCriticalSection(&sLock);
+ }
+
+ ~ReentrancySentinel()
+ {
+ DWORD currentThreadId = GetCurrentThreadId();
+ EnterCriticalSection(&sLock);
+ (*sThreadMap)[currentThreadId] = mPreviousDllName;
+ LeaveCriticalSection(&sLock);
+ }
+
+ bool BailOut() const
+ {
+ return mReentered;
+ };
+
+ static void InitializeStatics()
+ {
+ InitializeCriticalSection(&sLock);
+ sThreadMap = new std::map<DWORD, const char*>;
+ }
+
+private:
+ static CRITICAL_SECTION sLock;
+ static std::map<DWORD, const char*>* sThreadMap;
+
+ const char* mPreviousDllName;
+ bool mReentered;
+};
+
+CRITICAL_SECTION ReentrancySentinel::sLock;
+std::map<DWORD, const char*>* ReentrancySentinel::sThreadMap;
+
static NTSTATUS NTAPI
patched_LdrLoadDll (PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileName, PHANDLE handle)
{
@@ -153,6 +214,11 @@ patched_LdrLoadDll (PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileNam
#endif
if (info->maxVersion != ALL_VERSIONS) {
+ ReentrancySentinel sentinel(dllName);
+ if (sentinel.BailOut()) {
+ goto continue_loading;
+ }
+
// In Windows 8, the first parameter seems to be used for more than just the
// path name. For example, its numerical value can be 1. Passing a non-valid
// pointer to SearchPathW will cause a crash, so we need to check to see if we
@@ -221,11 +287,15 @@ patched_LdrLoadDll (PWCHAR filePath, PULONG flags, PUNICODE_STRING moduleFileNam
WindowsDllInterceptor NtDllIntercept;
+} // anonymous namespace
+
void
XRE_SetupDllBlocklist()
{
NtDllIntercept.Init("ntdll.dll");
+ ReentrancySentinel::InitializeStatics();
+
bool ok = NtDllIntercept.AddHook("LdrLoadDll", reinterpret_cast<intptr_t>(patched_LdrLoadDll), (void**) &stub_LdrLoadDll);
#ifdef DEBUG
Please sign in to comment.
Something went wrong with that request. Please try again.