New subject: [PATCH v2] ntdll/unix: Don't access empty IMAGE_FILE_IMPORT_DIRECTORY.

24 Nov 2020

If there are no imports at all, winebuild will omit __wine_spec_imports,
so both VirtualAddress and Size will be 0. However, get_rva(module, 0)
does not point to a valid IMAGE_IMPORT_DESCRIPTOR.

Signed-off-by: Kevin Puetz <PuetzKevinA(a)JohnDeere.com>

---
My aarch64 builds would "work" on real hardware, but crash in wineboot
(or pretty much anything else non-trivial) if run in qemu-user 5.1.0.
The first crash would occur while loading gdi32.dll.so/gdi32.so.
wine 5.0.x worked, so it seems like there was enough hope to keep digging
and this seems to be the culprit.

I see desc->Name == 0xffff (actually reinterpreted bytes from the ELF header)
and on real aarch64 hardware the resulting bad pointer seems to avoid disaster,
(char*)get_rva(module,0xFFFF) == '\0'. I assume something just zero filled
betwen the ELF header and the __wine_spec_nt_header.

It does, however, hit the ERR trace with it's, printing:
"err:module:fixup_ntdll_imports module "/usr/lib/wine/gdi32.so" is importing "
(note empty %s)

In qemu-user, get_rva(module,0xFFFF) is unmapped so it just segfaults.
(probably just finer-grained tracking of sections and less zero-filling,
since qemu needs to move things around for the dynamic recompilation).

I fixed it to check DataDirectory[IMAGE_FILE_IMPORT_DIRECTORY].Size
before dereferencing the table, as map_so_dll does, so it doesn't access
a nonexistant imports section. This seems right, but I certainly won't
claim to understand all the machinery in here...
---
Patch below is mostly just indentation changing, diff -w looks like:

 static NTSTATUS fixup_ntdll_imports( const char *name, HMODULE module )
 {
     const IMAGE_NT_HEADERS *nt;
-    const IMAGE_IMPORT_DESCRIPTOR *descr;
+    const IMAGE_DATA_DIRECTORY *dir;
     const IMAGE_THUNK_DATA *import_list;
     IMAGE_THUNK_DATA *thunk_list;
 
     nt = get_rva( module, ((IMAGE_DOS_HEADER *)module)->e_lfanew );
-    descr = get_rva( module, nt->OptionalHeader.DataDirectory[IMAGE_FILE_IMPORT_DIRECTORY].VirtualAddress );
+
+    dir = &nt->OptionalHeader.DataDirectory[IMAGE_FILE_IMPORT_DIRECTORY];
+    if (dir->Size)
+    {
+        const IMAGE_IMPORT_DESCRIPTOR *descr= get_rva( module, dir->VirtualAddress );
+
         for (; descr->Name && descr->FirstThunk; descr++)
         { ... }
+    }
     return STATUS_SUCCESS;
 }

diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c
index c2b6ea603e3..5d81e26cc5b 100644
--- a/dlls/ntdll/unix/loader.c
+++ b/dlls/ntdll/unix/loader.c
@@ -809,43 +809,49 @@ static inline void *get_rva( void *module, ULONG_PTR addr )
 static NTSTATUS fixup_ntdll_imports( const char *name, HMODULE module )
 {
     const IMAGE_NT_HEADERS *nt;
-    const IMAGE_IMPORT_DESCRIPTOR *descr;
+    const IMAGE_DATA_DIRECTORY *dir;
     const IMAGE_THUNK_DATA *import_list;
     IMAGE_THUNK_DATA *thunk_list;
 
     nt = get_rva( module, ((IMAGE_DOS_HEADER *)module)->e_lfanew );
-    descr = get_rva( module, nt->OptionalHeader.DataDirectory[IMAGE_FILE_IMPORT_DIRECTORY].VirtualAddress );
-    for (; descr->Name && descr->FirstThunk; descr++)
+
+    dir = &nt->OptionalHeader.DataDirectory[IMAGE_FILE_IMPORT_DIRECTORY];
+    if (dir->Size)
     {
-        thunk_list = get_rva( module, descr->FirstThunk );
+        const IMAGE_IMPORT_DESCRIPTOR *descr= get_rva( module, dir->VirtualAddress );
 
-        /* ntdll must be the only import */
-        if (strcmp( get_rva( module, descr->Name ), "ntdll.dll" ))
+        for (; descr->Name && descr->FirstThunk; descr++)
         {
-            ERR( "module %s is importing %s\n", debugstr_a(name), (char *)get_rva( module, descr->Name ));
-            return STATUS_PROCEDURE_NOT_FOUND;
-        }
-        if (descr->u.OriginalFirstThunk)
-            import_list = get_rva( module, descr->u.OriginalFirstThunk );
-        else
-            import_list = thunk_list;
+            thunk_list = get_rva( module, descr->FirstThunk );
 
-        while (import_list->u1.Ordinal)
-        {
-            if (IMAGE_SNAP_BY_ORDINAL( import_list->u1.Ordinal ))
+            /* ntdll must be the only import */
+            if (strcmp( get_rva( module, descr->Name ), "ntdll.dll" ))
             {
-                int ordinal = IMAGE_ORDINAL( import_list->u1.Ordinal ) - ntdll_exports->Base;
-                thunk_list->u1.Function = find_ordinal_export( ntdll_module, ntdll_exports, ordinal );
-                if (!thunk_list->u1.Function) ERR( "%s: ntdll.%u not found\n", debugstr_a(name), ordinal );
+                ERR( "module %s is importing %s\n", debugstr_a(name), (char *)get_rva( module, descr->Name ));
+                return STATUS_PROCEDURE_NOT_FOUND;
             }
-            else  /* import by name */
+            if (descr->u.OriginalFirstThunk)
+                import_list = get_rva( module, descr->u.OriginalFirstThunk );
+            else
+                import_list = thunk_list;
+
+            while (import_list->u1.Ordinal)
             {
-                IMAGE_IMPORT_BY_NAME *pe_name = get_rva( module, import_list->u1.AddressOfData );
-                thunk_list->u1.Function = find_pe_export( ntdll_module, ntdll_exports, pe_name );
-                if (!thunk_list->u1.Function) ERR( "%s: ntdll.%s not found\n", debugstr_a(name), pe_name->Name );
+                if (IMAGE_SNAP_BY_ORDINAL( import_list->u1.Ordinal ))
+                {
+                    int ordinal = IMAGE_ORDINAL( import_list->u1.Ordinal ) - ntdll_exports->Base;
+                    thunk_list->u1.Function = find_ordinal_export( ntdll_module, ntdll_exports, ordinal );
+                    if (!thunk_list->u1.Function) ERR( "%s: ntdll.%u not found\n", debugstr_a(name), ordinal );
+                }
+                else  /* import by name */
+                {
+                    IMAGE_IMPORT_BY_NAME *pe_name = get_rva( module, import_list->u1.AddressOfData );
+                    thunk_list->u1.Function = find_pe_export( ntdll_module, ntdll_exports, pe_name );
+                    if (!thunk_list->u1.Function) ERR( "%s: ntdll.%s not found\n", debugstr_a(name), pe_name->Name );
+                }
+                import_list++;
+                thunk_list++;
             }
-            import_list++;
-            thunk_list++;
         }
     }
     return STATUS_SUCCESS;

    

[PATCH] ntdll/unix: do not access nonexistent IMAGE_FILE_IMPORT_DIRECTORY

Kevin Puetz

Kevin Puetz

Zebediah Figura (she/her)

tags

participants (2)