0

I am doing a global hook to add my DLL to the hook chain:

HHOOK handle = SetWindowsHookEx(WH_CALLWNDPROC, addr, dll, 0);

Inside my DLL I am using Detours to intercept several WINAPI function calls. Everything works fine, except for WaitForSingleObject calls. Whenever I add WaitForSingleObject to the detoured functions, several programs crash when I unhook my DLL (Chrome, Skype, ...). Here is how the DLL looks:

DWORD (WINAPI* Real_WaitForSingleObject)( HANDLE hHandle, DWORD dwMilliseconds) = WaitForSingleObject;
DWORD WINAPI Mine_WaitForSingleObject(HANDLE hHandle, DWORD dwMilliseconds);
INT APIENTRY DllMain(HMODULE hDLL, DWORD Reason, LPVOID Reserved) {

    switch(Reason) {
        case DLL_PROCESS_ATTACH: 
            DetourTransactionBegin();
            DetourUpdateThread(GetCurrentThread());
            DetourAttach(&(PVOID&)Real_WaitForSingleObject, Mine_WaitForSingleObject);
            DetourTransactionCommit();
            break;
        case DLL_PROCESS_DETACH: 
            DetourTransactionBegin(); 
            DetourUpdateThread(GetCurrentThread());
            DetourDetach(&(PVOID&)Real_WaitForSingleObject, Mine_WaitForSingleObject);
            DetourTransactionCommit();
            break;
        case DLL_THREAD_ATTACH:

            break;
        case DLL_THREAD_DETACH:

            break;
    }
    return TRUE;
}
DWORD WINAPI Mine_WaitForSingleObject(HANDLE hHandle, DWORD dwMilliseconds) {

    return Real_WaitForSingleObject(hHandle, dwMilliseconds);
}

extern "C" __declspec(dllexport) int meconnect(int code, WPARAM wParam, LPARAM lParam) {

    return CallNextHookEx(NULL, code, wParam, lParam);
}

Could someone help me to understand why this is happening and how I can get around that Problem? Thanks!

moccajoghurt
  • 131
  • 6
  • Hardly surprising that such egregious hackery would lead to problems. – David Heffernan Nov 14 '15 at 22:11
  • 1
    Presumably, when you remove the hook the DLL gets unloaded. You remove the detour, but that won't stop threads that are already in the middle of a detoured call from crashing. The only reason WaitForSingleObject is particularly likely to cause a crash is that it tends to block for long periods. (The other detoured functions will only crash occasionally, because most of the time they won't be running at the exact point at which the DLL is unloaded.) – Harry Johnston Nov 14 '15 at 22:28
  • Is there a way to safely unload the DLL without causing detoured calls to crash? Isn't DetourDetach() supposed to prevent such crashes? – moccajoghurt Nov 14 '15 at 23:23
  • I don't see how it could, particularly in this context. But you should consult the documentation. – Harry Johnston Nov 15 '15 at 03:17
  • What you are doing is beyond the pale. You should not expect to be able to do this. How could you expect code to continue executing after you unload it? How could it be possible to unload the code safely? – David Heffernan Nov 15 '15 at 08:16
  • Did you read the documentation for [DllMain](https://msdn.microsoft.com/en-us/library/windows/desktop/ms682583.aspx), specifically the document on [Dynamic-Link Library Best Practices](https://msdn.microsoft.com/en-us/library/windows/desktop/dn633971.aspx)? Your code suggests, that you haven't. – IInspectable Nov 15 '15 at 14:45

2 Answers2

2

I think this is happening because many programs (Chrome, Skype, ...) have a thread pool, where background thread[s] are waiting on WaitForSingleObject() for something interesting for them to happen, and when it does happen, that thread[s] wake up and do something.

So, your thread A is calling DetourDetach while another thread B of the same process is currently inside Mine_WaitForSingleObject() Then DLL unloads, and everything crashes. You can verify by using debugger, attach to that problematic process, set breakpoint in DLL_PROCESS_DETACH, and when the breakpoint will hit, look through the stacks of another threads for Mine_WaitForSingleObject.

I’m not sure how to fix. But, one way that you might try — enumerate threads, and call DetourUpdateThread() for every thread of the process. This way, maybe the Detours will do something about that.

Soonts
  • 20,079
  • 9
  • 57
  • 130
  • `DetourUpdateThread` rewrites instruction pointers "within the rewritten code in either the target function or the trampoline function." In other words, if it's about to delete the trampoline, but the thread is executing the trampoline, then it redirects the thread to execute the corresponding instructions in the original function. It doesn't unwind the stack (not that it would know how, anyway), so if `Mine_WaitForSingleObject` is on the stack, it will still be inside after you update the thread. – Raymond Chen Nov 15 '15 at 15:47
2

You are detouring a function that almost any process uses. And it is particularly dangerous since it is very likely that such a process has a call on that function active. A blocking call in almost any case. As soon as it unblocks, the code will resume into your detour that is no longer there.

Kaboom.

Realistically, the only way to unload your detour is by logging out so that every process that could have been detoured is no longer running.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536