[PATCH 1/4] ntdll: Introduce ldr_data_section CS and lock it for loader structures modifications.
Signed-off-by: Paul Gofman <pgofman(a)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 ); -- 2.28.0
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/loader.c | 70 ++++++++++++++++++++++++++++++++++++ dlls/ntdll/loader.c | 4 +-- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 67fd62ef6aa..5d8991bf0f0 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -70,6 +70,7 @@ 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); +static NTSTATUS (WINAPI *pLdrGetProcedureAddress)(HMODULE, const ANSI_STRING *, ULONG, void **); static void (WINAPI *pRtlInitUnicodeString)(PUNICODE_STRING,LPCWSTR); static void (WINAPI *pRtlAcquirePebLock)(void); static void (WINAPI *pRtlReleasePebLock)(void); @@ -3992,6 +3993,73 @@ static void test_LoadPackagedLibrary(void) h, GetLastError()); } +static HANDLE test_loader_lock_event, test_loader_lock_test_done_event; + +static DWORD WINAPI test_loader_lock_thread(void *param) +{ + NTSTATUS status; + ULONG_PTR magic; + DWORD ret; + + status = pLdrLockLoaderLock(0, NULL, &magic); + ok(!status, "Got unexpected status %#x.\n", status); + pRtlAcquirePebLock(); + SetEvent(test_loader_lock_event); + + ret = WaitForSingleObject(test_loader_lock_test_done_event, 2000); + ok(ret == WAIT_OBJECT_0 || broken(ret == WAIT_TIMEOUT) /* before Win8 */, + "Got unexpected ret %#x.\n", ret); + pRtlReleasePebLock(); + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + + return 0; +} + +static void test_loader_lock_scope(void) +{ + ANSI_STRING name; + HMODULE hmodule; + ULONG_PTR magic; + NTSTATUS status; + HANDLE hthread; + void *address; + ULONG result; + DWORD ret; + + if (!pRtlAcquirePebLock || !pLdrLockLoaderLock) + { + win_skip("RtlAcquirePebLock or LdrLockLoaderLock is not available.\n"); + return; + } + + test_loader_lock_event = CreateEventA(NULL, FALSE, FALSE, NULL); + test_loader_lock_test_done_event = CreateEventA(NULL, FALSE, FALSE, NULL); + + hmodule = GetModuleHandleA("ntdll.dll"); + ok(!!hmodule, "Got NULL hmodule.\n"); + + hthread = CreateThread(NULL, 0, test_loader_lock_thread, NULL, 0, NULL); + ok(!!hthread, "Thread creation failed.\n"); + + ret = WaitForSingleObject(test_loader_lock_event, INFINITE); + ok(ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret); + + result = 0xdeadbeef; + status = pLdrLockLoaderLock(2, &result, &magic); + ok(!status && result == 2, "Got unexpected status %#x, result %#x,\n", status, result); + + RtlInitAnsiString(&name, "LdrLockLoaderLock"); + address = (void *)0xdeadbeef; + /* Locks up on loader lock before Win7. */ + status = pLdrGetProcedureAddress(hmodule, &name, 0, &address); + ok(!status && address == pLdrLockLoaderLock, "Got unexpected status %#x, address %p.\n", status, address); + + SetEvent(test_loader_lock_test_done_event); + ret = WaitForSingleObject(hthread, INFINITE); + ok(ret == WAIT_OBJECT_0, "Got unexpected ret %#x.\n", ret); +} + START_TEST(loader) { int argc; @@ -4016,6 +4084,7 @@ START_TEST(loader) pLdrUnlockLoaderLock = (void *)GetProcAddress(ntdll, "LdrUnlockLoaderLock"); pLdrLoadDll = (void *)GetProcAddress(ntdll, "LdrLoadDll"); pLdrUnloadDll = (void *)GetProcAddress(ntdll, "LdrUnloadDll"); + pLdrGetProcedureAddress = (void *)GetProcAddress(ntdll, "LdrGetProcedureAddress"); pRtlInitUnicodeString = (void *)GetProcAddress(ntdll, "RtlInitUnicodeString"); pRtlAcquirePebLock = (void *)GetProcAddress(ntdll, "RtlAcquirePebLock"); pRtlReleasePebLock = (void *)GetProcAddress(ntdll, "RtlReleasePebLock"); @@ -4068,6 +4137,7 @@ START_TEST(loader) test_dll_file( "kernel32.dll" ); test_dll_file( "advapi32.dll" ); test_dll_file( "user32.dll" ); + test_loader_lock_scope(); /* 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 15690524b63..9048885d803 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1752,7 +1752,7 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name, DWORD exp_size; NTSTATUS ret = STATUS_PROCEDURE_NOT_FOUND; - RtlEnterCriticalSection( &loader_section ); + lock_ldr_data(); /* check if the module itself is invalid to return the proper error */ if (!get_modref( module )) ret = STATUS_DLL_NOT_FOUND; @@ -1769,7 +1769,7 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name, } } - RtlLeaveCriticalSection( &loader_section ); + unlock_ldr_data(); return ret; } -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=81270 Your paranoid android. === w1064v1809 (64 bit report) === kernel32: loader.c:709: Test failed: 1197: wrong status c000011b/c0000130 loader.c:709: Test failed: 1202: wrong status c000011b/c0000130 loader.c:709: Test failed: 1207: wrong status c000011b/c000007b loader.c:709: Test failed: 1212: wrong status c000011b/c000007b loader.c:709: Test failed: 1217: wrong status c000011b/c000007b loader.c:709: Test failed: 1222: wrong status c000011b/c000007b loader.c:709: Test failed: 1227: wrong status c000011b/c000007b loader.c:709: Test failed: 1243: wrong status c000011b/0 loader.c:709: Test failed: 1247: wrong status c000011b/0 loader.c:709: Test failed: 1252: wrong status c000011b/0 loader.c:709: Test failed: 1256: wrong status c000011b/0 loader.c:709: Test failed: 1260: wrong status c000011b/0 === w10pro64_ja (64 bit report) === kernel32: loader.c:709: Test failed: 1175: wrong status c000012f/c000011b loader.c:709: Test failed: 1180: wrong status c000012f/c000011b loader.c:709: Test failed: 1180: wrong status c000012f/c000011b loader.c:709: Test failed: 1180: wrong status c000012f/c0000131 loader.c:709: Test failed: 1180: wrong status c000012f/c000011b === debiant (32 bit report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit French report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit Japanese:Japan report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit Chinese:China report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit WoW report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 ntdll: threadpool.c:1904: Test failed: WaitForSingleObject returned 258 === debiant (64 bit WoW report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f780 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b30 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/loader.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 9048885d803..66956c3d373 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2255,7 +2255,8 @@ static NTSTATUS get_dll_load_path_search_flags( LPCWSTR module, DWORD flags, WCH * Open a file for a new dll. Helper for find_dll_file. */ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void **module, - SECTION_IMAGE_INFORMATION *image_info, struct file_id *id ) + SECTION_IMAGE_INFORMATION *image_info, struct file_id *id, + BOOL loaded_only ) { FILE_BASIC_INFORMATION info; OBJECT_ATTRIBUTES attr; @@ -2308,6 +2309,14 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void } } + if (loaded_only) + { + NtClose( handle ); + NtUnmapViewOfSection( NtCurrentProcess(), *module ); + *module = NULL; + return STATUS_DLL_NOT_FOUND; + } + size.QuadPart = 0; status = NtCreateSection( &mapping, STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_EXECUTE, @@ -2532,7 +2541,7 @@ done: */ static NTSTATUS search_dll_file( LPCWSTR paths, LPCWSTR search, UNICODE_STRING *nt_name, WINE_MODREF **pwm, void **module, SECTION_IMAGE_INFORMATION *image_info, - struct file_id *id ) + struct file_id *id, BOOL loaded_only ) { WCHAR *name; BOOL found_image = FALSE; @@ -2559,7 +2568,7 @@ static NTSTATUS search_dll_file( LPCWSTR paths, LPCWSTR search, UNICODE_STRING * nt_name->Buffer = NULL; if ((status = RtlDosPathNameToNtPathName_U_WithStatus( name, nt_name, NULL, NULL ))) goto done; - status = open_dll_file( nt_name, pwm, module, image_info, id ); + status = open_dll_file( nt_name, pwm, module, image_info, id, loaded_only ); if (status == STATUS_IMAGE_MACHINE_TYPE_MISMATCH) found_image = TRUE; else if (status != STATUS_DLL_NOT_FOUND) goto done; RtlFreeUnicodeString( nt_name ); @@ -2588,7 +2597,8 @@ done: */ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, const WCHAR *default_ext, UNICODE_STRING *nt_name, WINE_MODREF **pwm, void **module, - SECTION_IMAGE_INFORMATION *image_info, struct file_id *id ) + SECTION_IMAGE_INFORMATION *image_info, struct file_id *id, + BOOL loaded_only ) { WCHAR *ext, *dllname; NTSTATUS status; @@ -2639,9 +2649,9 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, con } if (RtlDetermineDosPathNameType_U( libname ) == RELATIVE_PATH) - status = search_dll_file( load_path, libname, nt_name, pwm, module, image_info, id ); + status = search_dll_file( load_path, libname, nt_name, pwm, module, image_info, id, loaded_only ); else if (!(status = RtlDosPathNameToNtPathName_U_WithStatus( libname, nt_name, NULL, NULL ))) - status = open_dll_file( nt_name, pwm, module, image_info, id ); + status = open_dll_file( nt_name, pwm, module, image_info, id, loaded_only ); if (status == STATUS_IMAGE_MACHINE_TYPE_MISMATCH) status = STATUS_INVALID_IMAGE_FORMAT; @@ -2671,7 +2681,7 @@ static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, const WC TRACE( "looking for %s in %s\n", debugstr_w(libname), debugstr_w(load_path) ); - nts = find_dll_file( load_path, libname, default_ext, &nt_name, pwm, &module, &image_info, &id ); + nts = find_dll_file( load_path, libname, default_ext, &nt_name, pwm, &module, &image_info, &id, FALSE ); if (*pwm) /* found already loaded module */ { @@ -2749,7 +2759,7 @@ static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, const WC LdrUnloadDll( (*pwm)->ldr.DllBase ); nts = STATUS_DLL_NOT_FOUND; /* map the dll again if it was unmapped */ - if (!module && open_dll_file( &nt_name, pwm, &module, &image_info, &id )) break; + if (!module && open_dll_file( &nt_name, pwm, &module, &image_info, &id, FALSE )) break; } if (nts == STATUS_DLL_NOT_FOUND) nts = load_native_dll( load_path, &nt_name, &module, &image_info, &id, flags, pwm ); @@ -2860,14 +2870,10 @@ NTSTATUS WINAPI LdrGetDllHandle( LPCWSTR load_path, ULONG flags, const UNICODE_S if (!load_path) load_path = NtCurrentTeb()->Peb->ProcessParameters->DllPath.Buffer; - status = find_dll_file( load_path, name->Buffer, dllW, &nt_name, &wm, &module, &image_info, &id ); + if (!(status = find_dll_file( load_path, name->Buffer, dllW, + &nt_name, &wm, &module, &image_info, &id, TRUE ))) + *base = wm->ldr.DllBase; - if (wm) *base = wm->ldr.DllBase; - else - { - if (status == STATUS_SUCCESS) NtUnmapViewOfSection( NtCurrentProcess(), module ); - status = STATUS_DLL_NOT_FOUND; - } RtlFreeUnicodeString( &nt_name ); RtlLeaveCriticalSection( &loader_section ); -- 2.28.0
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/loader.c | 5 ++--- dlls/ntdll/loader.c | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 5d8991bf0f0..8b4be58036d 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -4036,9 +4036,6 @@ static void test_loader_lock_scope(void) test_loader_lock_event = CreateEventA(NULL, FALSE, FALSE, NULL); test_loader_lock_test_done_event = CreateEventA(NULL, FALSE, FALSE, NULL); - hmodule = GetModuleHandleA("ntdll.dll"); - ok(!!hmodule, "Got NULL hmodule.\n"); - hthread = CreateThread(NULL, 0, test_loader_lock_thread, NULL, 0, NULL); ok(!!hthread, "Thread creation failed.\n"); @@ -4052,6 +4049,8 @@ static void test_loader_lock_scope(void) RtlInitAnsiString(&name, "LdrLockLoaderLock"); address = (void *)0xdeadbeef; /* Locks up on loader lock before Win7. */ + hmodule = GetModuleHandleA("ntdll.dll"); + ok(!!hmodule, "Got NULL hmodule.\n"); status = pLdrGetProcedureAddress(hmodule, &name, 0, &address); ok(!status && address == pLdrLockLoaderLock, "Got unexpected status %#x, address %p.\n", status, address); diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 66956c3d373..443cf49e10b 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NTSTATUS status; HANDLE handle, mapping; + if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fullname_module( nt_name ))) { NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */ return STATUS_SUCCESS; } + if (loaded_only) + unlock_ldr_data(); attr.Length = sizeof(attr); attr.RootDirectory = 0; @@ -2298,6 +2303,8 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void if (!NtFsControlFile( handle, 0, NULL, NULL, &io, FSCTL_GET_OBJECT_ID, NULL, 0, &fid, sizeof(fid) )) { memcpy( id, fid.ObjectId, sizeof(*id) ); + if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fileid_module( id ))) { TRACE( "%s is the same file as existing module %p %s\n", debugstr_w( nt_name->Buffer ), @@ -2305,8 +2312,11 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NtClose( handle ); NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */ return STATUS_SUCCESS; } + if (loaded_only) + unlock_ldr_data(); } if (loaded_only) @@ -2640,11 +2650,17 @@ static NTSTATUS find_dll_file( const WCHAR *load_path, const WCHAR *libname, con else { if (status != STATUS_SXS_KEY_NOT_FOUND) goto done; + + if (loaded_only) + lock_ldr_data(); if ((*pwm = find_basename_module( libname )) != NULL) { status = STATUS_SUCCESS; + /* ldr_data_section to be unlocked by the caller. */ goto done; } + if (loaded_only) + unlock_ldr_data(); } } @@ -2866,17 +2882,16 @@ NTSTATUS WINAPI LdrGetDllHandle( LPCWSTR load_path, ULONG flags, const UNICODE_S SECTION_IMAGE_INFORMATION image_info; struct file_id id; - RtlEnterCriticalSection( &loader_section ); - if (!load_path) load_path = NtCurrentTeb()->Peb->ProcessParameters->DllPath.Buffer; if (!(status = find_dll_file( load_path, name->Buffer, dllW, &nt_name, &wm, &module, &image_info, &id, TRUE ))) + { *base = wm->ldr.DllBase; - + unlock_ldr_data(); + } RtlFreeUnicodeString( &nt_name ); - RtlLeaveCriticalSection( &loader_section ); TRACE( "%s -> %p (load path %s)\n", debugstr_us(name), status ? NULL : *base, debugstr_w(load_path) ); return status; } -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=81272 Your paranoid android. === w10pro64 (32 bit report) === kernel32: loader.c:2799: Test failed: attached thread count should be 2 === w10pro64_2scr (32 bit report) === kernel32: loader.c:2799: Test failed: attached thread count should be 2 loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b loader.c:709: Test failed: 1180: wrong status c0000130/c000011b === w1064v1809 (64 bit report) === kernel32: loader.c:705: Test failed: 1427: got test dll but expected fallback loader.c:705: Test failed: 1433: got test dll but expected fallback loader.c:705: Test failed: 1439: got test dll but expected fallback loader.c:705: Test failed: 1446: got test dll but expected fallback loader.c:705: Test failed: 1473: got test dll but expected fallback === debiant (32 bit report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit French report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit Japanese:Japan report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (32 bit Chinese:China report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 ntdll: threadpool.c:1904: Test failed: WaitForSingleObject returned 258 === debiant (32 bit WoW report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15 === debiant (64 bit WoW report) === kernel32: loader.c:3965: Test failed: ntdll.dll:0: wrong OptionalHeader.AddressOfEntryPoint 5f830 / 5f730 loader.c:3969: Test failed: ntdll.dll:0: wrong OptionalHeader.DataDirectory[i].VirtualAddress 75000 / 74000 loader.c:3969: Test failed: ntdll.dll:1: wrong OptionalHeader.DataDirectory[i].VirtualAddress 89000 / 88000 loader.c:3969: Test failed: ntdll.dll:2: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8a000 / 89000 loader.c:3969: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].VirtualAddress 8b000 / 8a000 loader.c:3970: Test failed: ntdll.dll:5: wrong OptionalHeader.DataDirectory[i].Size 3b44 / 3b10 loader.c:3975: Test failed: ntdll.dll: wrong section 0 loader.c:3975: Test failed: ntdll.dll: wrong section 1 loader.c:3975: Test failed: ntdll.dll: wrong section 3 loader.c:3975: Test failed: ntdll.dll: wrong section 4 loader.c:3975: Test failed: ntdll.dll: wrong section 5 loader.c:3975: Test failed: ntdll.dll: wrong section 6 loader.c:3975: Test failed: ntdll.dll: wrong section 7 loader.c:3975: Test failed: ntdll.dll: wrong section 8 loader.c:3975: Test failed: ntdll.dll: wrong section 9 loader.c:3975: Test failed: ntdll.dll: wrong section 10 loader.c:3975: Test failed: ntdll.dll: wrong section 11 loader.c:3975: Test failed: ntdll.dll: wrong section 12 loader.c:3975: Test failed: ntdll.dll: wrong section 13 loader.c:3975: Test failed: ntdll.dll: wrong section 14 loader.c:3975: Test failed: ntdll.dll: wrong section 15
Paul Gofman <pgofman(a)codeweavers.com> writes:
@@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NTSTATUS status; HANDLE handle, mapping;
+ if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fullname_module( nt_name ))) { NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */
Not releasing the lock in the same scope is ugly and fragile, please try to find a better way. -- Alexandre Julliard julliard(a)winehq.org
On 11/4/20 19:20, Alexandre Julliard wrote:
Paul Gofman <pgofman(a)codeweavers.com> writes:
@@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NTSTATUS status; HANDLE handle, mapping;
+ if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fullname_module( nt_name ))) { NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */ Not releasing the lock in the same scope is ugly and fragile, please try to find a better way.
The straightforward alternative would be to factor out the dll finding parts and use just them in LdrGetDllHandle(). But looking at further removing loader lock from functions like LdrAddRefDll and LdrUnloadDll it seems that it will be necessary to introduce a per-module lock (presumably with SRW lock using Lock field in struct _LDR_DATA_TABLE_ENTRY). And probably keeping locks / unlocks contained in the same functions for that one will be much harder. Will it work if I introduce this per-module lock before this patch and use a concept of getting and putting module reference? That is, lock / unlock of ldr_data will always happen in the same function, but if the module reference is returned it will always have the lock on it and the caller responsibility will be to unlock the module reference (pretty much similar to how it is now done in wineserver WRT to getting and putting object references)?
Paul Gofman <pgofman(a)codeweavers.com> writes:
On 11/4/20 19:20, Alexandre Julliard wrote:
Paul Gofman <pgofman(a)codeweavers.com> writes:
@@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NTSTATUS status; HANDLE handle, mapping;
+ if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fullname_module( nt_name ))) { NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */ Not releasing the lock in the same scope is ugly and fragile, please try to find a better way.
The straightforward alternative would be to factor out the dll finding parts and use just them in LdrGetDllHandle().
But looking at further removing loader lock from functions like LdrAddRefDll and LdrUnloadDll it seems that it will be necessary to introduce a per-module lock (presumably with SRW lock using Lock field in struct _LDR_DATA_TABLE_ENTRY). And probably keeping locks / unlocks contained in the same functions for that one will be much harder. Will it work if I introduce this per-module lock before this patch and use a concept of getting and putting module reference? That is, lock / unlock of ldr_data will always happen in the same function, but if the module reference is returned it will always have the lock on it and the caller responsibility will be to unlock the module reference (pretty much similar to how it is now done in wineserver WRT to getting and putting object references)?
That sounds better, yes. -- Alexandre Julliard julliard(a)winehq.org
On 11/4/20 22:19, Alexandre Julliard wrote:
Paul Gofman <pgofman(a)codeweavers.com> writes:
On 11/4/20 19:20, Alexandre Julliard wrote:
Paul Gofman <pgofman(a)codeweavers.com> writes:
@@ -2267,12 +2267,17 @@ static NTSTATUS open_dll_file( UNICODE_STRING *nt_name, WINE_MODREF **pwm, void NTSTATUS status; HANDLE handle, mapping;
+ if (loaded_only) + lock_ldr_data(); if ((*pwm = find_fullname_module( nt_name ))) { NtUnmapViewOfSection( NtCurrentProcess(), *module ); *module = NULL; + /* ldr_data_section to be unlocked by the caller. */ Not releasing the lock in the same scope is ugly and fragile, please try to find a better way.
The straightforward alternative would be to factor out the dll finding parts and use just them in LdrGetDllHandle().
But looking at further removing loader lock from functions like LdrAddRefDll and LdrUnloadDll it seems that it will be necessary to introduce a per-module lock (presumably with SRW lock using Lock field in struct _LDR_DATA_TABLE_ENTRY). And probably keeping locks / unlocks contained in the same functions for that one will be much harder. Will it work if I introduce this per-module lock before this patch and use a concept of getting and putting module reference? That is, lock / unlock of ldr_data will always happen in the same function, but if the module reference is returned it will always have the lock on it and the caller responsibility will be to unlock the module reference (pretty much similar to how it is now done in wineserver WRT to getting and putting object references)? That sounds better, yes.
Then maybe it is better to hold the whole series for now, as it would likely make sense to change the first patch to lock / unlock ldr_data_section in get_modref() instead of providing that lock in many places outside.
participants (3)
-
Alexandre Julliard -
Marvin -
Paul Gofman