1709236 - crash near null in [@ Top]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

First found while fuzzing m-c 20210320-f56d2bf535d6 (--enable-debug --enable-fuzzing)

==12549==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000348 (pc 0x7f5f91a10920 bp 0x7ffd20d95690 sp 0x7ffd20d95670 T12549)
==12549==The signal is caused by a READ memory access.
==12549==Hint: address points to the zero page.
    #0 0x7f5f91a10920 in operator bool /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:310:45
    #1 0x7f5f91a10920 in Top src/docshell/base/BrowsingContext.cpp:185:10
    #2 0x7f5f91a10920 in mozilla::dom::BrowsingContext::CrossOriginIsolated() src/docshell/base/BrowsingContext.cpp:1326:10
    #3 0x7f5f8dc2e9bc in mozilla::dom::Document::CrossOriginIsolated() const src/dom/base/Document.cpp:7186:16
    #4 0x7f5f8dc4c191 in mozilla::dom::Document::AssertDocGroupMatchesKey() const src/dom/base/Document.cpp:4047:26
    #5 0x7f5f91a200e4 in GetDocGroup /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Document.h:3669:5
    #6 0x7f5f91a200e4 in mozilla::dom::BrowsingContextGroup::RemoveDocument(nsTSubstring<char> const&, mozilla::dom::Document*) src/docshell/base/BrowsingContextGroup.cpp:416:42
    #7 0x7f5f8dc41fb8 in mozilla::dom::Document::cycleCollection::Unlink(void*) src/dom/base/Document.cpp:2528:48
    #8 0x7f5f8c01485e in nsCycleCollector::CollectWhite() src/xpcom/base/nsCycleCollector.cpp:3081:26
    #9 0x7f5f8c015cfb in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) src/xpcom/base/nsCycleCollector.cpp:3435:26
    #10 0x7f5f8c018409 in nsCycleCollector_collectSlice(js::SliceBudget&, bool) src/xpcom/base/nsCycleCollector.cpp:3923:21
    #11 0x7f5f8ddf0faa in nsJSContext::RunCycleCollectorSlice(mozilla::TimeStamp) src/dom/base/nsJSEnvironment.cpp
    #12 0x7f5f8ddf26fe in CCRunnerFired(mozilla::TimeStamp) src/dom/base/nsJSEnvironment.cpp:1648:9
    #13 0x7f5f8c0d0afb in operator() /builds/worker/fetches/clang/bin/../lib/gcc/x86_64-unknown-linux-gnu/7.4.0/../../../../include/c++/7.4.0/bits/std_function.h:706:14
    #14 0x7f5f8c0d0afb in mozilla::IdleTaskRunner::Run() src/xpcom/threads/IdleTaskRunner.cpp:109:14
    #15 0x7f5f8c0d1441 in mozilla::TimedOut(nsITimer*, void*) src/xpcom/threads/IdleTaskRunner.cpp:127:11
    #16 0x7f5f8c116d3e in nsTimerImpl::Fire(int) src/xpcom/threads/nsTimerImpl.cpp:562:7
    #17 0x7f5f8c0ee4cd in nsTimerEvent::Run() src/xpcom/threads/TimerThread.cpp:252:11
    #18 0x7f5f8c1087ce in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:482:16
    #19 0x7f5f8c0e6119 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:766:26
    #20 0x7f5f8c0e5074 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:621:15
    #21 0x7f5f8c0e5203 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:405:36
    #22 0x7f5f8c10bef9 in operator() src/xpcom/threads/TaskController.cpp:141:37
    #23 0x7f5f8c10bef9 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_1>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:534:5
    #24 0x7f5f8c0f7e9f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1159:16
    #25 0x7f5f8c0feb5a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:548:10
    #26 0x7f5f8c9de7c4 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:107:5
    #27 0x7f5f8c948867 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:335:10
    #28 0x7f5f8c948782 in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
    #29 0x7f5f8c948782 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
    #30 0x7f5f906c9dd8 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #31 0x7f5f91fa1df3 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:911:20
    #32 0x7f5f8c9df70a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
    #33 0x7f5f8c948867 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:335:10
    #34 0x7f5f8c948782 in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
    #35 0x7f5f8c948782 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
    #36 0x7f5f91fa1a0e in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:743:34
    #37 0x55cdf259bb36 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #38 0x55cdf259bb36 in main src/browser/app/nsBrowserApp.cpp:313:18
    #39 0x7f5fa111d0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #40 0x55cdf257893c in _start (/home/worker/builds/m-c-20210501093251-fuzzing-debug/firefox-bin+0x1593c)

Comment 1

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Unable to reproduce bug using the following builds:

mozilla-central 20210504033521-17594d43a3dc
mozilla-central 20210320085643-f56d2bf535d6
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 2

[Kris Maglione [:kmag]]

It looks like the problem is that when called during unlink, we wind up here when we have a parent WindowContext but that WindowContext's BrowsingContext field has been unlinked: https://searchfox.org/mozilla-central/rev/aec7c53cdbbff65305d41c9d805a70efc0e902ed/docshell/base/BrowsingContext.cpp#185-186

We could try to handle that case in Top() and return null, but that puts us in a slightly dodgy position, and will actually probably not fix the root problem, since we're in AssertDocGroupMatchesKey(), which won't do the right thing if we can't get the correct top BC.

I think we actually just want to not call AssertDocGroupMatchesKey() when removing the document from the DocGroup during unlink.

Andrew, any chance you have time to look at this? You were nominated during triage.

Comment 3

[BugBot [:suhaib / :marco]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 4

[Andrew McCreight [:mccr8]]

This is debug-only.

Comment 5

[Andrew McCreight [:mccr8]]

I couldn't reproduce the crash, but Kris's explanation was clear.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1709236 - Avoid calling Document::GetDocGroup() during unlinking.

Running AssertDocGroupMatchesKey() on an unlinked document can cause
a crash in debug builds, so pass in the group directly, because all
of the callers are Documents so they already have it.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf7da30f6bba
Avoid calling Document::GetDocGroup() during unlinking. r=farre

Comment 8

[Iulian Moraru]

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

Comment 9

[BugBot [:suhaib / :marco]]

:mccr8, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 10

[Andrew McCreight [:mccr8]]

Bug in the bot.