ntdll: Call system hook in LdrResolveDelayLoadedAPI when dll hook is missing

[André Hentschel]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46089
Signed-off-by: Andr� Hentschel <nerv(a)dawncrow.de>
---
 dlls/kernel32/tests/loader.c | 39 +++++++++++++++++++++++++++++++++---
 dlls/ntdll/loader.c          | 23 +++++++++++++++++++--
 include/delayloadhandler.h   |  1 +
 3 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c
index bc88c5f6cf..f5204fec0d 100644
--- a/dlls/kernel32/tests/loader.c
+++ b/dlls/kernel32/tests/loader.c
@@ -48,7 +48,7 @@ struct PROCESS_BASIC_INFORMATION_PRIVATE
 };
 
 static LONG *child_failures;
-static WORD cb_count;
+static WORD cb_count, cb_count_sys;
 static DWORD page_size;
 static BOOL is_win64 = sizeof(void *) > sizeof(int);
 static BOOL is_wow64;
@@ -70,7 +70,8 @@ static NTSTATUS (WINAPI *pLdrUnlockLoaderLock)(ULONG, ULONG_PTR);
 static void (WINAPI *pRtlAcquirePebLock)(void);
 static void (WINAPI *pRtlReleasePebLock)(void);
 static PVOID    (WINAPI *pResolveDelayLoadedAPI)(PVOID, PCIMAGE_DELAYLOAD_DESCRIPTOR,
-                                                 PDELAYLOAD_FAILURE_DLL_CALLBACK, PVOID,
+                                                 PDELAYLOAD_FAILURE_DLL_CALLBACK,
+                                                 PDELAYLOAD_FAILURE_SYSTEM_ROUTINE,
                                                  PIMAGE_THUNK_DATA ThunkAddress,ULONG);
 static PVOID (WINAPI *pRtlImageDirectoryEntryToData)(HMODULE,BOOL,WORD,ULONG *);
 static DWORD (WINAPI *pFlsAlloc)(PFLS_CALLBACK_FUNCTION);
@@ -3339,6 +3340,14 @@ static PVOID WINAPI failuredllhook(ULONG ul, DELAYLOAD_INFO* pd)
     return (void*)0xdeadbeef;
 }
 
+static PVOID WINAPI failuresyshook(const char *dll, const char *function)
+{
+    ok(!strcmp(dll, "secur32.dll"), "wrong dll: %s\n", dll);
+    ok(!((ULONG_PTR)function >> 16), "expected ordinal, got %p\n", function);
+    cb_count_sys++;
+    return NULL;
+}
+
 static void test_ResolveDelayLoadedAPI(void)
 {
     static const char test_dll[] = "secur32.dll";
@@ -3573,8 +3582,30 @@ static void test_ResolveDelayLoadedAPI(void)
                 load = (void *)GetProcAddress(htarget, (char*)iibn->Name);
             }
 
+            /* test without failure dll callback */
+            cb_count = 0;
+            cb_count_sys = 0;
+            ret = pResolveDelayLoadedAPI(hlib, delaydir, NULL, failuresyshook, &itda[i], 0);
+            if (td[i].succeeds)
+            {
+                ok(ret != NULL, "Test %u: ResolveDelayLoadedAPI failed\n", i);
+                ok(ret == load, "Test %u: expected %p, got %p\n", i, load, ret);
+                ok(ret == (void*)itda[i].u1.AddressOfData, "Test %u: expected %p, got %p\n",
+                   i, ret, (void*)itda[i].u1.AddressOfData);
+                ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count);
+                ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys);
+            }
+            else
+            {
+                ok(!ret, "Test %u: ResolveDelayLoadedAPI succeeded with %p\n", i, ret);
+                ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count);
+                ok(cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys);
+            }
+
+            /* test with failure dll callback */
             cb_count = 0;
-            ret = pResolveDelayLoadedAPI(hlib, delaydir, failuredllhook, NULL, &itda[i], 0);
+            cb_count_sys = 0;
+            ret = pResolveDelayLoadedAPI(hlib, delaydir, failuredllhook, failuresyshook, &itda[i], 0);
             if (td[i].succeeds)
             {
                 ok(ret != NULL, "Test %u: ResolveDelayLoadedAPI failed\n", i);
@@ -3582,11 +3613,13 @@ static void test_ResolveDelayLoadedAPI(void)
                 ok(ret == (void*)itda[i].u1.AddressOfData, "Test %u: expected %p, got %p\n",
                    i, ret, (void*)itda[i].u1.AddressOfData);
                 ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count);
