[PATCH] ntdll: Don't fail if LDR_MODULE.Flags is modified during load

[Myah Caron]

Modifications to EntryPoint are respected, which is why it uses the
original wm->ldr instead of ldr.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48817
Signed-off-by: Myah Caron <qsniyg(a)mail.com>
---
 dlls/ntdll/loader.c | 46 +++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 1b641f1b9d..c54ef24c97 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -696,7 +696,7 @@ static FARPROC find_ordinal_export( HMODULE module, const IMAGE_EXPORT_DIRECTORY
     proc = get_rva( module, functions[ordinal] );

     /* if the address falls into the export dir, it's a forward */
-    if (((const char *)proc >= (const char *)exports) &&
+    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 );

@@ -1301,7 +1301,7 @@ static void call_tls_callbacks( HMODULE module, UINT reason )
 /*************************************************************************
  *              MODULE_InitDLL
  */
-static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, UINT reason, LPVOID lpReserved )
+static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, LDR_MODULE ldr, UINT reason, LPVOID lpReserved )
 {
     WCHAR mod_name[32];
     NTSTATUS status = STATUS_SUCCESS;
@@ -1311,19 +1311,19 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, UINT reason, LPVOID lpReserved

     /* Skip calls for modules loaded with special load flags */

-    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 (ldr.Flags & LDR_DONT_RESOLVE_REFS) return STATUS_SUCCESS;
+    if (ldr.TlsIndex != -1) call_tls_callbacks( wm->ldr.BaseAddress, reason );
+    if (!entry || !(ldr.Flags & LDR_IMAGE_IS_DLL)) return STATUS_SUCCESS;

     if (TRACE_ON(relay))
     {
-        size_t len = min( wm->ldr.BaseDllName.Length, sizeof(mod_name)-sizeof(WCHAR) );
-        memcpy( mod_name, wm->ldr.BaseDllName.Buffer, len );
+        size_t len = min( ldr.BaseDllName.Length, sizeof(mod_name)-sizeof(WCHAR) );
+        memcpy( mod_name, ldr.BaseDllName.Buffer, len );
         mod_name[len / sizeof(WCHAR)] = 0;
         TRACE_(relay)("\1Call PE DLL (proc=%p,module=%p %s,reason=%s,res=%p)\n",
                       entry, module, debugstr_w(mod_name), reason_names[reason], lpReserved );
     }
-    else TRACE("(%p %s,%s,%p) - CALL\n", module, debugstr_w(wm->ldr.BaseDllName.Buffer),
+    else TRACE("(%p %s,%s,%p) - CALL\n", module, debugstr_w(ldr.BaseDllName.Buffer),
                reason_names[reason], lpReserved );

     __TRY
@@ -1389,6 +1389,7 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved )
     NTSTATUS status = STATUS_SUCCESS;
     ULONG_PTR cookie;
     int i;
+    LDR_MODULE ldr_backup;

     if (process_detaching) return status;

@@ -1404,6 +1405,9 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved )
     if (lpReserved) wm->ldr.LoadCount = -1;  /* pin it if imported by the main exe */
     if (wm->ldr.ActivationContext) RtlActivateActivationContext( 0, wm->ldr.ActivationContext, &cookie );

+    /* The flags can be modified by DLL dependencies  */
+    ldr_backup = wm->ldr;
+
     /* Recursively attach all DLLs this one depends on */
     for ( i = 0; i < wm->nDeps; i++ )
     {
@@ -1422,14 +1426,14 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved )
         current_modref = wm;

         call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_LOADED, &wm->ldr );
-        status = MODULE_InitDLL( wm, DLL_PROCESS_ATTACH, lpReserved );
+        status = MODULE_InitDLL( wm, ldr_backup, DLL_PROCESS_ATTACH, lpReserved );
         if (status == STATUS_SUCCESS)
         {
             wm->ldr.Flags |= LDR_PROCESS_ATTACHED;
         }
         else
         {
-            MODULE_InitDLL( wm, DLL_PROCESS_DETACH, lpReserved );
+            MODULE_InitDLL( wm, ldr_backup, DLL_PROCESS_DETACH, lpReserved );
             call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_UNLOADED, &wm->ldr );

             /* point to the name so LdrInitializeThunk can print it */
