1911745 - Assertion failure: opener->mUseRemoteTabs == mUseRemoteTabs, at docshell/base/BrowsingContext.cpp:1778 with IPC fuzzing

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P2)

Description

[Christian Holler (:decoder)]

In experimental IPC fuzzing, we found the following crash on mozilla-central revision 20240804-f19c35b800f5 (ccov-fuzzing-asan-nyx-opt build):

Assertion failure: opener->mUseRemoteTabs == mUseRemoteTabs, at /docshell/base/BrowsingContext.cpp:1778
=================================================================
==1639==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7ffff7fb4983 bp 0x0000000006f2 sp 0x7fffffffb7b0 T0)
==1639==The signal is caused by a WRITE memory access.
==1639==Hint: address points to the zero page.
    #0 0x7ffff7fb4983  (ld_preload_fuzz.so+0x3983)
    #1 0x7ffff7fb677a  (ld_preload_fuzz.so+0x577a)
    #2 0x7fffea159199 in mozilla::dom::BrowsingContext::AssertCoherentLoadContext() /docshell/base/BrowsingContext.cpp:1778:7
    #3 0x7fffea1503f6 in mozilla::dom::BrowsingContext::Attach(bool, mozilla::dom::ContentParent*) /docshell/base/BrowsingContext.cpp:836:3
    #4 0x7fffea1538cc in mozilla::dom::BrowsingContext::CreateFromIPC(mozilla::dom::BrowsingContext::IPCInitializer&&, mozilla::dom::BrowsingContextGroup*, mozilla::dom::ContentParent*) /docshell/base/BrowsingContext.cpp:583:19
    #5 0x7fffe7c6f3a4 in mozilla::dom::ContentParent::RecvCreateBrowsingContext(unsigned long, mozilla::dom::BrowsingContext::IPCInitializer&&) /dom/ipc/ContentParent.cpp:6934:10
    #6 0x7fffe7f43421 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:14628:81
    #7 0x7fffde9dd623 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /ipc/glue/MessageChannel.cpp:1820:25
    [...]
    #32 0x7fffd6629e3f in __libc_start_main ??:0:0

The attached testcase can be reproduced using a special build to inject IPC messages.

Filing this s-s until it is confirmed that the assert is harmless.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Andrew McCreight [:mccr8]]

Nika, does this need to continue to be hidden? It looks like there's a bunch of these flags we just take in from the child process so I'd guess that if this was a big problem we would have done something about this already. But it does sound potentially spooky. Thanks.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

I don't think these flags are a huge deal, but it's probably better to enforce these basic coherency checks at the IPC layer than to not. I'll post a patch which just moves the checks over to IPC_FAIL if we're attaching over IPC, as it won't be too hard.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1911745 - Unify BrowsingContext flag coherency checks, r=mccr8!

Previously these checks were largely diagnostic tools for finding bugs
in other code as it evolves. This unifies the checks a bit more and
makes them stronger for BrowsingContexts created over IPC, providing a
place for more coherency checks to be added in the future.

Comment 6

[Andrew McCreight [:mccr8]]

Feel free to unhide this if you think that's appropriate. I'll mark this sec-want for now.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85ff58376b4c
Unify BrowsingContext flag coherency checks, r=mccr8

Comment 8

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/mozilla-central/rev/85ff58376b4c

Comment 9

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

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

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

For more information, please visit BugBot documentation.

Comment 10

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

The bug is marked as tracked for firefox130 (beta). However, the bug still has low severity.

:jstutte, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

I'm not sure this warrants an uplift, I think we can let it ride the trains, though it'll be low risk to uplift if we want to.

Comment 12

[Jens Stutte [:jstutte]]

I agree.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Should this be in FIXED state now?