+                ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys);
             }
             else
             {
                 ok(ret == (void*)0xdeadbeef, "Test %u: ResolveDelayLoadedAPI succeeded with %p\n", i, ret);
                 ok(cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count);
+                ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys);
             }
         }
         delaydir++;
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index c5e8d0d7c1..838be31c11 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -2934,7 +2934,8 @@ BOOLEAN WINAPI RtlDllShutdownInProgress(void)
  *              LdrResolveDelayLoadedAPI   (NTDLL.@)
  */
 void* WINAPI LdrResolveDelayLoadedAPI( void* base, const IMAGE_DELAYLOAD_DESCRIPTOR* desc,
-                                       PDELAYLOAD_FAILURE_DLL_CALLBACK dllhook, void* syshook,
+                                       PDELAYLOAD_FAILURE_DLL_CALLBACK dllhook,
+                                       PDELAYLOAD_FAILURE_SYSTEM_ROUTINE syshook,
                                        IMAGE_THUNK_DATA* addr, ULONG flags )
 {
     IMAGE_THUNK_DATA *pIAT, *pINT;
@@ -2992,7 +2993,25 @@ fail:
     delayinfo.TargetModuleBase = *phmod;
     delayinfo.Unused = NULL;
     delayinfo.LastError = nts;
-    return dllhook(4, &delayinfo);
+
+    if (!dllhook && syshook)
+    {
+        if (IMAGE_SNAP_BY_ORDINAL(pINT[id].u1.Ordinal))
+        {
+            DWORD_PTR ord = LOWORD(pINT[id].u1.Ordinal);
+            dllhook = syshook(name, (char*)ord);
+        }
+        else
+        {
+            const IMAGE_IMPORT_BY_NAME* iibn = get_rva(base, pINT[id].u1.AddressOfData);
+            dllhook = syshook(name, (char*)iibn->Name);
+        }
+    }
+
+    if (dllhook)
+        return dllhook(4, &delayinfo);
+    else
+        return NULL;
 }
 
 /******************************************************************
diff --git a/include/delayloadhandler.h b/include/delayloadhandler.h
index 06b658990c..e8336111ff 100644
--- a/include/delayloadhandler.h
+++ b/include/delayloadhandler.h
@@ -47,6 +47,7 @@ typedef struct _DELAYLOAD_INFO
 } DELAYLOAD_INFO, *PDELAYLOAD_INFO;
 
 typedef PVOID (WINAPI *PDELAYLOAD_FAILURE_DLL_CALLBACK)(ULONG, PDELAYLOAD_INFO);
+typedef PVOID (WINAPI *PDELAYLOAD_FAILURE_SYSTEM_ROUTINE)(LPCSTR, LPCSTR);
 
 #ifdef __cplusplus
 }
-- 
2.17.1

[Marvin]

Hi,

While running your changed tests on Windows, 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=44007

Your paranoid android.


=== debian9 (64 bit Wow Wine report) ===

kernel32:
loader.c:531: Test failed: 1273: loading failed err 193
loader.c:531: Test failed: 1277: loading failed err 193
loader.c:531: Test failed: 1285: loading failed err 193

=== debian9 (build log) ===

[André Hentschel]

Am 06.11.18 um 22:00 schrieb Marvin:
> Hi,
>
> While running your changed tests on Windows, 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=44007
>
> Your paranoid android.
>
> === debian9 (64 bit Wow Wine report) ===
>
> kernel32:
> loader.c:531: Test failed: 1273: loading failed err 193
> loader.c:531: Test failed: 1277: loading failed err 193
> loader.c:531: Test failed: 1285: loading failed err 193
>
> === debian9 (build log) ===

those are pre-existing

[Francois Gouget]

On Wed, 7 Nov 2018, André Hentschel wrote:
[...]
> > kernel32:
> > loader.c:531: Test failed: 1273: loading failed err 193
> > loader.c:531: Test failed: 1277: loading failed err 193
> > loader.c:531: Test failed: 1285: loading failed err 193
>
> those are pre-existing

The reason why the TestBot does not notice is that the error message 
contains the line number. I'm not talking about the initial ':531' (that 
one is ignored) but the ': 1273:' that follows which identifies the 
map_image_section() call site. That changed so it thinks it's a new 
error.

That should only be an issue when the test is modified so that it's not 
a big deal.

Otherwise a solution may be to use ok_() in map_image_section() so it 
reports the line of the call site. This means losing the exact 
map_image_section() line where the error occurred but that's probably ok 
since each error message is easily identifiable.

-- 
Francois Gouget <fgouget(a)codeweavers.com>

[Alexandre Julliard]

André Hentschel <nerv(a)dawncrow.de> writes:
> @@ -2992,7 +2993,25 @@ fail:
>      delayinfo.TargetModuleBase = *phmod;
>      delayinfo.Unused = NULL;
>      delayinfo.LastError = nts;
> -    return dllhook(4, &delayinfo);
> +
> +    if (!dllhook && syshook)
> +    {
> +        if (IMAGE_SNAP_BY_ORDINAL(pINT[id].u1.Ordinal))
> +        {
> +            DWORD_PTR ord = LOWORD(pINT[id].u1.Ordinal);
> +            dllhook = syshook(name, (char*)ord);
> +        }
> +        else
> +        {
> +            const IMAGE_IMPORT_BY_NAME* iibn = get_rva(base, pINT[id].u1.AddressOfData);
> +            dllhook = syshook(name, (char*)iibn->Name);
> +        }
> +    }
> +
> +    if (dllhook)
> +        return dllhook(4, &delayinfo);
> +    else
> +        return NULL;

Is the syshook really supposed to return another hook? This would need
test cases.

-- 
Alexandre Julliard
julliard(a)winehq.org