Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/kernel32/tests/loader.c | 177 +++++++++++++++++++++++++++++++++++ dlls/ntdll/loader.c | 58 ++++++++++-- include/winternl.h | 2 +- 3 files changed, 230 insertions(+), 7 deletions(-)
diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 4f1b11338a6..045338c1289 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -70,6 +70,10 @@ static NTSTATUS (WINAPI *pLdrLockLoaderLock)(ULONG, ULONG *, ULONG_PTR *); static NTSTATUS (WINAPI *pLdrUnlockLoaderLock)(ULONG, ULONG_PTR); static NTSTATUS (WINAPI *pLdrLoadDll)(LPCWSTR,DWORD,const UNICODE_STRING *,HMODULE*); static NTSTATUS (WINAPI *pLdrUnloadDll)(HMODULE); + +typedef void (CALLBACK *LDRENUMPROC)(LDR_DATA_TABLE_ENTRY *, void *, BOOLEAN *); +static NTSTATUS (WINAPI *pLdrEnumerateLoadedModules)( void *unknown, LDRENUMPROC callback, void *context ); + static void (WINAPI *pRtlInitUnicodeString)(PUNICODE_STRING,LPCWSTR); static void (WINAPI *pRtlAcquirePebLock)(void); static void (WINAPI *pRtlReleasePebLock)(void); @@ -4036,6 +4040,177 @@ static void test_Wow64Transition(void) debugstr_wn(name->SectionFileName.Buffer, name->SectionFileName.Length / sizeof(WCHAR))); }
+static BOOL test_loader_lock_repeat_lock; +static unsigned int test_loader_notification_count; +static const WCHAR *ldr_notify_track_dll; +static HANDLE lock_ready_event, next_test_event; +static BOOL test_loader_lock_abort_test; +static volatile LONG test_loader_lock_timeout_count; + +#define BLOCKING_TESTS_ENABLED 0 + +static DWORD WINAPI test_loader_lock_thread(void *param) +{ + ULONG_PTR magic; + NTSTATUS status; + DWORD result; + + WaitForSingleObject(next_test_event, INFINITE); + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + + /* 1. Test with loader lock held. */ + do + { + status = pLdrLockLoaderLock(0, NULL, &magic); + ok(!status, "Got unexpected status %#x.\n", status); + SetEvent(lock_ready_event); + + result = WaitForSingleObject(next_test_event, 3000); + if (result == WAIT_TIMEOUT) + ++test_loader_lock_timeout_count; + + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + + if (result == WAIT_TIMEOUT) + { + WaitForSingleObject(next_test_event, INFINITE); + test_loader_lock_timeout_count = 0; + if (test_loader_lock_abort_test) + return 0; + } + } while (test_loader_lock_repeat_lock); + + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + + SetEvent(lock_ready_event); + return 0; +} + +static void CALLBACK test_loader_lock_ldr_notify(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + if (!test_loader_lock_timeout_count && !lstrcmpW(data->Loaded.BaseDllName->Buffer, ldr_notify_track_dll)) + ++test_loader_notification_count; +} + +static void CALLBACK test_loader_lock_enum_callback(LDR_DATA_TABLE_ENTRY *mod, void *context, BOOLEAN *stop) +{ + *stop = TRUE; +} + +#define check_timeout(expected_timeout) check_timeout_(__LINE__, expected_timeout) +static void check_timeout_(unsigned int line, BOOL expected_timeout) +{ + ok_(__FILE__, line)(!!test_loader_lock_timeout_count == !!expected_timeout, + "Got timeout count %u, expected_timeout %d.\n", + test_loader_lock_timeout_count, expected_timeout); + if (test_loader_lock_timeout_count) + { + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + } +} + +static void test_loader_lock_next_test(void) +{ + /* Exit previous test loop. */ + test_loader_lock_repeat_lock = FALSE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_repeat_lock = TRUE; + + /* Star new loop. */ + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); +} + +static void test_loader_lock(void) +{ + static const WCHAR not_loaded_dll_name[] = L"authz.dll"; + static const WCHAR preloaded_dll_name[] = L"winmm.dll"; + HMODULE hmodule_preloaded, hmodule; + NTSTATUS status; + HANDLE thread; + void *cookie; + BOOL bret; + + lock_ready_event = CreateEventA(NULL, FALSE, FALSE, "test_lock_ready_event"); + next_test_event = CreateEventA(NULL, FALSE, FALSE, "test_next_test_event"); + + thread = CreateThread(NULL, 0, test_loader_lock_thread, NULL, 0, NULL); + + hmodule_preloaded = LoadLibraryW(preloaded_dll_name); + ok(!!hmodule_preloaded, "LoadLibrary failed, err %u.\n", GetLastError()); + hmodule = GetModuleHandleW(not_loaded_dll_name); + ok(!hmodule, "%s is already loaded.\n", not_loaded_dll_name); + + test_loader_notification_count = 0; + + /* 1. Test with loader lock held. */ + trace("Test 1.\n"); + test_loader_lock_next_test(); + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + if (test_loader_lock_timeout_count) + { + test_loader_lock_abort_test = TRUE; + SetEvent(next_test_event); + win_skip("Old loader, tests skipped.\n"); + goto done; + } + check_timeout(FALSE); + ldr_notify_track_dll = not_loaded_dll_name; + + bret = GetModuleHandleExW(0, preloaded_dll_name, &hmodule); + ok(bret, "GetModuleHandleEx failed, err %u.\n", GetLastError()); + ok(hmodule == hmodule_preloaded, "Got unexpected hmodule %p, expected %p.\n", hmodule, hmodule_preloaded); + check_timeout(FALSE); + + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + check_timeout(FALSE); + + ok(!test_loader_notification_count, "Got unexpected test_loader_notification_count %u.\n", + test_loader_notification_count); + + if (BLOCKING_TESTS_ENABLED) + { + /* With loader lock held notification callback is called which should mean that: + * - LDR notifications themselves do not wait on loader lock; + * - The library load goes far enough to call the LDR notification until it blocks on the loader lock. + */ + test_loader_notification_count = 0; + hmodule = LoadLibraryW(not_loaded_dll_name); + todo_wine ok(test_loader_notification_count == 1 + || broken(!test_loader_notification_count) /* before Win10 1607. */, + "Got unexpected notification count %u.\n", test_loader_notification_count); + check_timeout(TRUE); + + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + check_timeout(TRUE); + + pLdrEnumerateLoadedModules(NULL, test_loader_lock_enum_callback, NULL); + check_timeout(TRUE); + } + + LdrUnregisterDllNotification( cookie ); + check_timeout(FALSE); + + test_loader_lock_next_test(); + +done: + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + FreeLibrary(hmodule_preloaded); + CloseHandle(lock_ready_event); + CloseHandle(next_test_event); +} + START_TEST(loader) { int argc; @@ -4060,6 +4235,7 @@ START_TEST(loader) pLdrUnlockLoaderLock = (void *)GetProcAddress(ntdll, "LdrUnlockLoaderLock"); pLdrLoadDll = (void *)GetProcAddress(ntdll, "LdrLoadDll"); pLdrUnloadDll = (void *)GetProcAddress(ntdll, "LdrUnloadDll"); + pLdrEnumerateLoadedModules = (void *)GetProcAddress(ntdll, "LdrEnumerateLoadedModules"); pRtlInitUnicodeString = (void *)GetProcAddress(ntdll, "RtlInitUnicodeString"); pRtlAcquirePebLock = (void *)GetProcAddress(ntdll, "RtlAcquirePebLock"); pRtlReleasePebLock = (void *)GetProcAddress(ntdll, "RtlReleasePebLock"); @@ -4113,6 +4289,7 @@ START_TEST(loader) test_dll_file( "advapi32.dll" ); test_dll_file( "user32.dll" ); test_Wow64Transition(); + test_loader_lock(); /* loader test must be last, it can corrupt the internal loader state on Windows */ test_Loader(); } diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 5ee4215f875..d0d4e5448ed 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -160,6 +160,8 @@ static RTL_CRITICAL_SECTION_DEBUG critsect_debug = }; static RTL_CRITICAL_SECTION loader_section = { &critsect_debug, -1, 0, 0, 0, 0 };
+static RTL_SRWLOCK loader_srw_lock = RTL_SRWLOCK_INIT; + static CRITICAL_SECTION dlldir_section; static CRITICAL_SECTION_DEBUG dlldir_critsect_debug = { @@ -217,24 +219,58 @@ static inline BOOL contains_path( LPCWSTR name ) return ((*name && (name[1] == ':')) || wcschr(name, '/') || wcschr(name, '\')); }
+/************************************************************************* + * inc_recursion_count + * + * Increment thread local internal loader lock recursion count and return the old value. + */ +static ULONG inc_recursion_count(void) +{ + return NtCurrentTeb()->Spare2++; +} + +/************************************************************************* + * dec_recursion_count + * + * Decrement thread local internal loader lock recursion count and return the new value. + */ +static ULONG dec_recursion_count(void) +{ + return --NtCurrentTeb()->Spare2; +} + /************************************************************************* * lock_loader_exclusive * - * Take exclusive loader lock. + * Take exclusive ownership of internal loader lock. + * Recursive locking is allowed. */ static void lock_loader_exclusive(void) { - RtlEnterCriticalSection( &loader_section ); + ULONG recursion_count = inc_recursion_count(); + + TRACE( "recursion_count %u.\n", recursion_count ); + if (!recursion_count && !RtlDllShutdownInProgress()) + RtlAcquireSRWLockExclusive( &loader_srw_lock ); }
/************************************************************************* * unlock_loader * - * Release loader lock. + * Release internal loader lock. */ static void unlock_loader(void) { - RtlLeaveCriticalSection( &loader_section ); + ULONG recursion_count = dec_recursion_count(); + + TRACE( "recursion_count %u.\n", recursion_count ); + + if (RtlDllShutdownInProgress()) return; + + assert( recursion_count != ~0u ); + + if (!recursion_count) + RtlReleaseSRWLockExclusive( &loader_srw_lock ); }
#define RTL_UNLOAD_EVENT_TRACE_NUMBER 64 @@ -494,6 +530,7 @@ static void call_ldr_notifications( ULONG reason, LDR_DATA_TABLE_ENTRY *module ) data.Loaded.DllBase = module->DllBase; data.Loaded.SizeOfImage = module->SizeOfImage;
+ RtlEnterCriticalSection( &loader_section ); RtlEnterCriticalSection( &ldr_notifications_section ); LIST_FOR_EACH_ENTRY_SAFE( notify, notify_next, &ldr_notifications, struct ldr_notification, entry ) { @@ -506,6 +543,7 @@ static void call_ldr_notifications( ULONG reason, LDR_DATA_TABLE_ENTRY *module ) notify->callback, reason, &data, notify->context ); } RtlLeaveCriticalSection( &ldr_notifications_section ); + RtlLeaveCriticalSection( &loader_section ); }
/************************************************************************* @@ -1355,6 +1393,8 @@ static void call_tls_callbacks( HMODULE module, UINT reason ) dir = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_TLS, &dirsize ); if (!dir || !dir->AddressOfCallBacks) return;
+ RtlEnterCriticalSection( &loader_section ); + for (callback = (const PIMAGE_TLS_CALLBACK *)dir->AddressOfCallBacks; *callback; callback++) { TRACE_(relay)("\1Call TLS callback (proc=%p,module=%p,reason=%s,reserved=0)\n", @@ -1373,6 +1413,8 @@ static void call_tls_callbacks( HMODULE module, UINT reason ) TRACE_(relay)("\1Ret TLS callback (proc=%p,module=%p,reason=%s,reserved=0)\n", *callback, module, reason_names[reason] ); } + + RtlLeaveCriticalSection( &loader_section ); }
/************************************************************************* @@ -1405,6 +1447,8 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, UINT reason, LPVOID lpReserved else TRACE("(%p %s,%s,%p) - CALL\n", module, debugstr_w(wm->ldr.BaseDllName.Buffer), reason_names[reason], lpReserved );
+ RtlEnterCriticalSection( &loader_section ); + __TRY { retv = call_dll_entry_point( entry, module, reason, lpReserved ); @@ -1419,6 +1463,8 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, UINT reason, LPVOID lpReserved } __ENDTRY
+ RtlLeaveCriticalSection( &loader_section ); + /* The state of the module list may have changed due to the call to the dll. We cannot assume that this module has not been deleted. */ @@ -1658,7 +1704,7 @@ NTSTATUS WINAPI LdrEnumerateLoadedModules( void *unknown, LDRENUMPROC callback, return STATUS_INVALID_PARAMETER;
lock_loader_exclusive(); - + RtlEnterCriticalSection( &loader_section ); mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; for (entry = mark->Flink; entry != mark; entry = entry->Flink) { @@ -1666,7 +1712,7 @@ NTSTATUS WINAPI LdrEnumerateLoadedModules( void *unknown, LDRENUMPROC callback, callback( mod, context, &stop ); if (stop) break; } - + RtlLeaveCriticalSection( &loader_section ); unlock_loader(); return STATUS_SUCCESS; } diff --git a/include/winternl.h b/include/winternl.h index b6f93c116d8..521454396b4 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -466,7 +466,7 @@ typedef struct _TEB PVOID Instrumentation[16]; /* f2c/16b8 */ PVOID WinSockData; /* f6c/1738 */ ULONG GdiBatchCount; /* f70/1740 */ - ULONG Spare2; /* f74/1744 */ + ULONG Spare2; /* f74/1744 used for ntdll loader data in Wine */ ULONG GuaranteedStackBytes; /* f78/1748 */ PVOID ReservedForPerf; /* f7c/1750 */ PVOID ReservedForOle; /* f80/1758 */