1656925 - Hit MOZ_CRASH(DocumentChannel::SetLoadFlags: Don't set flags after creation) at /builds/worker/checkouts/gecko/netwerk/ipc/DocumentChannel.cpp:311

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 84b257d07031.

==10612==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7f7ce1aca695 bp 0x7ffef1a817f0 sp 0x7ffef1a81720 T0)
==10612==The signal is caused by a WRITE memory access.
==10612==Hint: address points to the zero page.
    #0 0x7f7ce1aca694 in mozilla::net::DocumentChannel::SetLoadFlags(unsigned int) /builds/worker/checkouts/gecko/netwerk/ipc/DocumentChannel.cpp:311:5
    #1 0x7f7ce0d653d9 in mozilla::net::nsLoadGroup::MergeLoadFlags(nsIRequest*, unsigned int&) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:939:20
    #2 0x7f7ce0d64665 in mozilla::net::nsLoadGroup::AddRequest(nsIRequest*, nsISupports*) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:449:10
    #3 0x7f7ce1acd4d8 in mozilla::net::DocumentChannelChild::AsyncOpen(nsIStreamListener*) /builds/worker/checkouts/gecko/netwerk/ipc/DocumentChannelChild.cpp:64:17
    #4 0x7f7ce34297b0 in nsURILoader::OpenURI(nsIChannel*, unsigned int, nsIInterfaceRequestor*) /builds/worker/checkouts/gecko/uriloader/base/nsURILoader.cpp:696:17
    #5 0x7f7cec261e6f in nsDocShell::OpenInitializedChannel(nsIChannel*, nsIURILoader*, unsigned int) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:10017:20
    #6 0x7f7cec25a00c in nsDocShell::DoURILoad(nsDocShellLoadState*, nsIRequest**) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:9862:10
    #7 0x7f7cec1c101b in nsDocShell::InternalLoad(nsDocShellLoadState*) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:9033:8
    #8 0x7f7cec26f8a0 in nsDocShell::OnLinkClickSync(nsIContent*, nsDocShellLoadState*, bool, nsIPrincipal*) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:12182:17
    #9 0x7f7cec27bc45 in OnLinkClickEvent::Run() /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:11932:17
    #10 0x7f7ce0a6294d in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/checkouts/gecko/xpcom/threads/SchedulerGroup.cpp:146:20
    #11 0x7f7ce0a6d0b9 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:242:16
    #12 0x7f7ce0a695a5 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:512:26
    #13 0x7f7ce0a67462 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:371:15
    #14 0x7f7ce0a6789f in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:168:36
    #15 0x7f7ce0a78ec1 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:83:37
    #16 0x7f7ce0a78ec1 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
    #17 0x7f7ce0a9df7c in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1234:14
    #18 0x7f7ce0aa8e6c in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:513:10
    #19 0x7f7ce1e6963f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:87:21
    #20 0x7f7ce1d4a0e7 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #21 0x7f7ce1d4a0e7 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #22 0x7f7ce1d4a0e7 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #23 0x7f7ce90a7c48 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:137:27
    #24 0x7f7cecc91516 in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:913:20
    #25 0x7f7ce1d4a0e7 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:334:10
    #26 0x7f7ce1d4a0e7 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:327:3
    #27 0x7f7ce1d4a0e7 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:309:3
    #28 0x7f7cecc90aff in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:744:34
    #29 0x556a2952f8b3 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #30 0x556a2952f8b3 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:303:18
    #31 0x7f7d04b80b96 in __libc_start_main /build/glibc-2ORdQG/glibc-2.27/csu/../csu/libc-start.c:310

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /builds/worker/checkouts/gecko/netwerk/ipc/DocumentChannel.cpp:311:5 in mozilla::net::DocumentChannel::SetLoadFlags(unsigned int)

Comment 1

[Honza Bambas (:mayhemer)]

Matt, can you look at this please?

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200804091327-7cb90fa4f485.
Failed to bisect testcase (Start build crashes!):
> Start: e8b7c48d4e7ed1b63aeedff379b51e566ea499d9 (20191107015224)
> End: f6a3b097f8af440a142b02450a2cdaa978fdde9f (20200803033015)
> BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=False, coverage=False, valgrind=False)

Comment 3

[u480271]

Attachment: Bug 1656925 - DocumentChannel: Don't add download loads to load group.

Starting a load from a link click, to a link with the 'download' attribute
doesn't cancel any existing loads (since it's known to be just a download, not a
navigation, so doesn't need to replace the existing one). But then we have two
DocumentChannels alive at the same time, on the same loadgroup, the loadgroup
copies flags from the old one to the download one, since they're different, and
triggers the assert about setting LoadFlags.

The download one shouldn't really be added to the loadgroup; the progress of the
download should NOT block the 'load' event of the existing navigation.

Comment 5

[Julien Cristau [:jcristau] (back April 22)]

Dan is this ready to land?

Comment 6

[u480271]

I was waiting to see if :smaug had any objections.

Comment 7

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

(looks like this was just added to my review queue)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cfbb9b48b706
DocumentChannel: Don't add download loads to load group. r=mattwoodrow

Comment 9

[Natalia Csoregi [:nataliaCs]]

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

Comment 10

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20200811091731-3fa98e681edd.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Please request beta uplift when you get a chance.

Comment 12

[u480271]

Comment on attachment 9168063 [details]
Bug 1656925 - DocumentChannel: Don't add download loads to load group.

Beta/Release Uplift Approval Request

• User impact if declined: Tab crash from diagnostic assert in a page containing an <object> tag and a download link.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change avoid triggering a diagnostic assert found by fuzzing and reported crashes. The crash has been added as a crashtest to check for regression.
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9168063 [details]
Bug 1656925 - DocumentChannel: Don't add download loads to load group.

approved for 80.0b8

Comment 14

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/81974442aa71

Comment 15

[[:philipp]]

the volume of this crash on beta seems to have reduced since the patch landed, but it's not totally gone. should we open a new bug for the remaining issue or reopen the one here?

Comment 16

[u480271]

We can relax the hard assertion which would make the crash go away. :nika is currently working on moving object/embed that are in fact documents into the parent. I would like her input on how useful the diagnostic assert is.