Signed-off-by: Paul Gofman pgofman@codeweavers.com --- Looks like the loader functions which do not involve actual loading or unloading DLL are not taking loader lock since Windows 8. That is the case even for LdrAddRefDll and LdrUnloadDll (at least if the latter does not result in actual dll unload).
dlls/ntdll/loader.c | 56 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 9 deletions(-)
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 368448c9f8d..15690524b63 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -147,6 +147,23 @@ static RTL_CRITICAL_SECTION_DEBUG critsect_debug = }; static RTL_CRITICAL_SECTION loader_section = { &critsect_debug, -1, 0, 0, 0, 0 };
+/* ldr_data_section allows for read only access to module linked lists in PEB and + * the underlying structures without taking loader lock. The relations between + * ldr_data_section and loader_sections are: + * - modification to the module linked lists is done with both sections locked + * (loader section to be always locked first); + * - read only access is allowed with either section locked; + * - in case of taking one section only for the read access the lock scope should + * include the access to the underlying structures. */ +static CRITICAL_SECTION ldr_data_section; +static CRITICAL_SECTION_DEBUG ldr_data_section_debug = +{ + 0, 0, &ldr_data_section, + { &ldr_data_section_debug.ProcessLocksList, &ldr_data_section_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": ldr_data_section") } +}; +static CRITICAL_SECTION ldr_data_section = { &ldr_data_section_debug, -1, 0, 0, 0, 0 }; + static CRITICAL_SECTION dlldir_section; static CRITICAL_SECTION_DEBUG dlldir_critsect_debug = { @@ -181,6 +198,16 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY static FARPROC find_named_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, const char *name, int hint, LPCWSTR load_path );
+static inline void lock_ldr_data(void) +{ + RtlEnterCriticalSection( &ldr_data_section ); +} + +static inline void unlock_ldr_data(void) +{ + RtlLeaveCriticalSection( &ldr_data_section ); +} + /* convert PE image VirtualAddress to Real Address */ static inline void *get_rva( HMODULE module, DWORD va ) { @@ -466,7 +493,7 @@ static void call_ldr_notifications( ULONG reason, LDR_DATA_TABLE_ENTRY *module ) * get_modref * * Looks for the referenced HMODULE in the current process - * The loader_section must be locked while calling this function. + * The ldr_data_section or loader_section must be locked while calling this function. */ static WINE_MODREF *get_modref( HMODULE hmod ) { @@ -490,7 +517,7 @@ static WINE_MODREF *get_modref( HMODULE hmod ) * find_basename_module * * Find a module from its base name. - * The loader_section must be locked while calling this function + * The ldr_data_section or loader_section must be locked while calling this function */ static WINE_MODREF *find_basename_module( LPCWSTR name ) { @@ -520,7 +547,7 @@ static WINE_MODREF *find_basename_module( LPCWSTR name ) * find_fullname_module * * Find a module from its full path name. - * The loader_section must be locked while calling this function + * The ldr_data_section or loader_section must be locked while calling this function */ static WINE_MODREF *find_fullname_module( const UNICODE_STRING *nt_name ) { @@ -552,7 +579,7 @@ static WINE_MODREF *find_fullname_module( const UNICODE_STRING *nt_name ) * find_fileid_module * * Find a module from its file id. - * The loader_section must be locked while calling this function + * The ldr_data_section or loader_section must be locked while calling this function */ static WINE_MODREF *find_fileid_module( const struct file_id *id ) { @@ -601,7 +628,7 @@ static WINE_MODREF **grow_module_deps( WINE_MODREF *wm, int count ) * find_forwarded_export * * Find the final function pointer for a forwarded function. - * The loader_section must be locked while calling this function. + * The ldr_data_section or loader_section must be locked while calling this function. */ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWSTR load_path ) { @@ -673,7 +700,7 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS * * Find an exported function by ordinal. * The exports base must have been subtracted from the ordinal already. - * The loader_section must be locked while calling this function. + * The ldr_data_section or loader_section must be locked while calling this function. */ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, DWORD ordinal, LPCWSTR load_path ) @@ -713,7 +740,7 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY * find_named_export * * Find an exported function by name. - * The loader_section must be locked while calling this function. + * The ldr_data_section or loader_section must be locked while calling this function. */ static FARPROC find_named_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, const char *name, int hint, LPCWSTR load_path ) @@ -1199,11 +1226,13 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name wm->ldr.EntryPoint = (char *)hModule + nt->OptionalHeader.AddressOfEntryPoint; }
+ lock_ldr_data(); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList, &wm->ldr.InLoadOrderLinks); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList, &wm->ldr.InMemoryOrderLinks); /* wait until init is called for inserting into InInitializationOrderModuleList */ + unlock_ldr_data();
if (!(nt->OptionalHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) { @@ -1410,8 +1439,12 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved ) }
if (!wm->ldr.InInitializationOrderLinks.Flink) + { + lock_ldr_data(); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList, &wm->ldr.InInitializationOrderLinks); + unlock_ldr_data(); + }
/* Call DLL entry point */ if (status == STATUS_SUCCESS) @@ -1908,8 +1941,10 @@ static NTSTATUS build_module( LPCWSTR load_path, const UNICODE_STRING *nt_name, if (status != STATUS_SUCCESS) { /* the module has only be inserted in the load & memory order lists */ + lock_ldr_data(); RemoveEntryList(&wm->ldr.InLoadOrderLinks); RemoveEntryList(&wm->ldr.InMemoryOrderLinks); + unlock_ldr_data();
/* FIXME: there are several more dangling references * left. Including dlls loaded by this dll before the @@ -3276,10 +3311,13 @@ void WINAPI LdrShutdownThread(void) */ static void free_modref( WINE_MODREF *wm ) { + lock_ldr_data(); RemoveEntryList(&wm->ldr.InLoadOrderLinks); RemoveEntryList(&wm->ldr.InMemoryOrderLinks); if (wm->ldr.InInitializationOrderLinks.Flink) RemoveEntryList(&wm->ldr.InInitializationOrderLinks); + if (cached_modref == wm) cached_modref = NULL; + unlock_ldr_data();
TRACE(" unloading %s\n", debugstr_w(wm->ldr.FullDllName.Buffer)); if (!TRACE_ON(module)) @@ -3298,7 +3336,6 @@ static void free_modref( WINE_MODREF *wm ) RtlReleaseActivationContext( wm->ldr.ActivationContext ); unix_funcs->unload_builtin_dll( wm->ldr.DllBase ); NtUnmapViewOfSection( NtCurrentProcess(), wm->ldr.DllBase ); - if (cached_modref == wm) cached_modref = NULL; RtlFreeUnicodeString( &wm->ldr.FullDllName ); RtlFreeHeap( GetProcessHeap(), 0, wm->deps ); RtlFreeHeap( GetProcessHeap(), 0, wm ); @@ -4125,7 +4162,8 @@ static NTSTATUS process_init(void) } #endif
- /* the main exe needs to be the first in the load order list */ + /* the main exe needs to be the first in the load order list. + * ldr_data_section locking is redundant here. */ RemoveEntryList( &wm->ldr.InLoadOrderLinks ); InsertHeadList( &peb->LdrData->InLoadOrderModuleList, &wm->ldr.InLoadOrderLinks ); RemoveEntryList( &wm->ldr.InMemoryOrderLinks );