1604836 - Crash [@ realm]

Status: VERIFIED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.zip

Testcase found while fuzzing mozilla-central rev e928d6001344. Testcase must be served via a local webserver in order to reproduce.

==1691==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000090 (pc 0x7f39cf887f7b bp 0x7f399cc5f130 sp 0x7f399cc5f130 T89)
==1691==The signal is caused by a READ memory access.
==1691==Hint: address points to the zero page.
    #0 0x7f39cf887f7a in realm /builds/worker/workspace/build/src/js/src/vm/JSContext.h:356:37
    #1 0x7f39cf887f7a in JS::CurrentGlobalOrNull(JSContext*) /builds/worker/workspace/build/src/js/src/jsapi.cpp:1158:12
    #2 0x7f39cb23fd04 in mozilla::dom::WorkerJSContext::DispatchToMicroTask(already_AddRefed<mozilla::MicroTaskRunnable>) /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:984:38
    #3 0x7f39c378b28f in mozilla::CycleCollectedJSContext::enqueuePromiseJob(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/xpcom/base/CycleCollectedJSContext.cpp:248:3
    #4 0x7f39cfd4eb24 in JSRuntime::enqueuePromiseJob(JSContext*, JS::Handle<JSFunction*>, JS::Handle<JSObject*>, JS::Handle<js::GlobalObject*>) /builds/worker/workspace/build/src/js/src/vm/Runtime.cpp:634:24
    #5 0x7f39cfae2bee in EnqueuePromiseReactionJob(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::PromiseState) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1232:25
    #6 0x7f39cfaa44a9 in operator() /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1586:12
    #7 0x7f39cfaa44a9 in ForEachReaction<(lambda at /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1585:44)> /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1558:12
    #8 0x7f39cfaa44a9 in TriggerPromiseReactions /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1585:10
    #9 0x7f39cfaa44a9 in ResolvePromise(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>, JS::PromiseState) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1279:10
    #10 0x7f39cfaddb90 in FulfillMaybeWrappedPromise(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:1313:10
    #11 0x7f39cfa95c08 in ResolvePromiseInternal(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:968:12
    #12 0x7f39cfaa3a9e in js::PromiseObject::resolve(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/js/src/builtin/Promise.cpp:5399:12
    #13 0x7f39cf8a4c1d in ResolveOrRejectPromise(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, bool) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3868:19
    #14 0x7f39cb306673 in mozilla::dom::Promise::MaybeResolve(JSContext*, JS::Handle<JS::Value>) /builds/worker/workspace/build/src/dom/promise/Promise.cpp:294:8
    #15 0x7f39cb308032 in void mozilla::dom::Promise::MaybeSomething<JS::Handle<JS::Value> const&>(JS::Handle<JS::Value> const&, void (mozilla::dom::Promise::*)(JSContext*, JS::Handle<JS::Value>)) /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/Promise.h:309:5
    #16 0x7f39cb2a4d32 in mozilla::MozPromise<bool, nsresult, true>::ThenValue<mozilla::dom::ServiceWorkerGlobalScope::SkipWaiting(mozilla::ErrorResult&)::$_1, mozilla::dom::ServiceWorkerGlobalScope::SkipWaiting(mozilla::ErrorResult&)::$_2>::DoResolveOrRejectInternal(mozilla::MozPromise<bool, nsresult, true>::ResolveOrRejectValue&) /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h
    #17 0x7f39c5fcebd1 in mozilla::MozPromise<bool, nsresult, true>::ThenValueBase::ResolveOrRejectRunnable::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h:402:21
    #18 0x7f39c3977337 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1241:14
    #19 0x7f39c397196e in NS_ProcessPendingEvents(nsIThread*, unsigned int) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:434:19
    #20 0x7f39cb2766cf in mozilla::dom::WorkerPrivate::ClearMainEventQueue(mozilla::dom::WorkerPrivate::WorkerRanOrNot) /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:3446:5
    #21 0x7f39cb23ee3d in mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run() /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:2344:21
    #22 0x7f39c3977337 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1241:14
    #23 0x7f39c3981b3c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #24 0x7f39c4ba6164 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:332:5
    #25 0x7f39c4a9e6c7 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #26 0x7f39c4a9e6c7 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #27 0x7f39c4a9e6c7 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #28 0x7f39c397045a in nsThread::ThreadFunc(void*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:459:11
    #29 0x7f39e731625e in _pt_root /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:201:5
    #30 0x7f39e6f5e6da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)
    #31 0x7f39e5f3c88e in clone /build/glibc-OTsEL5/glibc-2.27/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:95

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/workspace/build/src/js/src/vm/JSContext.h:356:37 in realm
Thread T89 (DOM Worker) created by T0 here:
    #0 0x56411b9b645a in pthread_create /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:209:3
    #1 0x7f39e7308b75 in _PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:458:14
    #2 0x7f39e72f68de in PR_CreateThread /builds/worker/workspace/build/src/nsprpub/pr/src/pthreads/ptthread.c:533:12
    #3 0x7f39c3972ec7 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:675:8
    #4 0x7f39cb298edb in mozilla::dom::WorkerThread::Create(mozilla::dom::WorkerThreadFriendKey const&) /builds/worker/workspace/build/src/dom/workers/WorkerThread.cpp:92:7
    #5 0x7f39cb218aa6 in mozilla::dom::workerinternals::RuntimeService::ScheduleWorker(mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1428:14
    #6 0x7f39cb2174df in mozilla::dom::workerinternals::RuntimeService::RegisterWorker(mozilla::dom::WorkerPrivate*) /builds/worker/workspace/build/src/dom/workers/RuntimeService.cpp:1293:19
    #7 0x7f39cb26c7ae in mozilla::dom::WorkerPrivate::Constructor(JSContext*, nsTSubstring<char16_t> const&, bool, mozilla::dom::WorkerType, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, mozilla::dom::WorkerLoadInfo*, mozilla::ErrorResult&, nsTString<char16_t>) /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp:2369:24
    #8 0x7f39cb2a83ad in mozilla::dom::RemoteWorkerChild::ExecWorkerOnMainThread(mozilla::dom::RemoteWorkerData&&) /builds/worker/workspace/build/src/dom/workers/remoteworkers/RemoteWorkerChild.cpp:438:41
    #9 0x7f39cb2c07c9 in operator() /builds/worker/workspace/build/src/dom/workers/remoteworkers/RemoteWorkerChild.cpp:314:29
    #10 0x7f39cb2c07c9 in mozilla::detail::RunnableFunction<mozilla::dom::RemoteWorkerChild::ExecWorker(mozilla::dom::RemoteWorkerData const&)::$_2>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:564:5
    #11 0x7f39c3977337 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1241:14
    #12 0x7f39c3981b3c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
    #13 0x7f39c4ba43df in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:87:21
    #14 0x7f39c4a9e6c7 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
    #15 0x7f39c4a9e6c7 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308:3
    #16 0x7f39c4a9e6c7 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290:3
    #17 0x7f39cb922638 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
    #18 0x7f39cf23293f in nsAppStartup::Run() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:272:30
    #19 0x7f39cf444e80 in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4600:22
    #20 0x7f39cf446db2 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4737:8
    #21 0x7f39cf4482f3 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4818:21
    #22 0x56411b9fe79f in do_main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:217:22
    #23 0x56411b9fe79f in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:339:16
    #24 0x7f39e5e3cb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310

