Closed Bug 1835390 Opened 2 years ago Closed 2 years ago

crash near null in [@ mozilla::dom::FetchStreamReader::StartConsuming]

Categories

(Core :: DOM: Streams, defect)

Product:
Core
Component:
DOM: Streams
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
115 Branch
Tracking Flags:
Tracking Status
firefox-esr102 --- unaffected
firefox113 --- unaffected
firefox114 --- unaffected
firefox115 --- fixed

People

(Reporter: tsmith, Assigned: saschanaz)

References

(Blocks 1 open bug, Regression)

Details

(4 keywords, Whiteboard: [bugmon:bisected,confirmed], [wptsync upstream])

Attachments

(3 files)

testcase.html
338 bytes, text/html
Details
Bug 1835390 - Only close the input in DOM_WINDOW_DESTROYED_TOPIC observer in BodyStream r=smaug
48 bytes, text/x-phabricator-request
Details | Review
testcase2.html
266 bytes, text/html
Details
Attached file testcase.html —

Found while fuzzing m-c 20230516-6854d5a61f68 (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html --repeat 50

Note: This test case is not very reliable.

==13751==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7f9cb81d4489 bp 0x7fff1141aab0 sp 0x7fff1141aa60 T0)
==13751==The signal is caused by a READ memory access.
==13751==Hint: address points to the zero page.
    #0 0x7f9cb81d4489 in operator! /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:350:36
    #1 0x7f9cb81d4489 in mozilla::dom::FetchStreamReader::StartConsuming(JSContext*, mozilla::dom::ReadableStream*, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/fetch/FetchStreamReader.cpp:177:3
    #2 0x7f9cb81d7539 in mozilla::dom::FetchBody<mozilla::dom::Response>::ConsumeBody(JSContext*, mozilla::dom::BodyConsumer::ConsumeType, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/fetch/Fetch.cpp:1323:3
    #3 0x7f9cb5b3091b in Json /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Fetch.h:154:12
    #4 0x7f9cb5b3091b in json /builds/worker/workspace/obj-build/dom/bindings/ResponseBinding.cpp:1886:60
    #5 0x7f9cb5b3091b in mozilla::dom::Response_Binding::json_promiseWrapper(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/ResponseBinding.cpp:1902:13
    #6 0x7f9cb72f6c1d in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3335:13
    #7 0x7f9cc2002b63 in CallJSNative /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:486:13
    #8 0x7f9cc2002b63 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:580:12
    #9 0x7f9cc2027d06 in InternalCall /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:647:10
    #10 0x7f9cc2027d06 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:652:10
    #11 0x7f9cc2027d06 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3395:16
    #12 0x7f9cc20018b5 in MaybeEnterInterpreterTrampoline /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:400:10
    #13 0x7f9cc20018b5 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:458:13
    #14 0x7f9cc2002d1c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:612:13
    #15 0x7f9cc2004c96 in InternalCall /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:647:10
    #16 0x7f9cc2004c96 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:679:8
    #17 0x7f9cc217183b in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CallAndConstruct.cpp:117:10
    #18 0x7f9cb6c9ad3f in mozilla::dom::EventListener::HandleEvent(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventListenerBinding.cpp:62:8
    #19 0x7f9cb8111946 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventListenerBinding.h:65:12
    #20 0x7f9cb811121c in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1235:43
    #21 0x7f9cb8112daf in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1427:21
    #22 0x7f9cb80fac84 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:342:17
    #23 0x7f9cb80f8a93 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:545:16
    #24 0x7f9cb80fee15 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:1133:11
    #25 0x7f9cbc72e7f6 in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/checkouts/gecko/layout/base/nsDocumentViewer.cpp:1082:7
    #26 0x7f9cc0956608 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:6393:20
    #27 0x7f9cc0955170 in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:5786:7
    #28 0x7f9cc0957ed6 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp
    #29 0x7f9cb2ab2473 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:1380:3
    #30 0x7f9cb2ab0c4d in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:978:14
    #31 0x7f9cb2aac118 in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:797:9
    #32 0x7f9cb2aaf26a in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:680:5
    #33 0x7f9cc09abc8a in nsDocShell::OnStopRequest(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:13848:23
    #34 0x7f9cb0c540b3 in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:631:22
    #35 0x7f9cb0c575d4 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:535:10
    #36 0x7f9cb45ca53e in DoUnblockOnload /builds/worker/checkouts/gecko/dom/base/Document.cpp:11647:18
    #37 0x7f9cb45ca53e in mozilla::dom::Document::UnblockOnload(bool) /builds/worker/checkouts/gecko/dom/base/Document.cpp:11585:9
    #38 0x7f9cb4602d0f in mozilla::dom::Document::DispatchContentLoadedEvents() /builds/worker/checkouts/gecko/dom/base/Document.cpp:8118:3
    #39 0x7f9cb4731aeb in operator()<> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1164:18
    #40 0x7f9cb4731aeb in __invoke_impl<void, (lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/7/../../../../include/c++/7/bits/invoke.h:60:14
    #41 0x7f9cb4731aeb in __invoke<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/7/../../../../include/c++/7/bits/invoke.h:95:14
    #42 0x7f9cb4731aeb in __apply_impl<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/7/../../../../include/c++/7/tuple:1662:14
    #43 0x7f9cb4731aeb in apply<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1163:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/7/../../../../include/c++/7/tuple:1671:14
    #44 0x7f9cb4731aeb in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1162:12
    #45 0x7f9cb4731aeb in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1213:13
    #46 0x7f9cb07fc630 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/checkouts/gecko/xpcom/threads/SchedulerGroup.cpp:114:20
    #47 0x7f9cb081727a in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:555:16
    #48 0x7f9cb0807fea in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:879:26
    #49 0x7f9cb0804ee7 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:702:15
    #50 0x7f9cb08057cf in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:491:36
    #51 0x7f9cb081c961 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:218:37
    #52 0x7f9cb081c961 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
    #53 0x7f9cb0848757 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1240:16
    #54 0x7f9cb0856094 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:479:10
    #55 0x7f9cb2478c2e in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
    #56 0x7f9cb22a232a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:368:10
    #57 0x7f9cb22a232a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:361:3
    #58 0x7f9cb22a232a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:343:3
    #59 0x7f9cbbc650d9 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
    #60 0x7f9cc1ba7b6e in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:724:20
    #61 0x7f9cb22a232a in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:368:10
    #62 0x7f9cb22a232a in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:361:3
    #63 0x7f9cb22a232a in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:343:3
    #64 0x7f9cc1ba71fe in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:659:34
    #65 0x55f19e82b77e in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
    #66 0x55f19e82b77e in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
    #67 0x7f9cd7429d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #68 0x7f9cd7429e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #69 0x55f19e754d98 in _start (/home/user/workspace/browsers/m-c-20230526162417-fuzzing-asan-opt/firefox+0x107d98) (BuildId: f801d2f27b77fc961ad9447bbd4dcdf450c61d2b)