@@ -1486,13 +1490,14 @@ static void process_detach(void)
 {
     PLIST_ENTRY mark, entry;
     PLDR_MODULE mod;
+    WINE_MODREF *wm;

     mark = &NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList;
     do
     {
         for (entry = mark->Blink; entry != mark; entry = entry->Blink)
         {
-            mod = CONTAINING_RECORD(entry, LDR_MODULE,
+            mod = CONTAINING_RECORD(entry, LDR_MODULE,
                                     InInitializationOrderModuleList);
             /* Check whether to detach this DLL */
             if ( !(mod->Flags & LDR_PROCESS_ATTACHED) )
@@ -1502,7 +1507,8 @@ static void process_detach(void)

             /* Call detach notification */
             mod->Flags &= ~LDR_PROCESS_ATTACHED;
-            MODULE_InitDLL( CONTAINING_RECORD(mod, WINE_MODREF, ldr),
+            wm = CONTAINING_RECORD(mod, WINE_MODREF, ldr);
+            MODULE_InitDLL( wm, wm->ldr,
                             DLL_PROCESS_DETACH, ULongToPtr(process_detaching) );
             call_ldr_notifications( LDR_DLL_NOTIFICATION_REASON_UNLOADED, mod );

@@ -1524,18 +1530,20 @@ static void thread_attach(void)
 {
     PLIST_ENTRY mark, entry;
     PLDR_MODULE mod;
+    WINE_MODREF *wm;

     mark = &NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList;
     for (entry = mark->Flink; entry != mark; entry = entry->Flink)
     {
-        mod = CONTAINING_RECORD(entry, LDR_MODULE,
+        mod = CONTAINING_RECORD(entry, LDR_MODULE,
                                 InInitializationOrderModuleList);
         if ( !(mod->Flags & LDR_PROCESS_ATTACHED) )
             continue;
         if ( mod->Flags & LDR_NO_DLL_CALLS )
             continue;

-        MODULE_InitDLL( CONTAINING_RECORD(mod, WINE_MODREF, ldr), DLL_THREAD_ATTACH, NULL );
+        wm = CONTAINING_RECORD(mod, WINE_MODREF, ldr);
+        MODULE_InitDLL( wm, wm->ldr, DLL_THREAD_ATTACH, NULL );
     }
 }

@@ -3032,7 +3040,7 @@ static NTSTATUS load_dll( const WCHAR *load_path, const WCHAR *libname, const WC
             case LO_DEFAULT:
                 nts = load_builtin_dll( load_path, &nt_name, &module, flags, pwm );
                 if (nts == STATUS_SUCCESS && loadorder == LO_DEFAULT &&
-                    (MODULE_InitDLL( *pwm, DLL_WINE_PREATTACH, NULL ) != STATUS_SUCCESS))
+                    (MODULE_InitDLL( *pwm, (*pwm)->ldr, DLL_WINE_PREATTACH, NULL ) != STATUS_SUCCESS))
                 {
                     /* stub-only dll, try native */
                     TRACE( "%s pre-attach returned FALSE, preferring native\n", debugstr_us(&nt_name) );
@@ -3252,7 +3260,7 @@ IMAGE_BASE_RELOCATION * WINAPI LdrProcessRelocationBlock( void *page, UINT count
  *		LdrQueryProcessModuleInformation
  *
  */
-NTSTATUS WINAPI LdrQueryProcessModuleInformation(PSYSTEM_MODULE_INFORMATION smi,
+NTSTATUS WINAPI LdrQueryProcessModuleInformation(PSYSTEM_MODULE_INFORMATION smi,
                                                  ULONG buf_size, ULONG* req_size)
 {
     SYSTEM_MODULE*      sm = &smi->Modules[0];
@@ -3522,6 +3530,7 @@ void WINAPI LdrShutdownThread(void)
 {
     PLIST_ENTRY mark, entry;
     PLDR_MODULE mod;
+    WINE_MODREF *wm;
     UINT i;
     void **pointers;

@@ -3535,14 +3544,15 @@ void WINAPI LdrShutdownThread(void)
     mark = &NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList;
     for (entry = mark->Blink; entry != mark; entry = entry->Blink)
     {
-        mod = CONTAINING_RECORD(entry, LDR_MODULE,
+        mod = CONTAINING_RECORD(entry, LDR_MODULE,
                                 InInitializationOrderModuleList);
         if ( !(mod->Flags & LDR_PROCESS_ATTACHED) )
             continue;
         if ( mod->Flags & LDR_NO_DLL_CALLS )
             continue;

-        MODULE_InitDLL( CONTAINING_RECORD(mod, WINE_MODREF, ldr),
+        wm = CONTAINING_RECORD(mod, WINE_MODREF, ldr);
+        MODULE_InitDLL( wm, wm->ldr,
                         DLL_THREAD_DETACH, NULL );
     }

--
2.25.1

[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=68207

Your paranoid android.


=== debiant (build log) ===

error: patch failed: dlls/ntdll/loader.c:696
Task: Patch failed to apply

=== debiant (build log) ===

error: patch failed: dlls/ntdll/loader.c:696
Task: Patch failed to apply

[Myah Caron]

Sorry, I think this is because of my email server truncating empty spaces at the end of lines.

I tried creating a new patch without the parts where my editor removed the empty spaces, but it still modifies some lines with trailing spaces.

Would uploading the patch as an attachment be acceptable instead, or is it better just to leave it?
> Sent: Thursday, March 26, 2020 at 8:15 AM
> From: "Marvin" <testbot(a)winehq.org>
> To: qsniyg(a)mail.com
> Cc: wine-devel(a)winehq.org
> Subject: Re: [PATCH] ntdll: Don't fail if LDR_MODULE.Flags is modified during load
>
> 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=68207
>
> Your paranoid android.
>
> === debiant (build log) ===
>
> error: patch failed: dlls/ntdll/loader.c:696
> Task: Patch failed to apply
>
> === debiant (build log) ===
>
> error: patch failed: dlls/ntdll/loader.c:696
> Task: Patch failed to apply

[Zebediah Figura]

On 3/26/20 10:40 AM, Myah Caron wrote:
> Sorry, I think this is because of my email server truncating empty spaces at the end of lines.
>
> I tried creating a new patch without the parts where my editor removed the empty spaces, but it still modifies some lines with trailing spaces.
>
> Would uploading the patch as an attachment be acceptable instead, or is it better just to leave it?

Uploading the patch as an attachment should work, yes.
> > Sent: Thursday, March 26, 2020 at 8:15 AM
> > From: "Marvin" <testbot(a)winehq.org>
> > To: qsniyg(a)mail.com
> > Cc: wine-devel(a)winehq.org
> > Subject: Re: [PATCH] ntdll: Don't fail if LDR_MODULE.Flags is modified during load
> >
> > 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=68207
> >
> > Your paranoid android.
> >
> > === debiant (build log) ===
> >
> > error: patch failed: dlls/ntdll/loader.c:696
> > Task: Patch failed to apply
> >
> > === debiant (build log) ===
> >
> > error: patch failed: dlls/ntdll/loader.c:696
> > Task: Patch failed to apply