==1691==ABORTING

Comment 1

[Andrew McCreight [:mccr8]]

I just looked at stack a little, but I think the JSContext being passed to JS::CurrentGlobalOrNull is null.

Comment 2

[Jens Stutte [:jstutte]]

It is not null but close to null, as there is an assertion right before on cx it cannot be null (assuming, that DEBUG was enabled here), but the access to the member variable realm_ of the base class RootingContext points to 0x000000000090 which is very close to null. So the interesting part is probably: where does this broken cx come from?

    JSContext* cx = GetCurrentWorkerThreadJSContext();
    NS_ASSERTION(cx, "This should never be null!");

    JS::Rooted<JSObject*> global(cx, JS::CurrentGlobalOrNull(cx));
    NS_ASSERTION(global, "This should never be null!");

see 1

It comes from GetCurrentWorkerThreadJSContext() and there I see:

  // Touched on multiple threads, protected with mMutex.
  JSContext* mJSContext;

which is not very encouraging for further code reading...

:jkratzer, may I ask for a pernosco trace here if this is reproducable? Thanks!

Comment 3

[Jens Stutte [:jstutte]]

It's code common to Workers and Service Workers but it probably points to WorkersPrivate.

Comment 4

[Jason Kratzer [:jkratzer]]

:jstutte, I've uploaded the following pernosco trace of this issue:
https://pernos.co/debug/rpwS49fEiUn0kFclgBmJvw/index.html

