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

Status: RESOLVED FIXED

(Firefox :: Launcher Process, defect, P3)

Description

[Toshihito Kikuchi [:toshi]]

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

Comment 1

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1592486 - Store ModuleLoadFrame::sTopFrame as a global variable if TLS is not ready. r=aklotz

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6866be136e53
Store ModuleLoadFrame::sTopFrame as a global variable if TLS is not ready.  r=aklotz

Comment 3

[Narcis Beleuzu [:NarcisB]]

Backed out for cppunit failures on TestNativeNt.exe

Back out link: https://hg.mozilla.org/integration/autoland/rev/30d70d402f129680a93490db6898d933013b67b3
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=287706602&repo=autoland&lineNumber=1182

Comment 4

[Toshihito Kikuchi [:toshi]]

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd480c4e9afe
Store ModuleLoadFrame::sTopFrame as a global variable if TLS is not ready.  r=aklotz

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/fd480c4e9afe