Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regex can result in recursive SRW lock acquire through apphelp #12

Closed
jxy-s opened this issue Nov 4, 2022 · 0 comments · Fixed by #13
Closed

regex can result in recursive SRW lock acquire through apphelp #12

jxy-s opened this issue Nov 4, 2022 · 0 comments · Fixed by #13
Assignees

Comments

@jxy-s
Copy link
Owner

jxy-s commented Nov 4, 2022

APPLICATION_VERIFIER_SRWLOCK_RECURSIVE_ACQUIRE (253)
The SRW lock is being acquired recursively by the same thread.
This stop is generated if the SRW lock (Param1) is being acquired
recursively by the same thread.
This will result in a deadlock and the thread would block indefinitely.
Recursive acquisition of an SRW lock in exclusive mode will cause a deadlock.
Recursive acquisition of an SRW lock in shared mode will cause a deadlock when
there is a thread waiting for exclusive access. Consider the example below:
- Thread A acquires the SRW lock in shared mode
- Thread B tries to acquire the SRW lock in exclusive mode and waits
- Thread A tries to acquire the SRW lock in shared mode recursively. This will
  be successful as long as there is no exclusive waiter (in this case B). Since
  SRW locks do not have writer starvation, thread A waits behind thread B.
  Now, Thread B is waiting for Thread A which is inturn waiting for Thread B
  causing a circular wait and hence a deadlock.
$ kb - to get the current stack trace. This is where the SRW lock is being
acquired recursively. 
$ dps Param2 - to get the stack trace for the first acquire. 
Arguments:
Arg1: 0a14afd8, SRW Lock 
Arg2: 02747b64, Address of the first acquire stack trace. Use dps <address> to see where the SRW lock was acquired. 
Arg3: 00000000, Not used 
Arg4: 00000000, Not used 


vrfcore!VerifierStopMessageEx+0x5b8
vfbasics!AVrfpVerifySRWLockAcquire+0xb8
vfbasics!AVrfpRtlAcquireSRWLockShared+0x3a
vfcuzz!VfCuzzRtlAcquireSRWLockShared+0x36
apphelp!SE_GetProcAddressForCaller+0x3af
ntdll!LdrGetProcedureAddressForCaller+0x36b
KERNELBASE!GetProcAddressForCaller+0x4d
KERNEL32!GetProcAddressStub+0x14
vfdynf!try_get_function+0x52
vfdynf!try_get_CompareStringEx+0x16
vfdynf!__acrt_eagerly_load_locale_apis+0xa
vfdynf!_lock_locales+0x5
vfdynf!std::_Lockit::_Lockit+0x14
vfdynf!std::locale::_Init+0x17
vfdynf!std::_Regex_traits<wchar_t>::_Regex_traits<wchar_t>+0x60
vfdynf!std::vector<std::basic_regex<wchar_t,std::regex_traits<wchar_t> >,std::allocator<std::basic_regex<wchar_t,std::regex_tra+0x15d
vfdynf!fault::InitExclusionsRegex+0x106
vfdynf!fault::IsStackOverriddenByRegex+0x6f
vfdynf!fault::ShouldFaultInject+0x7e0
vfdynf!Hook_NtOpenFile+0x5b
apphelp!SeUtilsIsSystem+0x80
apphelp!SepRouterHookIAT+0x112
apphelp!SE_DllLoaded+0x8c
ntdll!LdrpSendPostSnapNotifications+0x119
ntdll!LdrpNotifyLoadOfGraph+0x44
ntdll!LdrpPrepareModuleForExecution+0x4f
ntdll!LdrpLoadDllInternal+0x11e
ntdll!LdrpLoadDll+0x7e
ntdll!LdrLoadDll+0x97
vfbasics!AVrfpLdrLoadDll+0x52
KERNELBASE!LoadLibraryExW+0x14f
[REDACTED]
KERNEL32!BaseThreadInitThunk+0x19
ntdll!__RtlUserThreadStart+0x2b
ntdll!_RtlUserThreadStart+0x1b

I did not realize that regex relies on local support, this code in vfdyn should not be done "once" on first invocation. Rather, it should be done during process attach. So this:
https://github.com/jxy-s/vfdynf/blob/main/vfdynf/vrf_fault.cpp#L100

Should be moved to here:
https://github.com/jxy-s/vfdynf/blob/main/vfdynf/vrf_fault.cpp#L448

And the init_once logic should be removed. I don't remember if there was a reason that I put the regex initialization downstream. I will investigate and apply an appropriate patch.

@jxy-s jxy-s self-assigned this Nov 4, 2022
@jxy-s jxy-s closed this as completed in #13 Jan 13, 2023
@jxy-s jxy-s mentioned this issue Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant