1432010 - Crash in xul!mozilla::DOMEventTargetHelper::GetParentObject

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect, P1)

Description

[Pawel Wylecial]

Attachment: Crashing testcase

(eacc.fe28): Access violation - code c0000005 (!!! second chance !!!)
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.
eax=a2a39c63 ebx=8fe3f9b8 ecx=00000000 edx=4c434f78 esi=00000000 edi=8b098000
eip=519f33c9 esp=8fe3f928 ebp=8fe3f9b0 iopl=0         nv up ei ng nz na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010286
xul!mozilla::DOMEventTargetHelper::GetParentObject+0x2 [inlined in xul!`anonymous namespace'::MessageEventRunnable::DispatchDOMEvent+0x32]:
519f33c9 8b06            mov     eax,dword ptr [esi]  ds:002b:00000000=????????
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.
0:305> kb
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.
 # ChildEBP RetAddr  Args to Child              
00 (Inline) -------- -------- -------- -------- xul!mozilla::DOMEventTargetHelper::GetParentObject+0x2 [z:\build\build\src\obj-firefox\dist\include\mozilla\domeventtargethelper.h @ 156] 
01 8fe3f9b8 51a72e04 8b098000 8e2507e0 00000000 xul!`anonymous namespace'::MessageEventRunnable::DispatchDOMEvent+0x32 [z:\build\build\src\dom\workers\workerprivate.cpp @ 713] 
02 8fe3f9e0 51a72ad6 8b098000 8e21bc00 8e2507e0 xul!`anonymous namespace'::MessageEventRunnable::WorkerRun+0x53 [z:\build\build\src\dom\workers\workerprivate.cpp @ 817] 
03 8fe3faac 51a495a9 8e2507e0 8e21bc00 80000000 xul!mozilla::dom::workers::WorkerRunnable::Run+0x11f [z:\build\build\src\dom\workers\workerrunnable.cpp @ 376] 
04 8fe3fb3c 51a72f3b 8b073920 80000000 8fe3fb57 xul!nsThread::ProcessNextEvent+0x15e [z:\build\build\src\xpcom\threads\nsthread.cpp @ 1040] 
05 8fe3fb58 51a71fa9 8e26a400 8b098000 8e2514dc xul!NS_ProcessNextEvent+0x1a [z:\build\build\src\xpcom\threads\nsthreadutils.cpp @ 521] 
06 8fe3fbdc 51d7da3c 8b098000 8e26a400 8b073920 xul!mozilla::dom::workers::WorkerPrivate::DoRunLoop+0x12d [z:\build\build\src\dom\workers\workerprivate.cpp @ 5191] 
07 8fe3fd84 51a495a9 8e26a400 8b0739d0 8b073900 xul!`anonymous namespace'::WorkerThreadPrimaryRunnable::Run+0x13c [z:\build\build\src\dom\workers\runtimeservice.cpp @ 2842] 
08 8fe3fe10 51a72f3b 8b073920 8b073900 8fe3fe2b xul!nsThread::ProcessNextEvent+0x15e [z:\build\build\src\xpcom\threads\nsthread.cpp @ 1040] 
09 8fe3fe2c 51a72eaf 8b0739d0 8b0739d0 64c92da0 xul!NS_ProcessNextEvent+0x1a [z:\build\build\src\xpcom\threads\nsthreadutils.cpp @ 521] 
0a 8fe3fe50 51d092fd 8b0739d0 a2a39b5b 8b073920 xul!mozilla::ipc::MessagePumpForNonMainThreads::Run+0x95 [z:\build\build\src\ipc\glue\messagepump.cpp @ 338] 
0b (Inline) -------- -------- -------- -------- xul!MessageLoop::RunInternal+0x8 [z:\build\build\src\ipc\chromium\src\base\message_loop.cc @ 326] 
0c 8fe3fe88 51d092bb 8b0739d0 00000001 64c92d00 xul!MessageLoop::RunHandler+0x20 [z:\build\build\src\ipc\chromium\src\base\message_loop.cc @ 320] 
0d 8fe3fea8 51d093fa 75ec6770 7f3a55c0 7f3a5670 xul!MessageLoop::Run+0x19 [z:\build\build\src\ipc\chromium\src\base\message_loop.cc @ 300] 
0e 8fe3fecc 64a8b259 010f7ec8 4c449d50 64a80cac xul!nsThread::ThreadFunc+0xb5 [z:\build\build\src\xpcom\threads\nsthread.cpp @ 429] 
0f 8fe3feec 64a80cb9 7f3a55c0 8fe3ff34 75dcaa5d nss3!_PR_NativeRunThread+0xcc [z:\build\build\src\nsprpub\pr\src\threads\combined\pruthr.c @ 397] 
10 8fe3fef8 75dcaa5d 7f3a55c0 d86a8c33 75dcaa10 nss3!pr_root+0xd [z:\build\build\src\nsprpub\pr\src\md\windows\w95thred.c @ 95] 
11 8fe3ff34 75ec8654 4c449d50 00000000 6f9801bc ucrtbase!thread_start<unsigned int (__stdcall*)(void *)>+0x4d
12 8fe3ff48 64c9bb8c 4c449d50 75dcaa10 75ec8630 KERNEL32!BaseThreadInitThunk+0x24
13 8fe3ff5c 77404a47 4c449d50 ccf7dbce 00000000 mozglue!patched_BaseThreadInitThunk+0x27 [z:\build\build\src\mozglue\build\windowsdllblocklist.cpp @ 833] 
14 8fe3ffa4 77404a17 ffffffff 77429ecf 00000000 ntdll!__RtlUserThreadStart+0x2f
15 8fe3ffb4 00000000 75dcaa10 4c449d50 00000000 ntdll!_RtlUserThreadStart+0x1b
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.
0:305> d eax
a2a39c63  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39c73  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39c83  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39c93  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39ca3  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39cb3  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39cc3  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
a2a39cd3  41 44 43 42 41 44 43 42-41 44 43 42 41 44 43 42  ADCBADCBADCBADCB
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.
0:305> u .
xul!mozilla::DOMEventTargetHelper::GetParentObject+0x2 [z:\build\build\src\dom\workers\workerprivate.cpp @ 713] [inlined in xul!`anonymous namespace'::MessageEventRunnable::DispatchDOMEvent+0x32 [z:\build\build\src\dom\workers\workerprivate.cpp @ 713]]:
519f33c9 8b06            mov     eax,dword ptr [esi]
519f33cb 897db0          mov     dword ptr [ebp-50h],edi
519f33ce 897dd8          mov     dword ptr [ebp-28h],edi
519f33d1 8975a0          mov     dword ptr [ebp-60h],esi
519f33d4 ff5058          call    dword ptr [eax+58h]
519f33d7 50              push    eax
519f33d8 8d4dd4          lea     ecx,[ebp-2Ch]
519f33db e86a0cf1ff      call    xul!nsCOMPtr<nsIGlobalObject>::nsCOMPtr<nsIGlobalObject> (5190404a)
WARNING: Stack pointer is outside the normal stack bounds. Stack unwinding can be inaccurate.

Comment 1

[Andrew McCreight [:mccr8]]

Looking at the stack, it might be some kind of UAF involving workers and request idle callback. (Though the idle callback would presumably mostly be a mechanism to run a bit of code later after something went away rather than a direct vector.)

Comment 2

[Daniel Veditz [:dveditz]]

I don't see any crash in a 32-bit Firefox 57. In the console I get two exceptions "too much recursion" followed by two "uncaught exception: out of memory". Looking at the code those seem completely expected, and seem to protect us from the crash. Of course I'm running the 32-bit version on a beefy 64-bit windows machine with a lot of memory.

Pawel: what are the limits of the machine (or VM) you're testing on? Maybe our limits are too generous for older machines (a guess).

Comment 3

[Pawel Wylecial]

I tested it on a Win10 64-bit machine, running 32-bit FF (i7, 32 GB). Latest 57.0.4 stable, the issue started appearing from version 55.0.3 as mentioned in the PoC.

Comment 4

[Andreas Farre [:farre]]

Probably not a rIC-related issue, with:

< 	window.requestIdleCallback(handler);
---
> 	window.setTimeout(handler, 100);

I still get a crash, but I end up with:

Thread 5 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 8103.8890]
(anonymous namespace)::ChildImpl::GetOrCreateForCurrentThread () at /home/farre/src/gecko/work-1/ipc/glue/BackgroundImpl.cpp:1506
1506	    MOZ_CRASH("Failed to create top level actor!");
(rr) bt
#0  0x00007f73b2b7b2a5 in (anonymous namespace)::ChildImpl::GetOrCreateForCurrentThread() () at /home/farre/src/gecko/work-1/ipc/glue/BackgroundImpl.cpp:1506
#1  0x00007f73b2b7b2a5 in mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread() () at /home/farre/src/gecko/work-1/ipc/glue/BackgroundImpl.cpp:721
#2  0x00007f73b485ddaf in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() (this=0x7f738eddc500) at /home/farre/src/gecko/work-1/dom/workers/RuntimeService.cpp:2676
#3  0x00007f73b2665662 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f738ede13a0, aMayWait=<optimized out>, aResult=0x7f730a2fdd77) at /home/farre/src/gecko/work-1/xpcom/threads/nsThread.cpp:1040
#4  0x00007f73b26704af in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7f73c2200020, aMayWait=true) at /home/farre/src/gecko/work-1/xpcom/threads/nsThreadUtils.cpp:517
#5  0x00007f73b2b92d2b in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (this=0x7f738eddc480, aDelegate=0x7f738edbe900)
    at /home/farre/src/gecko/work-1/ipc/glue/MessagePump.cpp:364
#6  0x00007f73b2b2e37b in MessageLoop::RunInternal() (this=<optimized out>) at /home/farre/src/gecko/work-1/ipc/chromium/src/base/message_loop.cc:326
#7  0x00007f73b2b2e37b in MessageLoop::RunHandler() (this=<optimized out>) at /home/farre/src/gecko/work-1/ipc/chromium/src/base/message_loop.cc:319
#8  0x00007f73b2b2e37b in MessageLoop::Run() (this=0x7f73b757be55) at /home/farre/src/gecko/work-1/ipc/chromium/src/base/message_loop.cc:299
#9  0x00007f73b2663cdb in nsThread::ThreadFunc(void*) (aArg=<optimized out>) at /home/farre/src/gecko/work-1/xpcom/threads/nsThread.cpp:423
#10 0x00007f73c3776df5 in _pt_root (arg=0x7f738ede14c0) at /home/farre/src/gecko/work-1/nsprpub/pr/src/pthreads/ptthread.c:201
#11 0x00007f73c324361b in start_thread (arg=0x7f730a2fe700) at pthread_create.c:465
#12 0x00007f73c247998f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

The script creates a whole heap of threads though, 500+ before it goes awry for me.

Ni to :baku and :bkelly due to the worker clues, do you have any ideas?

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

This script is exercising script startup and shutdown.  The `new Worker("x")` code will start a thread, but then the worker never starts up because "x" is not a valid script AFAICT.

Also, if you are hitting:

MOZ_CRASH("Failed to create top level actor!");

Then that is probably not a security bug.  MOZ_CRASH() is a safe crash.

My guess is this is failing because we've attempted to create so many PBackground transports simultaneously that we've run out of file descriptors.  You could add some instrumentation to that effect.

I would note, though, that comment 4 crash is very different from comment 0 crash.  I wonder if switching from rIC to setTimeout() is just changing the timing such that we don't hit comment 0 crash.

Comment 6

[Andrew Overholt [:overholt]]

Maybe Blake could take a look?

Comment 7

[Andrea Marchesini [:baku]]

For this MOZ_CRASH(), we can remove this assertion and throw an exception instead.
We already have a comment about this:

https://searchfox.org/mozilla-central/rev/c579ce13ca7864c5df9711eda730ceb00501aed3/dom/workers/RuntimeService.cpp#2649

Comment 8

[Andrea Marchesini [:baku]]

Attachment: worker_crash.patch

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8990959 [details] [diff] [review]
worker_crash.patch

Review of attachment 8990959 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/glue/BackgroundImpl.cpp
@@ -1507,5 @@
>    rv = PBackground::CreateEndpoints(content->OtherPid(),
>                                      base::GetCurrentProcId(),
>                                      &parent, &child);
>    if (NS_FAILED(rv)) {
> -    MOZ_CRASH("Failed to create top level actor!");

I feel like there was an open bug somewhere (maybe an intermittent orange?) that this change is going to fix.

Comment 10

[Andrea Marchesini [:baku]]

My patch fixed the MOZ_CRASH. I don't think it's a sec-bug. Just a confirm before landing the patch: I should not ask for a sec-approval, right?

Comment 11

[Andrea Marchesini [:baku]]

Yes a MOZ_CRASH is not a security bug. Let's land the patch.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d6c5037a84c304cff3fee01af216f4b71d757ae1
https://hg.mozilla.org/mozilla-central/rev/d6c5037a84c3