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

[Zebediah Figura (she/her)]

On 11/23/20 7:55 PM, Kevin Puetz wrote:
> If there are no imports at all, winebuild will omit __wine_spec_imports,
> so both VirtualAddress and Size will be 0. In this case get_rva(module, 0)
> does not point to a valid IMAGE_IMPORT_DESCRIPTOR.
> 
> Signed-off-by: Kevin Puetz <PuetzKevinA at JohnDeere.com>
> 
> ---
> v2: Tweaked whitespace and commit message per feedback from Gijs Vermeulen
> 
> 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;
> 

This seems like it could be simplified by just using early return instead.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201124/29c3c0ed/attachment.sig>