Closed Bug 1592486 Opened 5 years ago Closed 4 years ago

patched_LdrLoadDll hit a null read AV if the TLS storage is not initialized


(Firefox :: Launcher Process, defect, P3)




Firefox 74
Tracking Status
firefox74 --- fixed


(Reporter: toshi, Assigned: toshi)




(1 file)

If our hook is invoked in the very early stage, patched_LdrLoadDll may be called before the TLS storage is initialized, resulting in AV.

The example below is when Firefox is launched with a patch of Bug 1587642 and Windows Defender EAF on. The crash happens when accessing ModuleLoadFrame::sTopFrame.

1:004> r
rax=0000000000000000 rbx=0000000000000000 rcx=0000000000000000
rdx=00000079311fdeb0 rsi=00000079311fdde0 rdi=00000079311fdeb0
rip=00007ff6e430619e rsp=00000079311fdd70 rbp=0000000000071000
 r8=00000079311fdeb0  r9=00007ff9edbdbd70 r10=0000000000000000
r11=00007ff6e43060e0 r12=0000000000000000 r13=00000079309ab000
r14=00007ff9edbdbd70 r15=00007ffa00ba0000
iopl=0         nv up ei pl nz na pe nc
cs=0033  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010202
firefox!mozilla::detail::ThreadLocalNativeStorage<mozilla::freestanding::ModuleLoadFrame *>::get+0xf [inlined in firefox!mozilla::freestanding::ModuleLoadFrame::ModuleLoadFrame+0x1e]:
00007ff6`e430619e 488b04c1        mov     rax,qword ptr [rcx+rax*8] ds:00000000`00000000=????????????????
1:004> knL
 # Child-SP          RetAddr           Call Site
00 (Inline Function) --------`-------- firefox!mozilla::detail::ThreadLocalNativeStorage<mozilla::freestanding::ModuleLoadFrame *>::get+0xf
01 (Inline Function) --------`-------- firefox!mozilla::detail::ThreadLocal<mozilla::freestanding::ModuleLoadFrame *,::mozilla::detail::ThreadLocalNativeStorage>::get+0xf
02 00000079`311fdd70 00007ff6`e4306117 firefox!mozilla::freestanding::ModuleLoadFrame::ModuleLoadFrame+0x1e
03 00000079`311fddc0 00007ff9`edbaa861 firefox!mozilla::freestanding::patched_LdrLoadDll+0x37
04 00000079`311fde80 00007ff9`edba949c verifier!MitLibInitialize+0xe1
05 00000079`311fdf00 00007ffa`00bc50a1 verifier!DllMain+0xa4
06 00000079`311fe140 00007ffa`00c09405 ntdll!LdrpCallInitRoutine+0x65
07 00000079`311fe1b0 00007ffa`00c091f8 ntdll!LdrpInitializeNode+0x1b1
08 00000079`311fe2f0 00007ffa`00c7905e ntdll!LdrpInitializeGraphRecurse+0x80
09 00000079`311fe330 00007ffa`00c73ee1 ntdll!AVrfInitializeVerifier+0x2d6
0a 00000079`311ff440 00007ffa`00c61db5 ntdll!LdrpInitializeProcess+0x1799
0b 00000079`311ff880 00007ffa`00c11853 ntdll!_LdrpInitialize+0x50549
0c 00000079`311ff920 00007ffa`00c117fe ntdll!LdrpInitialize+0x3b
0d 00000079`311ff950 00000000`00000000 ntdll!LdrInitializeThunk+0xe

We had a thread-local varialbe ModuleLoadFrame::sTopFrame to track the topmost
stack frame of LdrLoadDll. However, our hook function patched_LdrLoadDll can
be called even before TLS is initialized. In such a case, accessing sTopFrame
causes AV.

This patch introduces SafeThreadLocal to safely access a thread-local varialbe.
If TLS is not initialized, it falls back to a global variable because in that
early stage there is only a single thread running.

Pushed by
Store ModuleLoadFrame::sTopFrame as a global variable if TLS is not ready.  r=aklotz

Updated the patch. The test had failed because PGO build optimized out a necessary code.

Flags: needinfo?(tkikuchi)
Pushed by
Store ModuleLoadFrame::sTopFrame as a global variable if TLS is not ready.  r=aklotz
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74
You need to log in before you can comment on or make changes to this bug.