[PATCH v2 01/22] ntdll: Protect module list access in LdrFindEntryForAddress() with SRW lock.

[Paul Gofman]

Fixes racy access to LDR lists in lookup_function_info().

Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
---
 dlls/ntdll/actctx.c |  6 ------
 dlls/ntdll/loader.c | 21 ++++++++++++++++++---
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c
index f9afb655885..bc222dadf27 100644
--- a/dlls/ntdll/actctx.c
+++ b/dlls/ntdll/actctx.c
@@ -705,10 +705,8 @@ static inline const char* debugstr_version(const struct assembly_version *ver)
 static NTSTATUS get_module_filename( HMODULE module, UNICODE_STRING *str, unsigned int extra_len )
 {
     NTSTATUS status;
-    ULONG_PTR magic;
     LDR_DATA_TABLE_ENTRY *pldr;
 
-    LdrLockLoaderLock(0, NULL, &magic);
     status = LdrFindEntryForAddress( module, &pldr );
     if (status == STATUS_SUCCESS)
     {
@@ -721,7 +719,6 @@ static NTSTATUS get_module_filename( HMODULE module, UNICODE_STRING *str, unsign
         }
         else status = STATUS_NO_MEMORY;
     }
-    LdrUnlockLoaderLock(0, magic);
     return status;
 }
 
@@ -3301,12 +3298,10 @@ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class )
     }
     else if (flags & (QUERY_ACTCTX_FLAG_ACTCTX_IS_ADDRESS|QUERY_ACTCTX_FLAG_ACTCTX_IS_HMODULE))
     {
-        ULONG_PTR magic;
         LDR_DATA_TABLE_ENTRY *pldr;
 
         if (!*handle) return STATUS_INVALID_PARAMETER;
 
-        LdrLockLoaderLock( 0, NULL, &magic );
         if (!LdrFindEntryForAddress( *handle, &pldr ))
         {
             if ((flags & QUERY_ACTCTX_FLAG_ACTCTX_IS_HMODULE) && *handle != pldr->DllBase)
@@ -3315,7 +3310,6 @@ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class )
                 *handle = pldr->ActivationContext;
         }
         else status = STATUS_DLL_NOT_FOUND;
-        LdrUnlockLoaderLock( 0, magic );
     }
     else if (!*handle && (class != ActivationContextBasicInformation))
         *handle = process_actctx;
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 797d72aedb6..e363c060ab1 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -176,6 +176,10 @@ static PEB_LDR_DATA ldr =
     { &ldr.InMemoryOrderModuleList, &ldr.InMemoryOrderModuleList },
     { &ldr.InInitializationOrderModuleList, &ldr.InInitializationOrderModuleList }
 };
+/* Ldr lists are modified with loader locked and exclusive ldr_data_srw_lock held.
+ * Taking shared ldr_data_srw_lock to access the data is required outside of
+ * loader lock only. */
+static RTL_SRWLOCK ldr_data_srw_lock = RTL_SRWLOCK_INIT;
 
 static RTL_BITMAP tls_bitmap;
 static RTL_BITMAP tls_expansion_bitmap;
@@ -1245,10 +1249,12 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name
             wm->ldr.EntryPoint = (char *)hModule + nt->OptionalHeader.AddressOfEntryPoint;
     }
 
+    RtlAcquireSRWLockExclusive( &ldr_data_srw_lock );
     InsertTailList(&NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList,
                    &wm->ldr.InLoadOrderLinks);
     InsertTailList(&NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList,
                    &wm->ldr.InMemoryOrderLinks);
+    RtlReleaseSRWLockExclusive( &ldr_data_srw_lock );
     /* wait until init is called for inserting into InInitializationOrderModuleList */
 
     if (!(nt->OptionalHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_NX_COMPAT))
@@ -1454,8 +1460,12 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved )
     }
 
     if (!wm->ldr.InInitializationOrderLinks.Flink)
+    {
+        RtlAcquireSRWLockExclusive( &ldr_data_srw_lock );
         InsertTailList(&NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList,
                 &wm->ldr.InInitializationOrderLinks);
+        RtlReleaseSRWLockExclusive( &ldr_data_srw_lock );
+    }
 
     /* Call DLL entry point */
     if (status == STATUS_SUCCESS)
@@ -1578,13 +1588,14 @@ NTSTATUS WINAPI LdrDisableThreadCalloutsForDll(HMODULE hModule)
 /******************************************************************
  *              LdrFindEntryForAddress (NTDLL.@)
  *
- * The loader_section must be locked while calling this function
  */
 NTSTATUS WINAPI LdrFindEntryForAddress( const void *addr, PLDR_DATA_TABLE_ENTRY *pmod )
 {
+    NTSTATUS ret = STATUS_NO_MORE_ENTRIES;
     PLIST_ENTRY mark, entry;
     PLDR_DATA_TABLE_ENTRY mod;
 
+    RtlAcquireSRWLockShared( &ldr_data_srw_lock );
     mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList;
     for (entry = mark->Flink; entry != mark; entry = entry->Flink)
     {
@@ -1593,10 +1604,12 @@ NTSTATUS WINAPI LdrFindEntryForAddress( const void *addr, PLDR_DATA_TABLE_ENTRY
             (const char *)addr < (char*)mod->DllBase + mod->SizeOfImage)
         {
             *pmod = mod;
-            return STATUS_SUCCESS;
+            ret = STATUS_SUCCESS;
+            break;
         }
     }
-    return STATUS_NO_MORE_ENTRIES;
+    RtlReleaseSRWLockShared( &ldr_data_srw_lock );
+    return ret;
 }
 
 /******************************************************************
@@ -3510,10 +3523,12 @@ void WINAPI LdrShutdownThread(void)
  */
 static void free_modref( WINE_MODREF *wm )
 {
+    RtlAcquireSRWLockExclusive( &ldr_data_srw_lock );
     RemoveEntryList(&wm->ldr.InLoadOrderLinks);
     RemoveEntryList(&wm->ldr.InMemoryOrderLinks);
     if (wm->ldr.InInitializationOrderLinks.Flink)
         RemoveEntryList(&wm->ldr.InInitializationOrderLinks);
+    RtlReleaseSRWLockExclusive( &ldr_data_srw_lock );
 
     TRACE(" unloading %s\n", debugstr_w(wm->ldr.FullDllName.Buffer));
     if (!TRACE_ON(module))
-- 
2.31.1