[PATCH v2] ntdll: Cache LDR_IMAGE_IS_DLL for InitDLL

[Myah Caron]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48817
Signed-off-by: Myah Caron <qsniyg(a)protonmail.com>
---
v2: Only cache LDR_IMAGE_IS_DLL.
Using a cached version for all flags can cause issues with Battle.net and Warframe (Thanks GloriousEggroll for testing!)
Supersedes "ntdll: Use a cached version of LDR_MODULE flags for InitDLL."
---

[Ilie Halip]

You could alternatively check if the IMAGE_FILE_DLL flag is set in the
NT header, maybe something like
this (not compiled, not tested):

--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -1309,7 +1309,14 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF
*wm, UINT reason, LPVOID lpReserved

     if (wm->ldr.Flags & LDR_DONT_RESOLVE_REFS) return STATUS_SUCCESS;
     if (wm->ldr.TlsIndex != -1) call_tls_callbacks(
wm->ldr.BaseAddress, reason );
-    if (!entry || !(wm->ldr.Flags & LDR_IMAGE_IS_DLL)) return STATUS_SUCCESS;
+    if (!entry) return STATUS_SUCCESS;
+    if (!(wm->ldr.Flags & LDR_IMAGE_IS_DLL))
+    {
+        if (RtlImageNtHeader(module)->FileHeader.Characteristics &
IMAGE_FILE_DLL)
+            TRACE("... warning: mismatch between flags ... continuing
anyway ...");
+        else
+            return STATUS_SUCCESS;
+    }

     if (TRACE_ON(relay))
     {

Not sure which one is the better approach, though.

[Marvin]

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=68776

Your paranoid android.


=== debiant (build log) ===

error: corrupt patch at line 8
Task: Patch failed to apply

=== debiant (build log) ===

error: corrupt patch at line 8
Task: Patch failed to apply

[Myah Caron]

Thanks for the suggestion! I don't think it's correct though, because under Windows 10, it will still load if BaseAddress (module) has been modified, even if it's set to 0.

See the test case here: https://github.com/qsniyg/wine_dll_load_test , uncomment the BaseAddress = 0 line in dll2.c

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, April 2, 2020 12:57 AM, Ilie Halip <ilie.halip(a)gmail.com> wrote:
> You could alternatively check if the IMAGE_FILE_DLL flag is set in the
> NT header, maybe something like
> this (not compiled, not tested):
>
> --- a/dlls/ntdll/loader.c
> +++ b/dlls/ntdll/loader.c
> @@ -1309,7 +1309,14 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF
> *wm, UINT reason, LPVOID lpReserved
>
> if (wm->ldr.Flags & LDR_DONT_RESOLVE_REFS) return STATUS_SUCCESS;
>
> if (wm->ldr.TlsIndex != -1) call_tls_callbacks(
>
> wm->ldr.BaseAddress, reason );
>
> -   if (!entry || !(wm->ldr.Flags & LDR_IMAGE_IS_DLL)) return STATUS_SUCCESS;
>
> -   if (!entry) return STATUS_SUCCESS;
>
> -   if (!(wm->ldr.Flags & LDR_IMAGE_IS_DLL))
>
> -   {
>
> -          if (RtlImageNtHeader(module)->FileHeader.Characteristics &
>
> IMAGE_FILE_DLL)
>
> -              TRACE("... warning: mismatch between flags ... continuing
>
> anyway ...");
>
> -          else
>
> -              return STATUS_SUCCESS;
>
> -   }
>
> if (TRACE_ON(relay))
>     {
>
> Not sure which one is the better approach, though.

[Alexandre Julliard]

Myah Caron <qsniyg(a)protonmail.com> writes:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48817
> Signed-off-by: Myah Caron <qsniyg(a)protonmail.com>
> ---
> v2: Only cache LDR_IMAGE_IS_DLL.
> Using a cached version for all flags can cause issues with Battle.net and Warframe (Thanks GloriousEggroll for testing!)
> Supersedes "ntdll: Use a cached version of LDR_MODULE flags for InitDLL."
> ---

This looks like a hack. You could probably tweak the ldr.EntryPoint
usage instead for that purpose.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Myah Caron]

Sorry, I'm not quite sure I understand... how could I tweak the usage of ldr.EntryPoint in order to achieve the same result?

To clarify, the issue is that under Windows, it seems like Flags & LDR_IMAGE_IS_DLL is only evaluated once at the beginning, and then cached for the rest of the DLL's lifetime. No matter how Flags is subsequently modified (even if it's modified before the initial DllMain call), it will always seem to call DllMain if the Characteristics was set as IMAGE_FILE_DLL.

Thank you for looking into this!! :)

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, April 6, 2020 3:24 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> Myah Caron qsniyg(a)protonmail.com writes:
> > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48817
> > Signed-off-by: Myah Caron qsniyg(a)protonmail.com
> >
> > --------------------------------------------------------------------------------------------------------
> >
> > v2: Only cache LDR_IMAGE_IS_DLL.
> > Using a cached version for all flags can cause issues with Battle.net and Warframe (Thanks GloriousEggroll for testing!)
> > Supersedes "ntdll: Use a cached version of LDR_MODULE flags for InitDLL."
> >
> > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> This looks like a hack. You could probably tweak the ldr.EntryPoint
> usage instead for that purpose.
>
> ------------------------------------------------------------------------------------------------------
>
> Alexandre Julliard
> julliard(a)winehq.org

[Alexandre Julliard]

Myah Caron <qsniyg(a)protonmail.com> writes:
> Sorry, I'm not quite sure I understand... how could I tweak the usage of ldr.EntryPoint in order to achieve the same result?
>
> To clarify, the issue is that under Windows, it seems like Flags &
> LDR_IMAGE_IS_DLL is only evaluated once at the beginning, and then
> cached for the rest of the DLL's lifetime. No matter how Flags is
> subsequently modified (even if it's modified before the initial
> DllMain call), it will always seem to call DllMain if the
> Characteristics was set as IMAGE_FILE_DLL.

You'd make sure ldr.EntryPoint is set only when we actually have a dll
entry point to call.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Myah Caron]

Thank you for explaining!!

However, that doesn't seem to match Windows's behavior, ldr.EntryPoint is always set, at least when tested under Windows 10 (you can test it with this commit: https://github.com/qsniyg/wine_dll_load_test/commit/d7df37845d0a6202ceafc9d4...). Unless you were referring to something else? Sorry if I misunderstood!

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, April 6, 2020 4:29 AM, Alexandre Julliard <julliard(a)winehq.org> wrote:
> Myah Caron qsniyg(a)protonmail.com writes:
> > Sorry, I'm not quite sure I understand... how could I tweak the usage of ldr.EntryPoint in order to achieve the same result?
> > To clarify, the issue is that under Windows, it seems like Flags &
> > LDR_IMAGE_IS_DLL is only evaluated once at the beginning, and then
> > cached for the rest of the DLL's lifetime. No matter how Flags is
> > subsequently modified (even if it's modified before the initial
> > DllMain call), it will always seem to call DllMain if the
> > Characteristics was set as IMAGE_FILE_DLL.
>
> You'd make sure ldr.EntryPoint is set only when we actually have a dll
> entry point to call.
>
> ----------------------------------------------------------------------------------------------
>
> Alexandre Julliard
> julliard(a)winehq.org