Comment 5

[Daniel Veditz [:dveditz]]

Probably a sec-low bug, but don't know how we'd get a null cx so slightly scary and we should understand that.

Comment 6

[Perry Jiang [:perry] [no longer employee, use ni?]]

1. Main thread: RemoteWorkerChild::CloseWorkerOnMainThread calls WorkerPrivate::Cancel (worker1.js)

   • A NotifyRunnable is dispatch to the WorkerPrivate to change its status from Running to Canceling

2. Worker thread: NotifyRunnable::WorkerRun calls WorkerPrivate::NotifyInternal with aStatus=Canceling

3. Worker thread: WorkerPrivate::DoRunLoop sees its status is Canceling and calls WorkerPrivate::NotifyInternal with aStatus=Killing

   • Within the same WorkerPrivate::DoRunLoop call, WorkerPrivate::mJSContext is assigned to nullptr

4. Worker thread: ServiceWorkerGlobalScope::SkipWaiting promise settles

   • Promise logic attempts to dispatch the promise's callback to the worker's microtask queue, which uses the null WorkerPrivate::mJSContext.

I think the ServiceWorkerGlobalScope::SkipWaiting needs a DOMMozPromiseRequestHolder so the dom::Promise won't try to dispatch the microtasks to a dead worker.

Comment 7

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1604836 - ServiceWorkerGlobalScope.skipWaiting should use DOMMozPromiseRequestHolder r?asuth

Comment 8

[Perry Jiang [:perry] [no longer employee, use ni?]]

Landing without sec-approval because this is sec-low and only possible to experience in Nightly.

Comment 9

[Jens Stutte [:jstutte]]

Thanks :jkratzer and :perry for the very fast help here! And I just learned, that these ASAN builds have NS_ASSERTION disabled and cx was nullptr as :mccr8 suspected.

Comment 10

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/mozilla-central/rev/471f2a4e57f6f421760ddc2a34ed9dd9be632651

Comment 11

[Daniel Bodea [:danibodea]]

I have managed to reproduce this issue in Nightly fuzzing asan opt build v72.0a1 from 2019-11-09 on Windows 10.
I have also reproduced this issue in Nightly fuzzing asan opt build v72.0a1 from 2019-12-19 on Ubuntu 18.
I have managed to verify the fix in Nightly fuzzing asan opt v73.0a1 from 2019-12-31 on both Windows 10 and Ubuntu 18.
The crash would not occur in the fixed version no matter how much I waited and saw the page refreshing.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Doesn't sound like we need to worry about backports here since this only affects Nightly in practice. That said, should we try to land a test for this?

Comment 13

[Perry Jiang [:perry] [no longer employee, use ni?]]

I don't believe it's possible to have a deterministic test for this (the fuzzing testcase would continually refresh the page until the crash occurred). Having a deterministic test would require terminating the ServiceWorker while a skipWaiting() promise is pending, and there doesn't seem to be a way/API to consistently do this.