1905256 - Assertion failure: ok (Incremental marking verification failed), at /builds/worker/checkouts/gecko/js/src/gc/Verifier.cpp:765

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P1)

Description

[Tyson Smith [:tsmith]]

Found while fuzzing m-c 20240516-f956d7e03a82 (--enable-debug --enable-fuzzing)

A reliable test case is not available. javascript.options.mem.gc_zeal.mode=11 was set.

Assertion failure: ok (Incremental marking verification failed), at /builds/worker/checkouts/gecko/js/src/gc/Verifier.cpp:765

#0 0x784d0d627c3e in js::gc::MarkingValidator::validate() /builds/worker/checkouts/gecko/js/src/gc/Verifier.cpp:765:3
#1 0x784d0d60f74c in validateIncrementalMarking /builds/worker/checkouts/gecko/js/src/gc/Verifier.cpp:781:23
#2 0x784d0d60f74c in js::gc::GCRuntime::beginSweepingSweepGroup(JS::GCContext*, js::SliceBudget&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:1572:3
#3 0x784d0d63d800 in sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2201:23
#4 0x784d0d637cce in sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2236:19
#5 0x784d0d614921 in js::gc::GCRuntime::performSweepActions(js::SliceBudget&) /builds/worker/checkouts/gecko/js/src/gc/Sweeping.cpp:2384:53
#6 0x784d0d575823 in js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, JS::GCReason, bool) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:3888:11
#7 0x784d0d578ab0 in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget const&, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:4402:3
#8 0x784d0d57a1f3 in js::gc::GCRuntime::collect(bool, js::SliceBudget const&, JS::GCReason) /builds/worker/checkouts/gecko/js/src/gc/GC.cpp:4593:9
#9 0x784d083a25ad in GarbageCollectImpl(JS::GCReason, nsJSContext::IsShrinking, js::SliceBudget const&) /builds/worker/checkouts/gecko/dom/base/nsJSEnvironment.cpp:1059:5
#10 0x784d083a2800 in nsJSContext::RunIncrementalGCSlice(JS::GCReason, nsJSContext::IsShrinking, js::SliceBudget&) /builds/worker/checkouts/gecko/dom/base/nsJSEnvironment.cpp:1096:3
#11 0x784d07f9fb13 in mozilla::CCGCScheduler::GCRunnerFiredDoGC(mozilla::TimeStamp, mozilla::GCRunnerStep const&) /builds/worker/checkouts/gecko/dom/base/CCGCScheduler.cpp:472:3
#12 0x784d07f9eec4 in mozilla::CCGCScheduler::GCRunnerFired(mozilla::TimeStamp) /builds/worker/checkouts/gecko/dom/base/CCGCScheduler.cpp:431:10
#13 0x784d05f11afe in operator() /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_function.h:687:14
#14 0x784d05f11afe in mozilla::IdleTaskRunner::Run() /builds/worker/checkouts/gecko/xpcom/threads/IdleTaskRunner.cpp:124:14
#15 0x784d05f1265e in mozilla::IdleTaskRunnerTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/IdleTaskRunner.cpp:45:15
#16 0x784d05f20ad6 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:907:26
#17 0x784d05f1f3fe in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:772:15
#18 0x784d05f1f715 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:516:36
#19 0x784d05f2e9b6 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:234:37
#20 0x784d05f2e9b6 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#21 0x784d05f42a4d in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1204:16
#22 0x784d05f49a2f in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#23 0x784d06bf32c5 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#24 0x784d06b0a681 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#25 0x784d06b0a681 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#26 0x784d0b9a7638 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#27 0x784d0ba5e638 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:469:33
#28 0x784d0ca1768b in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:712:20
#29 0x784d06bf41a6 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#30 0x784d06b0a681 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#31 0x784d06b0a681 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#32 0x784d0ca16f1b in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:647:34
#33 0x5a76387dfaaf in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#34 0x5a76387dfaaf in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:378:18
#35 0x784d1a5a5d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#36 0x784d1a5a5e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#37 0x5a76387b54e8 in _start (/home/worker/builds/m-c-20240620040816-fuzzing-debug/firefox-bin+0x584e8) (BuildId: 9a9cb147226c236d5dae11414f8873bb34cbd4f1)

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/A1FFcqTKF--mZbwZQVrbvw/index.html

Comment 2

[Jon Coppeard (:jonco)]

Since this is using a build from over a month ago (20240516) it could be a duplicate of bug 1896973 which landed 2024-06-13.

Comment 3

[Tyson Smith [:tsmith]]

That is the build the test case was originally reported with. This Pernosco session was recorded with m-c 20240620-ac120cec791e.

Comment 4

[Nicolas B. Pierron [:nbp]]

To provide more context 20240620-ac120cec791e got pushed on June 20th.
So this sounds like this might be a variation of the original issue, and not a duplicate.

Comment 5

[Daniel Veditz [:dveditz]]

Also, Tyson reports that this same failure continues to be reported by fuzztesting. It's not going away on its own.

Comment 6

[Daniel Veditz [:dveditz]]

If the assertion is true this sounds like we could collect something still alive and trigger a later UAF. Or it could be a bug in the verifier itself and not a security bug (which seems to be how bug 1896973 was judged). assuming the worst for now until someone's looked at the pernosco session to see if that's useful.

There are 421 reports of this assert currently, so if you need a different pernosco recording let Tyson know and he can get you one.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:willyelm, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Comment 8

[Jon Coppeard (:jonco)]

Investigating. This looks like an issue with incremental marking verification.

Comment 9

[Jon Coppeard (:jonco)]

The is an issue with incremental marking verification being too strict and is not security sensitive.

Comment 10

[Jon Coppeard (:jonco)]

Attachment: Bug 1905256 - Mark incremental marking verification more permissive r?sfink

This is a case where our incremental marking verification is too strict.

It's complaining that we're marking something gray when non-incrmental marking
would not have marked it at all. This happens because the prebarrier is marking
a weakmap key black and the map itself is gray, resulting in the map entry's
value being marked gray.

This case is OK because the value is actually unreachable. The opposite case
where incremental marking marks something gray that non-incremental marking
would have marked black is not OK and could lead to the cycle collector
thinking something is part of a garbage cycle when it's actually live. But that
can't happen here because the cycle collector won't be able to find the gray
value in the first place.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5afadee4e18f
Mark incremental marking verification more permissive r=sfink

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/5afadee4e18f

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jonco, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Jon Coppeard (:jonco)]

This can ride the trains.