[PATCH v7 0/2] MR11701: ntdll: Detect circular export forwarders and return the appropriate error.
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 -- v7: ntdll: Detect circular export forwarders and return the appropriate error. 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
Are you using LLM-based tool to prepare this patch? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11701#note_149673
On Tue Aug 25 01:30:32 2026 +0000, Jinoh Kang wrote:
Are you using LLM-based tool to prepare this patch? Partially, I wrote ntdll patch myself and used a simple program to compare LoadLibraryA and GetProcAddress return codes between Wine and Windows. The fix was straightforward until I started writing tests. It was tricky for me to modify the elegant gen_forward_chain_testdll since I have limited understanding of DLL format internals, so I use an LLM to come up with a method to update the DLL on the fly.
After reading several MR and wine copyright documents, I realize that the core team has strong concerns about copyright. I respect that, and I will rewrite the tests myself to submit a patch with no copyright concerns. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11701#note_149797
participants (3)
-
Jinoh Kang (＠iamahuman) -
Kyle Yang
-
Kyle Yang (＠kyle)