Closed Bug 1456054 Opened 2 years ago Closed 2 years ago

Crash in mozilla::interceptor::WritableTargetFunction<T>::ReadByte


(Core :: General, defect, critical)

Windows 10
Not set



Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed


(Reporter: calixte, Assigned: aklotz)


(Blocks 1 open bug)


(Keywords: crash, regression)

Crash Data


(1 file)

This bug was filed from the Socorro interface and is
report bp-14627681-53b3-45f1-9ec0-de7250180422.

Top 10 frames of crashing thread:

0 mozglue.dll mozilla::interceptor::WritableTargetFunction<mozilla::interceptor::MMPolicyInProcess>::ReadByte mozglue/misc/interceptor/TargetFunction.h:133
1 mozglue.dll mozilla::interceptor::WindowsDllDetourPatcher<mozilla::interceptor::VMSharingPolicyUnique<mozilla::interceptor::MMPolicyInProcess, 128> >::Clear mozglue/misc/interceptor/PatcherDetour.h:89
2 mozglue.dll static void `dynamic atexit destructor for 'NtDllIntercept'' 
3 ucrtbase.dll <lambda_f03950bc5685219e0bcd2087efbe011e>::operator 
4 ucrtbase.dll __crt_seh_guarded_call<int>::operator<<lambda_7777bce6b2f8c936911f934f8298dc43>, <lambda_f03950bc5685219e0bcd2087efbe011e>& __ptr64, <lambda_3883c3dff614d5e0c5f61bb1ac94921c> > 
5 ucrtbase.dll execute_onexit_table 
6 mozglue.dll static int dllmain_crt_process_detach f:/dd/vctools/crt/vcstartup/src/startup/dll_dllmain.cpp:105
7 mozglue.dll static int dllmain_dispatch f:/dd/vctools/crt/vcstartup/src/startup/dll_dllmain.cpp:211
8 ntdll.dll LdrpCallInitRoutine 
9 ntdll.dll LdrShutdownProcess 


There are 13 crashes (from 3 installations) in nightly 61 with buildid 20180421220102. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1432653.

Flags: needinfo?(aklotz)
This is... tricky.

My best guess is that something hooked LdrLoadDll ahead of us, and ResolveRedirectedAddress took us to an address that was not backed by an image. That VM was freed at some point, leaving us with a patched target function that no longer exists.

At the very least I should add a check in ResolveRedirectedAddress to ensure that we've locked the target module. If that fails, then we should not redirect.
Assignee: nobody → aklotz
Flags: needinfo?(aklotz)
This is speculative, but based on what I've seen in the crash reports, it looks like ResolveRedirectedAddress chased various jmps into an executable page of VirtualAlloc'd memory. Once that memory was freed, we had a hook pointing into the wrong place.

This patch simple ensures that we don't chase pointers into memory that isn't backed by an image.
Attachment #8970363 - Flags: review?(davidp99)
Comment on attachment 8970363 [details] [diff] [review]
Verify that a redirected target address is accessible and image-backed

Review of attachment 8970363 [details] [diff] [review]:

Yeah, this isn't something I thought was possible when loading DLLs but if someone is doing something evil with DLL loading then... good luck :)
Attachment #8970363 - Flags: review?(davidp99) → review+
Bug 1456054: Verify that a redirected address is accessible and backed by an image; r=handyman
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.