Flags: in-testsuite?

A Pernosco session is available here: https://pernos.co/debug/73oe9aecBYlr1LtO8lt2Dg/index.html

Keywords: pernosco

Verified bug as reproducible on mozilla-central 20230526215433-fc6056442a0f.
The bug appears to have been introduced in the following build range:

Start: a1c3dcc09af599d18e7f8b278d565686f7d486d1 (20230515221112)
End: 6854d5a61f68124288044381b0a94207c541e80a (20230516011519)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a1c3dcc09af599d18e7f8b278d565686f7d486d1&tochange=6854d5a61f68124288044381b0a94207c541e80a

Keywords: regression
Whiteboard: [bugmon:bisected,confirmed]

:saschanaz, looking at the Pushlog in Comment 2, could this be caused by Bug 1832326?

Flags: needinfo?(krosylight)

The issue is that DOM_WINDOW_DESTROYED_TOPIC is advertised when the JavaScript is still running, so it runs on a partially unlinked object (via ::Observe instead of cycle collection). That observer is being removed in bug 1811882.

Flags: needinfo?(krosylight)
Regressed by: 1832326
See Also: → 1811882
Depends on: 1835868

Set release status flags based on info from the regressing bug 1832326

status-firefox113: --- → unaffected
status-firefox114: --- → unaffected
status-firefox-esr102: --- → unaffected

(In reply to Kagami [:saschanaz] from comment #4)

The issue is that DOM_WINDOW_DESTROYED_TOPIC is advertised when the JavaScript is still running, so it runs on a partially unlinked object (via ::Observe instead of cycle collection). That observer is being removed in bug 1811882.

Thanks for the information.
What's the severity on this? It was caught by fuzzing but is it a likely problem to impact users?
Not sure about the timing of when you plan on landing Bug 1811882. Wondering about the impact for Fx115 if Bug 1811882 lands after Fx115 goes to beta.

(115 soft code freeze starts on 2023-06-01 and goes to beta on 2023-06-05)

Flags: needinfo?(krosylight)

I'd say S4 since synchronous XHR should be rare enough, especially when the page uses Fetch. But I'm not the triage owner, so I'm not setting S4 myself.

Flags: needinfo?(krosylight)

( Sync XHR is not very rare, unfortunately. )

How many websites mix sync XHR and Fetch?

I don't know why that would be particularly rare. Sites use many script libraries. Ads may for example use some other scripts than the main page.

And not just mixing it, this should be problematic when a page sequentially:

  1. Do a fetch call and access response.body (but not really do actual read, as doing so will block step 4)
  2. Trigger navigation
  3. Immediately do sync XHR so that the next JS call would be done after destroying window by the navigation
  4. And then immediately read the body via Response methods, not directly via .body.

(So I believe it should only be problematic when a single script do Fetch+XHR+navigation at once)

Since this is regressed by bug 1832326, I'd like to change the component to DOM:Streams.

Component: DOM: Networking → DOM: Streams

It was seemingly to cut a cycle between JS and DOM, but now no leak is observed even without it.

Attached file testcase2.html —

Oh btw, this doesn't need XHR as noted in bug 1835868.

Attachment #9337032 - Attachment mime type: text/plain → text/html
Attachment #9336989 - Attachment description: WIP: Bug 1835390 - Remove DOM_WINDOW_DESTROYED_TOPIC observation in BodyStream → WIP: Bug 1835390 - Check state before trying to consume the fetch body
Attachment #9336989 - Attachment description: WIP: Bug 1835390 - Check state before trying to consume the fetch body → WIP: Bug 1835390 - Only close the input in DOM_WINDOW_DESTROYED_TOPIC observer in BodyStream
Assignee: nobody → krosylight
Attachment #9336989 - Attachment description: WIP: Bug 1835390 - Only close the input in DOM_WINDOW_DESTROYED_TOPIC observer in BodyStream → Bug 1835390 - Only close the input in DOM_WINDOW_DESTROYED_TOPIC observer in BodyStream r=smaug
Status: NEW → ASSIGNED
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eda54a77b898 Only close the input in DOM_WINDOW_DESTROYED_TOPIC observer in BodyStream r=smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/40365 for changes under testing/web-platform/tests
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed], [wptsync upstream]

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

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox115: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch
Upstream PR merged by moz-wptsync-bot

Unable to reproduce bug 1835390 using build mozilla-central 20230516042430-6854d5a61f68. Without a baseline, bugmon is unable to analyze this bug.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Keywords: bugmon

Bugmon is failing to reproduce in many other bugs too 🤔

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: