[PATCH v6 0/2] MR11701: ntdll/tests: Add tests for export forwarder circular detection.
When export forwarders have circular dependencies, Wine currently causes a stack overflow, which differs from Windows behavior. This patch uses a stack to detect circular dependencies and returns the same error code as Windows. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=60130 Signed-off-by: Kyle Yang kyle.chaoxin.yang@gmail.com -- v6: loader: Detect circular export forwarders and return the appropriate error. kernel32/tests: Add tests for export forwarder circular detection. https://gitlab.winehq.org/wine/wine/-/merge_requests/11701
From: Kyle Yang <kyle.chaoxin.yang@gmail.com> --- dlls/kernel32/tests/loader.c | 485 +++++++++++++++++++++++++++++++++++ 1 file changed, 485 insertions(+) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index baf5a705fe2..f294ff73a9f 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -3039,6 +3039,475 @@ static void subtest_export_forwarder_dep_chain( size_t num_chained_export_module } } +static const char *get_file_name(const char *full_path) +{ + const char *file_name = strrchr(full_path, '\\'); + if (!file_name) + file_name = strrchr(full_path, '/'); + + return file_name ? (file_name + 1) : full_path; +} + + +static BOOL rva_to_file_offset_checked(PIMAGE_NT_HEADERS nt, DWORD rva, DWORD file_size, DWORD *file_offset) +{ + PIMAGE_SECTION_HEADER section; + WORD i; + + if (!nt || !file_offset || rva == 0) + return FALSE; + + /* RVA inside PE headers */ + if (rva < nt->OptionalHeader.SizeOfHeaders) + { + if (rva >= file_size) + return FALSE; + + *file_offset = rva; + return TRUE; + } + + section = IMAGE_FIRST_SECTION(nt); + + for (i = 0; i < nt->FileHeader.NumberOfSections; ++i, ++section) + { + DWORD va = section->VirtualAddress; + DWORD virtual_size = section->Misc.VirtualSize; + DWORD raw_size = section->SizeOfRawData; + DWORD section_size = (virtual_size > raw_size) ? virtual_size : raw_size; + DWORD relative; + DWORD offset; + + if (section_size == 0 || rva < va) + continue; + + relative = rva - va; + + if (relative >= section_size) + continue; + + if (relative >= raw_size) + { + /* RVA points into virtual zero-filled tail */ + return FALSE; + } + + offset = section->PointerToRawData + relative; + + if (offset < section->PointerToRawData || offset >= file_size) + return FALSE; + + *file_offset = offset; + return TRUE; + } + + return FALSE; +} + + +static BOOL update_dll_in_place(const char *dll_path, const char *target_dll) +{ + HANDLE h_file = INVALID_HANDLE_VALUE; + BYTE *p_buffer = NULL; + DWORD file_size = 0; + DWORD bytes_read = 0; + DWORD bytes_written = 0; + BOOL b_success = FALSE; + + PIMAGE_DOS_HEADER p_dos_header = NULL; + PIMAGE_NT_HEADERS p_nt_headers = NULL; + PIMAGE_DATA_DIRECTORY p_export_data_dir = NULL; + PIMAGE_EXPORT_DIRECTORY p_export_dir = NULL; + PIMAGE_SECTION_HEADER p_edata_sec = NULL; + PIMAGE_SECTION_HEADER p_sec = NULL; + PIMAGE_SECTION_HEADER first_section = NULL; + + DWORD export_offset = 0; + DWORD functions_offset = 0; + DWORD names_offset = 0; + DWORD ordinals_offset = 0; + DWORD nt_offset = 0; + + DWORD str_space_raw_offset = 0; + DWORD str_space_rva = 0; + DWORD str_space_remaining = 256; + char *p_str_buffer = NULL; + + DWORD *p_functions = NULL; + DWORD *p_names = NULL; + WORD *p_ordinals = NULL; + + char target_mod_name[MAX_PATH]; + const char *p_name_only = NULL; + char *p_dot = NULL; + int patch_count = 0; + WORD i = 0; + LARGE_INTEGER li_file_size; + LARGE_INTEGER zero; + + if (!dll_path || !dll_path[0] || !target_dll || !target_dll[0]) + { + if (winetest_debug > 1) + trace("update_dll_in_place: Invalid arguments\n"); + return FALSE; + } + + /* Open target file with write/shared access */ + h_file = CreateFileA(dll_path, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + + if (h_file == INVALID_HANDLE_VALUE) + { + if (winetest_debug > 1) + trace("CreateFileA(%s) failed: %lu\n", dll_path, GetLastError()); + return FALSE; + } + + /* Validate file size */ + if (!GetFileSizeEx(h_file, &li_file_size) || li_file_size.QuadPart <= 0 || li_file_size.QuadPart > MAXDWORD) + { + if (winetest_debug > 1) + trace("GetFileSizeEx failed or invalid size for %s\n", dll_path); + goto cleanup; + } + + file_size = (DWORD)li_file_size.QuadPart; + + /* Read into heap memory */ + p_buffer = HeapAlloc(GetProcessHeap(), 0, file_size); + if (!p_buffer) + { + if (winetest_debug > 1) + trace("HeapAlloc failed\n"); + goto cleanup; + } + + if (!ReadFile(h_file, p_buffer, file_size, &bytes_read, NULL) || bytes_read != file_size) + { + if (winetest_debug > 1) + trace("ReadFile failed for %s, err=%lu\n", dll_path, GetLastError()); + goto cleanup; + } + + p_dos_header = (PIMAGE_DOS_HEADER)p_buffer; + nt_offset = (DWORD)p_dos_header->e_lfanew; + p_nt_headers = (PIMAGE_NT_HEADERS)(p_buffer + nt_offset); + + if (p_nt_headers->FileHeader.SizeOfOptionalHeader < sizeof(IMAGE_OPTIONAL_HEADER)) + goto cleanup; + + /* Validate section table */ + if (p_nt_headers->FileHeader.NumberOfSections == 0) + goto cleanup; + + first_section = IMAGE_FIRST_SECTION(p_nt_headers); + + /* Locate Export Directory */ + if (p_nt_headers->OptionalHeader.NumberOfRvaAndSizes <= IMAGE_DIRECTORY_ENTRY_EXPORT) + goto cleanup; + + p_export_data_dir = &p_nt_headers->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_EXPORT]; + if (p_export_data_dir->VirtualAddress == 0 || p_export_data_dir->Size == 0) + goto cleanup; + + if (!rva_to_file_offset_checked(p_nt_headers, p_export_data_dir->VirtualAddress, file_size, &export_offset)) + goto cleanup; + + p_export_dir = (PIMAGE_EXPORT_DIRECTORY)(p_buffer + export_offset); + + /* Locate Export Section Header (.edata) */ + p_sec = first_section; + for (i = 0; i < p_nt_headers->FileHeader.NumberOfSections; ++i, ++p_sec) + { + DWORD va = p_sec->VirtualAddress; + DWORD vs = p_sec->Misc.VirtualSize ? p_sec->Misc.VirtualSize : p_sec->SizeOfRawData; + + if (p_export_data_dir->VirtualAddress >= va && (p_export_data_dir->VirtualAddress - va) < vs) + { + p_edata_sec = p_sec; + break; + } + } + + if (!p_edata_sec || p_edata_sec->PointerToRawData == 0 || p_edata_sec->SizeOfRawData < 256) + goto cleanup; + + /* Reserve space in the last 256 bytes of section raw data */ + str_space_raw_offset = p_edata_sec->PointerToRawData + p_edata_sec->SizeOfRawData - 256; + str_space_rva = p_edata_sec->VirtualAddress + (str_space_raw_offset - p_edata_sec->PointerToRawData); + p_str_buffer = (char *)(p_buffer + str_space_raw_offset); + + /* 8. Resolve Export Tables */ + if (p_export_dir->NumberOfFunctions == 0) + goto cleanup; + + if (!rva_to_file_offset_checked(p_nt_headers, p_export_dir->AddressOfFunctions, file_size, &functions_offset)) + goto cleanup; + + p_functions = (DWORD *)(p_buffer + functions_offset); + + if (p_export_dir->NumberOfNames > 0) + { + if (!rva_to_file_offset_checked(p_nt_headers, p_export_dir->AddressOfNames, file_size, &names_offset)) + goto cleanup; + + p_names = (DWORD *)(p_buffer + names_offset); + + if (!rva_to_file_offset_checked(p_nt_headers, p_export_dir->AddressOfNameOrdinals, file_size, &ordinals_offset)) + goto cleanup; + + p_ordinals = (WORD *)(p_buffer + ordinals_offset); + } + + /* Format Target Module Name */ + p_name_only = get_file_name(target_dll); + lstrcpynA(target_mod_name, p_name_only, sizeof(target_mod_name)); + + p_dot = strrchr(target_mod_name, '.'); + if (p_dot && lstrcmpiA(p_dot, ".dll") == 0) + *p_dot = '\0'; + + if (target_mod_name[0] == '\0') + goto cleanup; + + /* Perform Export Patching */ + for (i = 0; i < p_export_dir->NumberOfNames; ++i) + { + DWORD name_offset = 0; + char *func_name = NULL; + WORD ordinal_index = 0; + DWORD str_len = 0; + + if (!rva_to_file_offset_checked(p_nt_headers, p_names[i], file_size, &name_offset) || name_offset >= file_size) + goto cleanup; + + func_name = (char *)(p_buffer + name_offset); + if (!memchr(func_name, '\0', file_size - name_offset)) + goto cleanup; + + ordinal_index = p_ordinals[i]; + if (ordinal_index >= p_export_dir->NumberOfFunctions) + goto cleanup; + + if (strcmp(func_name, "forward_test_func") == 0) + { + snprintf(p_str_buffer, str_space_remaining, "%s.forward_test_func", target_mod_name); + } + else if (strcmp(func_name, "forward_test_func2") == 0) + { + snprintf(p_str_buffer, str_space_remaining, "%s.#2", target_mod_name); + } + else + { + continue; + } + + str_len = (DWORD)lstrlenA(p_str_buffer) + 1; + if (str_len > str_space_remaining) + { + if (winetest_debug > 1) + trace("Not enough space for forwarder string\n"); + goto cleanup; + } + + p_functions[ordinal_index] = str_space_rva; + if (winetest_debug > 1) + trace("Patched %s -> %s (RVA: 0x%08lx)\n", func_name, p_str_buffer, str_space_rva); + + p_str_buffer += str_len; + str_space_rva += str_len; + str_space_remaining -= str_len; + patch_count++; + } + + if (patch_count == 0) + { + if (winetest_debug > 1) + trace("No functions patched in %s\n", dll_path); + b_success = TRUE; + goto cleanup; + } + + /* Fix PE Header Sizes & Checksums */ + p_nt_headers->OptionalHeader.CheckSum = 0; + + if (str_space_rva > p_export_data_dir->VirtualAddress) + p_export_data_dir->Size = str_space_rva - p_export_data_dir->VirtualAddress; + + if (p_edata_sec->Misc.VirtualSize < p_edata_sec->SizeOfRawData) + p_edata_sec->Misc.VirtualSize = p_edata_sec->SizeOfRawData; + + { + DWORD section_alignment = p_nt_headers->OptionalHeader.SectionAlignment; + DWORD section_end_rva = p_edata_sec->VirtualAddress + p_edata_sec->Misc.VirtualSize; + DWORD aligned_image_size = (section_end_rva + section_alignment - 1) & ~(section_alignment - 1); + + if (p_nt_headers->OptionalHeader.SizeOfImage < aligned_image_size) + p_nt_headers->OptionalHeader.SizeOfImage = aligned_image_size; + } + + /* Write back to disk & Flush */ + zero.QuadPart = 0; + if (!SetFilePointerEx(h_file, zero, NULL, FILE_BEGIN)) + goto cleanup; + + if (WriteFile(h_file, p_buffer, file_size, &bytes_written, NULL) && bytes_written == file_size) + { + FlushFileBuffers(h_file); + b_success = TRUE; + } + +cleanup: + if (h_file != INVALID_HANDLE_VALUE) + CloseHandle(h_file); + + if (p_buffer) + HeapFree(GetProcessHeap(), 0, p_buffer); + + return b_success; +} + + +static void subtest_export_forwarder_circular_detect_child_process( size_t num_modules ) +{ + size_t importer_index = num_modules - 1; + DWORD imp_thunk_base_rva, exp_func_base_rva; + char temp_paths[4][MAX_PATH]; + const char *target_file_name; + HANDLE temp_files[4]; + HMODULE modules[4]; + DWORD last_error; + FARPROC proc; + BOOL res; + size_t i; + + assert(num_modules >= 1); + assert(num_modules <= ARRAY_SIZE(temp_paths)); + assert(num_modules <= ARRAY_SIZE(temp_files)); + assert(num_modules <= ARRAY_SIZE(modules)); + + for (i = 0; i < num_modules; i++) + { + temp_files[i] = gen_forward_chain_testdll( temp_paths[i], + i >= 1 ? temp_paths[i - 1] : NULL, + i < num_modules, + importer_index && i == importer_index, + i == 0 ? &exp_func_base_rva : NULL, + i == importer_index ? &imp_thunk_base_rva : NULL ); + } + + target_file_name = get_file_name(temp_paths[0]); + + res = update_dll_in_place(temp_paths[0], target_file_name); + ok(res, "update_dll_in_place failed for %s\n", temp_paths[0]); + + /* + * Single DLL with a self-forward: + * - LoadLibrary succeeds. + * - GetProcAddress fails and returns NULL with ERROR_BAD_EXE_FORMAT (193). + * + * Chained DLLs: + * - DLLs without imports can be loaded successfully. + * - GetProcAddress fails with ERROR_BAD_EXE_FORMAT (193). + * - The last DLL, which has an import, fails to load with LoadLibraryA, + * returning NULL and ERROR_BAD_EXE_FORMAT (193). + */ + for (i = 0; i < num_modules; i++) + { + modules[i] = LoadLibraryA( temp_paths[i] ); + last_error = GetLastError(); + + if ( i < num_modules-1 ) + { + ok( !!modules[i], "LoadLibraryA(%s): modules[%Iu/%Iu] = %p, err=%lu\n", + temp_paths[i], i, num_modules, modules[i], last_error ); + } + else + { + if ( num_modules != 1 ) + { + ok( !modules[i] && last_error == ERROR_BAD_EXE_FORMAT, + "LoadLibraryA(%s): modules[%Iu/%Iu] = %p, err=%lu\n", + temp_paths[i], i, num_modules, modules[i], last_error ); + } + else + { + ok( !!modules[i], "LoadLibraryA(%s): modules[%Iu/%Iu] = %p, err=%lu\n", + temp_paths[i], i, num_modules, modules[i], last_error ); + } + } + + proc = GetProcAddress( modules[i], "forward_test_func" ); + last_error = GetLastError(); + + if ( i < num_modules-1 ) + { + ok( !proc && last_error == ERROR_BAD_EXE_FORMAT, "modules[%Iu] %s GetProcAddress = %p, err=%lu\n", + i, temp_paths[i], (void *)proc, last_error ); + } + else + { + if ( num_modules != 1 ) + { + ok( !proc && last_error == ERROR_PROC_NOT_FOUND,"modules[%Iu] %s GetProcAddress = %p, err=%lu\n", + i, temp_paths[i], (void *)proc, last_error ); + } + else + { + ok( !proc && last_error == ERROR_BAD_EXE_FORMAT, "modules[%Iu] %s GetProcAddress = %p, err=%lu\n", + i, temp_paths[i], (void *)proc, last_error ); + } + } + + } + + for (i = num_modules; i < num_modules; i++) + { + res = FreeLibrary( modules[i] ); + ok( res, "FreeLibrary(modules[%Iu]) err=%lu\n", i, GetLastError() ); + } + + for (i = 0; i < num_modules; i++) + { + CloseHandle( temp_files[i] ); + } +} + +static void subtest_export_forwarder_circular_detect( size_t num_modules ) +{ + char cmdline[MAX_PATH * 2]; + char **argv; + DWORD ret; + PROCESS_INFORMATION pi; + STARTUPINFOA si = { sizeof(si) }; + + winetest_get_mainargs(&argv); + + *child_failures = -1; + sprintf(cmdline, "\"%s\" loader circular_detect %Iu", argv[0], num_modules); + + ret = CreateProcessA(argv[0], cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "CreateProcess(%s) error %ld\n", cmdline, GetLastError()); + + ret = WaitForSingleObject(pi.hProcess, 10000); + ok(ret == WAIT_OBJECT_0, "child process failed to terminate\n"); + if (ret != WAIT_OBJECT_0) TerminateProcess(pi.hProcess, 0); + + GetExitCodeProcess(pi.hProcess, &ret); + todo_wine + ok(ret == 0, "expected exit code 0, got %lu\n", ret); + + if (*child_failures) + { + trace("%ld failures in child process\n", *child_failures); + winetest_add_failures(*child_failures); + } + + CloseHandle(pi.hThread); + CloseHandle(pi.hProcess); +} + static void test_export_forwarder_dep_chain(void) { winetest_push_context( "no import" ); @@ -3069,6 +3538,14 @@ static void test_export_forwarder_dep_chain(void) winetest_push_context( "dynamic import of dll already loaded with DONT_RESOLVE_DLL_REFERENCES" ); subtest_export_forwarder_dep_chain( 2, 1, FALSE, DONT_RESOLVE_DLL_REFERENCES ); winetest_pop_context(); + + winetest_push_context( "static import of export forwarder circular detect with chained dll" ); + subtest_export_forwarder_circular_detect( 3 ); + winetest_pop_context(); + + winetest_push_context( "static import of export forwarder circular detect with self chained dll" ); + subtest_export_forwarder_circular_detect( 1 ); + winetest_pop_context(); } #define MAX_COUNT 10 @@ -4986,6 +5463,14 @@ START_TEST(loader) *child_failures = -1; argc = winetest_get_mainargs(&argv); + + if (argc == 4) + { + test_dll_phase = atoi(argv[3]); + subtest_export_forwarder_circular_detect_child_process(test_dll_phase); + return; + } + if (argc > 4) { test_dll_phase = atoi(argv[4]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11701
From: Kyle Yang <kyle.chaoxin.yang@gmail.com> --- dlls/kernel32/tests/loader.c | 1 - dlls/ntdll/loader.c | 156 ++++++++++++++++++++++++++--------- 2 files changed, 115 insertions(+), 42 deletions(-) diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index f294ff73a9f..3ad77f287a5 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -3495,7 +3495,6 @@ static void subtest_export_forwarder_circular_detect( size_t num_modules ) if (ret != WAIT_OBJECT_0) TerminateProcess(pi.hProcess, 0); GetExitCodeProcess(pi.hProcess, &ret); - todo_wine ok(ret == 0, "expected exit code 0, got %lu\n", ret); if (*child_failures) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index a0992cf68d3..4185a644d64 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -142,6 +142,11 @@ typedef struct _wine_modref BOOL system; } WINE_MODREF; +typedef struct _forward_stack { + struct list entry; + const char *forward; +} forward_stack; + static UINT tls_module_count = 32; /* number of modules with TLS directory */ static IMAGE_TLS_DIRECTORY *tls_dirs; /* array of TLS directories */ @@ -194,10 +199,16 @@ static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, DWORD fl static NTSTATUS process_attach( LDR_DDAG_NODE *node, LPVOID lpReserved ); static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, DWORD ordinal, LPCWSTR load_path, - WINE_MODREF *importer, BOOL is_dynamic ); + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret ); +static FARPROC find_ordinal_export_internal( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, + DWORD exp_size, DWORD ordinal, LPCWSTR load_path, + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret, forward_stack *stack ); static FARPROC find_named_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, const char *name, int hint, LPCWSTR load_path, - WINE_MODREF *importer, BOOL is_dynamic ); + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret ); +static FARPROC find_named_export_internal( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, + const char *name, int hint, LPCWSTR load_path, + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret, forward_stack *stack ); /* check whether the file name contains a path */ static inline BOOL contains_path( LPCWSTR name ) @@ -941,13 +952,31 @@ static NTSTATUS walk_node_dependencies( LDR_DDAG_NODE *node, void *context, return status; } + +static BOOL has_circular_forward( const forward_stack *stack, const char *forward ) +{ + forward_stack *elem; + + LIST_FOR_EACH_ENTRY_REV(elem, &stack->entry, forward_stack, entry) + { + if (elem->forward == forward) { + return TRUE; + } + } + + return FALSE; +} + + /************************************************************************* * find_forwarded_export * * Find the final function pointer for a forwarded function. * The loader_section must be locked while calling this function. */ -static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWSTR load_path, WINE_MODREF *importer, BOOL is_dynamic ) +static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWSTR load_path, + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret, + forward_stack *stack ) { const IMAGE_EXPORT_DIRECTORY *exports; DWORD exp_size; @@ -956,6 +985,7 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS const char *end = strrchr(forward, '.'); FARPROC proc = NULL; BOOL wm_loaded = FALSE; + forward_stack current_frame; if (!end) return NULL; if (build_import_name( importer, mod_name, forward, end - forward )) return NULL; @@ -987,41 +1017,46 @@ static FARPROC find_forwarded_export( HMODULE module, const char *forward, LPCWS } } + if (has_circular_forward(stack, forward)) { + proc = NULL; + *ret = STATUS_INVALID_IMAGE_FORMAT; + return NULL; + } + + current_frame.forward = forward; + list_add_tail(&stack->entry, ¤t_frame.entry); + if ((exports = RtlImageDirectoryEntryToData( wm->ldr.DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_EXPORT, &exp_size ))) { const char *name = end + 1; if (*name == '#') { /* ordinal */ - proc = find_ordinal_export( wm->ldr.DllBase, exports, exp_size, + proc = find_ordinal_export_internal( wm->ldr.DllBase, exports, exp_size, atoi(name+1) - exports->Base, load_path, - importer, is_dynamic ); + importer, is_dynamic, ret, stack ); } else - proc = find_named_export( wm->ldr.DllBase, exports, exp_size, name, -1, load_path, - importer, is_dynamic ); + proc = find_named_export_internal( wm->ldr.DllBase, exports, exp_size, name, -1, load_path, + importer, is_dynamic, ret, stack ); } - if (!proc) + if (!proc && *ret == STATUS_PROCEDURE_NOT_FOUND) { ERR("function not found for forward '%s' used by %s." " If you are using builtin %s, try using the native one instead.\n", forward, debugstr_w(get_modref(module)->ldr.FullDllName.Buffer), debugstr_w(get_modref(module)->ldr.BaseDllName.Buffer) ); } + + list_remove(¤t_frame.entry); return proc; } -/************************************************************************* - * find_ordinal_export - * - * 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. - */ -static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, +static FARPROC find_ordinal_export_internal( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, DWORD ordinal, LPCWSTR load_path, - WINE_MODREF *importer, BOOL is_dynamic ) + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret, + forward_stack *stack ) { FARPROC proc; const DWORD *functions = get_rva( module, exports->AddressOfFunctions ); @@ -1038,7 +1073,7 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY /* if the address falls into the export dir, it's a forward */ if (((const char *)proc >= (const char *)exports) && ((const char *)proc < (const char *)exports + exp_size)) - return find_forwarded_export( module, (const char *)proc, load_path, importer, is_dynamic ); + return find_forwarded_export( module, (const char *)proc, load_path, importer, is_dynamic, ret, stack ); if (TRACE_ON(snoop)) { @@ -1054,6 +1089,28 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY } +/************************************************************************* + * find_ordinal_export + * + * 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. + */ +static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, + DWORD exp_size, DWORD ordinal, LPCWSTR load_path, + WINE_MODREF *importer, BOOL is_dynamic, NTSTATUS *ret ) +{ + FARPROC proc; + forward_stack stack; + + list_init(&stack.entry); + proc = find_ordinal_export_internal(module, exports, exp_size, ordinal, load_path, + importer, is_dynamic, ret, &stack); + + return proc; +} + + /************************************************************************* * find_name_in_exports * @@ -1077,15 +1134,9 @@ static int find_name_in_exports( HMODULE module, const IMAGE_EXPORT_DIRECTORY *e } -/************************************************************************* - * find_named_export - * - * Find an exported function by name. - * The loader_section must be locked while calling this function. - */ -static FARPROC find_named_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, +static FARPROC find_named_export_internal( HMODULE module, const IMAGE_EXPORT_DIRECTORY *exports, DWORD exp_size, const char *name, int hint, LPCWSTR load_path, WINE_MODREF *importer, - BOOL is_dynamic ) + BOOL is_dynamic, NTSTATUS *ret, forward_stack *stack ) { const WORD *ordinals = get_rva( module, exports->AddressOfNameOrdinals ); const DWORD *names = get_rva( module, exports->AddressOfNames ); @@ -1096,13 +1147,35 @@ static FARPROC find_named_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY * { char *ename = get_rva( module, names[hint] ); if (!strcmp( ename, name )) - return find_ordinal_export( module, exports, exp_size, ordinals[hint], load_path, importer, is_dynamic ); + return find_ordinal_export_internal( module, exports, exp_size, ordinals[hint], + load_path, importer, is_dynamic, ret, stack ); } /* then do a binary search */ if ((ordinal = find_name_in_exports( module, exports, name )) == -1) return NULL; - return find_ordinal_export( module, exports, exp_size, ordinal, load_path, importer, is_dynamic ); + return find_ordinal_export_internal( module, exports, exp_size, ordinal, load_path, importer, is_dynamic, ret, stack ); + +} + +/************************************************************************* + * find_named_export + * + * Find an exported function by name. + * The 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, WINE_MODREF *importer, + BOOL is_dynamic, NTSTATUS *ret ) +{ + FARPROC proc; + forward_stack stack; + + list_init(&stack.entry); + proc = find_named_export_internal(module, exports, exp_size, name, hint, load_path, + importer, is_dynamic, ret, &stack); + + return proc; } @@ -1139,11 +1212,10 @@ void * WINAPI RtlFindExportedRoutineByName( HMODULE module, const char *name ) * Import the dll specified by the given import descriptor. * The loader_section must be locked while calling this function. */ -static BOOL import_dll( WINE_MODREF *wm, const IMAGE_IMPORT_DESCRIPTOR *descr, LPCWSTR load_path, WINE_MODREF **pwm ) +static BOOL import_dll( WINE_MODREF *wm, const IMAGE_IMPORT_DESCRIPTOR *descr, LPCWSTR load_path, WINE_MODREF **pwm, NTSTATUS *status ) { HMODULE module = wm->ldr.DllBase; BOOL system = wm->system || (wm->ldr.Flags & LDR_WINE_INTERNAL); - NTSTATUS status; WINE_MODREF *wmImp; HMODULE imp_mod; const IMAGE_EXPORT_DIRECTORY *exports; @@ -1170,17 +1242,17 @@ static BOOL import_dll( WINE_MODREF *wm, const IMAGE_IMPORT_DESCRIPTOR *descr, L return TRUE; } - status = build_import_name( wm, buffer, name, len ); - if (!status) status = load_dll( load_path, buffer, 0, &wmImp, system ); + *status = build_import_name( wm, buffer, name, len ); + if (!*status) *status = load_dll( load_path, buffer, 0, &wmImp, system ); - if (status) + if (*status) { - if (status == STATUS_DLL_NOT_FOUND) + if (*status == STATUS_DLL_NOT_FOUND) ERR("Library %s (which is needed by %s) not found\n", name, debugstr_w(wm->ldr.FullDllName.Buffer)); else ERR("Loading library %s (which is needed by %s) failed (error %lx).\n", - name, debugstr_w(wm->ldr.FullDllName.Buffer), status); + name, debugstr_w(wm->ldr.FullDllName.Buffer), *status); return FALSE; } @@ -1228,7 +1300,8 @@ static BOOL import_dll( WINE_MODREF *wm, const IMAGE_IMPORT_DESCRIPTOR *descr, L int ordinal = IMAGE_ORDINAL(import_list->u1.Ordinal); thunk_list->u1.Function = (ULONG_PTR)find_ordinal_export( imp_mod, exports, exp_size, - ordinal - exports->Base, load_path, wm, FALSE ); + ordinal - exports->Base, load_path, wm, FALSE, status ); + if (*status == STATUS_INVALID_IMAGE_FORMAT) return FALSE; if (!thunk_list->u1.Function) { thunk_list->u1.Function = allocate_stub( name, IntToPtr(ordinal) ); @@ -1244,7 +1317,7 @@ static BOOL import_dll( WINE_MODREF *wm, const IMAGE_IMPORT_DESCRIPTOR *descr, L pe_name = get_rva( module, (DWORD)import_list->u1.AddressOfData ); thunk_list->u1.Function = (ULONG_PTR)find_named_export( imp_mod, exports, exp_size, (const char*)pe_name->Name, - pe_name->Hint, load_path, wm, FALSE ); + pe_name->Hint, load_path, wm, FALSE, status ); if (!thunk_list->u1.Function) { thunk_list->u1.Function = allocate_stub( name, (const char*)pe_name->Name ); @@ -1513,8 +1586,9 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) for (i = 0; i < nb_imports; i++) { dep_after = wm->ldr.DdagNode->Dependencies.Tail; - if (!import_dll( wm, &imports[i], load_path, &imp )) - status = STATUS_DLL_NOT_FOUND; + if (!import_dll( wm, &imports[i], load_path, &imp, &status )) + break; + else if (imp && imp->ldr.DdagNode != node_ntdll && imp->ldr.DdagNode != node_kernel32) add_module_dependency_after( wm->ldr.DdagNode, imp->ldr.DdagNode, dep_after ); } @@ -2078,8 +2152,8 @@ NTSTATUS WINAPI LdrGetProcedureAddress(HMODULE module, const ANSI_STRING *name, else if ((exports = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_EXPORT, &exp_size ))) { - void *proc = name ? find_named_export( module, exports, exp_size, name->Buffer, -1, NULL, wm, TRUE ) - : find_ordinal_export( module, exports, exp_size, ord - exports->Base, NULL, wm, TRUE ); + void *proc = name ? find_named_export( module, exports, exp_size, name->Buffer, -1, NULL, wm, TRUE, &ret ) + : find_ordinal_export( module, exports, exp_size, ord - exports->Base, NULL, wm, TRUE, &ret ); if (proc) { *address = proc; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11701
On Mon Aug 24 08:09:07 2026 +0000, Sven Baars wrote:
The test and the fix should be in the same MR, just in different commits. Also, the commit message of your commit is currently wrong, since it contains only tests. Got it, thank you for clarifying. I have rebased and split the changes into two commits in this MR, and updated the commit messages.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11701#note_149659
participants (2)
-
Kyle Yang -
Kyle Yang (＠kyle)