NtGetNextThread exception in UWP apps
emoose opened this issue · 10 comments
For some reason in Minecraft NtGetNextThread throws an exception with code 0xC0000008: An invalid handle was specified., causing game crash, this is a UWP game afaik but I did use some launcher to set it up to let me copy DLLs into it.
IIRC some APIs are restricted in UWP land, guess maybe NtGetNextThread is one of them? Never had this exception in any non-UWP apps at least.
Call stack:
ntdll.dll!KiRaiseUserExceptionDispatcher() Unknown
> dxgi.dll!safetyhook::ThreadFreezer::ThreadFreezer(void) C++
dxgi.dll!safetyhook::InlineHook::e9_hook(class std::shared_ptr<class safetyhook::Allocator> const &) C++
dxgi.dll!safetyhook::InlineHook::create(class std::shared_ptr<class safetyhook::Allocator> const &,unsigned __int64,unsigned __int64) C++
dxgi.dll!safetyhook::InlineHook::create(unsigned __int64,unsigned __int64) C++
dxgi.dll!safetyhook::create_inline(void *,void *) C++
dxgi.dll!nvngx::init(struct HINSTANCE__ *) C++FWIW the older toolhelp method did used to work fine on this game too.
If you own Minecraft Bedrock (or have Xbox Game Pass PC) you can use https://github.com/bedrockLauncher/BedrockLauncher to set the game up in a writable folder, then build a wrapper DLL to load into it and try creating an inline hook.
E: strange, if I run with debugger attached and manually continue past the exception it runs fine, seems that some of the calls to NtGetNextThread are also successful too, not sure why this is causing a crash when debugger isn't attached...
(also disabling the exception type in VS debugger is still making it prompt for each exception too, not sure if that means anything)
Seems wrapping the NtGetNextThread call in a SEH handler works to stop it crashing the whole app:
// NtGetNextThread may throw a SEH exception that causes some apps to crash, this helper function just captures the exception code from that
DWORD SafeNtGetNextThread(HANDLE ProcessHandle, HANDLE ThreadHandle, ACCESS_MASK DesiredAccess, ULONG HandleAttributes,
ULONG Flags, PHANDLE NewThreadHandle) {
__try {
return NtGetNextThread(ProcessHandle, ThreadHandle, DesiredAccess, HandleAttributes, Flags, NewThreadHandle);
} __except (EXCEPTION_EXECUTE_HANDLER) {
return GetExceptionCode();
}
}(couldn't put that into ThreadFreezer ctor since VS would complain with error C2712: Cannot use __try in functions that require object unwinding, works fine as a helper func though)
Wonder why this exception is only happening on UWP games though, could UWP maybe be turning something on that would make it throw exceptions instead of just returning status code?
If that could be disabled during the ThreadFreezer code instead it might be better than polluting the lib with SEH stuff.
E: the game now seems to hang sometimes during/right after the hook init though, but not completely sure if it's ThreadFreezer related yet, maybe possible the exceptions here are actually something unusual, and now it's not fully freezing/unfreezing everything that it should be?
E2: changing ThreadFreezer to just immediately return seems to get rid of all the hanging, so guess it is still something thread related :/
From what I can tell hang usually happens inside nvngx.dll NVSDK_NGX_D3D12_Init_Ext func, that func has a while(true) that just loops until some variable is set.
The var it checks for seems to be setup in some thread dedicated to it, but I guess that thread either isn't starting or is being frozen & then not unfrozen for some reason, odd.
(it does sometimes hang in completely different DLLs like twinui stuff though, so don't think the problem is specific to nvngx itself)
Is it when you hook any API or a specific one? If it's a specific API can you provide a minimal example so I can repro it? Thanks.
Edit: I created a test project that just hooks a win32 API I know will get called (PeekMessage) and it seems like I can repro the crash in Minecraft Bedrock Edition. I'll look into it further this weekend. Thanks for the bug report.
I created a PR #31 that should address this issue. Can you try it and let me know if it fixes it for you?
Aha looks like crash is gone with that, thanks for the prompt fix!
I think the hanging issue might have been a brainfart on my part, my DLL has a known issue with debug builds on some games, and of course I was using debug build to find that exception 😅, release build is working fine though.
(also with the latest commit there it looks like debug builds might also be working for me now too, need to check some other games besides MC that I know hated debug, but seems promising)
E: hmm, seem to have hit a hang in the new HeapAllocator though (using release build)
> ntdll.dll!NtWaitForAlertByThreadId�() Unknown
ntdll.dll!RtlAcquireSRWLockExclusive() Unknown
ntdll.dll!RtlpHpVsContextAllocateInternal() Unknown
ntdll.dll!RtlpAllocateHeapInternal() Unknown
ntdll.dll!RtlpHpAllocWithExceptionProtection�() Unknown
nvngx.dll!std::vector<struct safetyhook::ThreadFreezer::FrozenThread,struct safetyhook::ThreadFreezer::HeapAllocator<struct safetyhook::ThreadFreezer::FrozenThread> >::_Emplace_reallocate<struct safetyhook::ThreadFreezer::FrozenThread>(struct safetyhook::ThreadFreezer::FrozenThread * const,struct safetyhook::ThreadFreezer::FrozenThread &&) C++
nvngx.dll!safetyhook::ThreadFreezer::ThreadFreezer(void) C++
nvngx.dll!safetyhook::InlineHook::e9_hook(class std::shared_ptr<class safetyhook::Allocator> const &) C++
nvngx.dll!safetyhook::InlineHook::create(class std::shared_ptr<class safetyhook::Allocator> const &,unsigned __int64,unsigned __int64) C++
nvngx.dll!safetyhook::MidHook::create(class std::shared_ptr<class safetyhook::Allocator> const &,unsigned __int64,void ) C++
nvngx.dll!safetyhook::MidHook::create(unsigned __int64,void ) C++
nvngx.dll!safetyhook::create_mid(void *,void ) C++
nvngx.dll!nvngx_dlss::hook(struct HINSTANCE__ *) C++
nvngx.dll!nvngx_dlss::init(struct HINSTANCE__ *) C++
nvngx.dll!LoaderNotificationCallback(unsigned long,union LDR_DLL_NOTIFICATION_DATA const *,void *) C++
It's not shown there but seems to be the HeapAlloc call which is then calling into RtlpHpAllocWithExceptionProtection etc.
This hang isn't that common though, but wonder if it might be because I'm calling from inside LdrRegisterDllNotification callback, might make the callback be inside the loader lock...
There is a way I can change this to make it run create_mid etc outside of the loader lock though, will give that a try soon, but would be great if this could work regardless of loader lock.
“NtWaitForAlertByThreadId” is what was what we were hanging from before in the Debug builds. The idea was to allocate before thread freezing and/or use a different allocator. If what you’re showing is true, then we’re still hanging from it even with HeapAlloc (instead of malloc/dbg malloc)… which means we’re at square one again.
The issue you originally had was probably from the library not closing handles correctly (which should be fixed now).
The previous solution was to allocate before freezing threads to avoid that syscall from happening. Obviously this isnt too ideal because it assumes 1024 threads (hope you dont have 1025 threads?) and allocates a chunk of memory (1024 FrozenThread entries).
I’m unsure though and just throwing out some ideas.
Adding m_frozen_threads.reserve(1024); to the top of ThreadFreezer ctor seems like it helped, guessing it doesn't have to call that _Emplace_reallocate during it now, but then the worry is if it ever has more than 1024 as you said :/
Wonder if it could be worth doing a dry run of NtGetNextThread just to count the threads without freezing them, then reserve/alloc for them, and then do the actual freezing after?
Could be a chance of more threads spawning in between though I guess, unless there's some way to lock something to prevent that?
(alternately, do a dry run that just inserts thread IDs from NtGetNextThread into m_frozen_threads without freezing, then loop over m_frozen_threads to handle freezing them/fetching CONTEXT/etc afterward - sounds like the issue is that allocs are happening during freeze, so maybe something like that could get around it?)
E: seems NtQuerySystemInformation can provide a NumberOfThreads field per-process, might be of use: https://github.com/microsoft/Detours/blob/e138422d5cdbc36f56c516cb22ebdb192862438c/src/detours.cpp#L2166
Also gives a list of threads for the process too, but not sure if it's as complete as what NtGetNextThread gives.
It gets complicated. Like you said, when you do a dry run or count threads, a new thread could’ve been created during that time. The current code with the two while loops makes sure that doesn’t happen. I don’t know though, honestly. I for one don’t like the idea of using these undocumented/semi documented APIs. Theres new documented functions such as “PssCaptureSnapshot” which is apparently fast, but it’s windows 8.1 and up only. If it were up to me, I’d just use that. Sadly it’s not ideal because I assume we want to support more windows versions other than 8.1/10/11 (if you ask me, I wouldn’t support operating systems before 10 anyway, but im biased). This doesn’t help with the weird allocation issue though and I have no idea if theres a lock to fix this. It’s such a deep rabbit hole of reverse engineering.
I’m not sure if Microsoft’s Detours fix the issue either or even run into it to begin with, I didn’t read too much.
What has me worried is I thought this was only an issue in Debug builds when the allocator calls new and the CRT calls into dbg malloc.. but you say you had it happen in a release build with HeapAlloc. Guess we really need a better solution.
Tried it with a couple different games and all seemed fine, even ones that had trouble with this before like Cyberpunk2077, good stuff, hope it can be merged soon once you've tested it more.
Only thought I had about it was with the handling of already suspended threads, made a comment on the PR about it, it's probably a very rare case for it to run into though.