Closed Bug 1456054 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: General, defect, critical)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: calixte, Assigned: aklotz)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(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.

[1] https://hg.mozilla.org/mozilla-central/rev?node=25a807fa07d1
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
Status: NEW → ASSIGNED
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/13863e08584ae95ac1262b517bf8ef06dd29dbdf
Bug 1456054: Verify that a redirected address is accessible and backed by an image; r=handyman
https://hg.mozilla.org/mozilla-central/rev/13863e08584a
Status: ASSIGNED → RESOLVED
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.