1928724 - crash at null [@ nsFloatManager::PushState]

Status: VERIFIED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing 20241031-2cc133b3c099 (--enable-address-sanitizer --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch -a --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

==133151==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7450cff1d38a bp 0x7ffcc72d6610 sp 0x7ffcc72d6610 T0)
==133151==The signal is caused by a READ memory access.
==133151==Hint: address points to the zero page.
    #0 0x7450cff1d38a in nsFloatManager::PushState(nsFloatManager::SavedState*) /builds/worker/checkouts/gecko/layout/generic/nsFloatManager.cpp:410:23
    #1 0x7450cfe60eff in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp:1560:33
    #2 0x7450cfc10e05 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:9986:11
    #3 0x7450cfc52237 in mozilla::PresShell::ProcessReflowCommands(bool) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:10156:22
    #4 0x7450cfc2322b in DoFlushLayout /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:10203:10
    #5 0x7450cfc2322b in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4426:9
    #6 0x7450c8d6bce7 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1456:5
    #7 0x7450c8d6bce7 in mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/dom/base/Document.cpp:11284:16
    #8 0x7450c8dd58b8 in FlushPendingNotifications /builds/worker/checkouts/gecko/dom/base/Document.cpp:11216:3
    #9 0x7450c8dd58b8 in nsIContent::GetPrimaryFrame(mozilla::FlushType) /builds/worker/checkouts/gecko/dom/base/Element.cpp:273:10
    #10 0x7450c8dda715 in mozilla::dom::Element::GetScrollContainerFrame(nsIFrame**, mozilla::FlushType) /builds/worker/checkouts/gecko/dom/base/Element.cpp:670:21
    #11 0x7450c8ddc153 in mozilla::dom::Element::ScrollBy(mozilla::dom::ScrollToOptions const&) /builds/worker/checkouts/gecko/dom/base/Element.cpp:868:30
    #12 0x7450ca7b9b84 in mozilla::dom::Element_Binding::scrollBy(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/./ElementBinding.cpp:4717:28
    #13 0x7450cab96ec4 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3290:13
    #14 0x7450d15f4234 in CallJSNative /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:528:13
    #15 0x7450d15f4234 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:624:12
    #16 0x7450d16110a7 in InternalCall /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:691:10
    #17 0x7450d16110a7 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:696:10
    #18 0x7450d16110a7 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3325:16
    #19 0x7450d15f312f in MaybeEnterInterpreterTrampoline /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:433:10
    #20 0x7450d15f312f in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:498:13
    #21 0x7450d15f43aa in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:656:13
    #22 0x7450d15f616c in InternalCall /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:691:10
    #23 0x7450d15f616c 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:723:8
    #24 0x7450d175de3a 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:119:10
    #25 0x7450ca6df061 in mozilla::dom::EventHandlerNonNull::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./EventHandlerBinding.cpp:65:37
    #26 0x7450cba74836 in void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget>>(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventHandlerBinding.h:82:12
    #27 0x7450cba73054 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) /builds/worker/checkouts/gecko/dom/events/JSEventHandler.cpp:200:12
    #28 0x7450cba28b34 in mozilla::EventListenerManager::HandleEventSingleListener(mozilla::EventListenerManager::Listener*, nsAtom*, mozilla::WidgetEvent*, mozilla::dom::Event*, mozilla::dom::EventTarget*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1346:22
    #29 0x7450cba2adf4 in mozilla::EventListenerManager::HandleEventWithListenerArray(mozilla::EventListenerManager::ListenerArray*, nsAtom*, mozilla::EventMessage, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1663:12
    #30 0x7450cba29c73 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /builds/worker/checkouts/gecko/dom/events/EventListenerManager.cpp:1560:35
    #31 0x7450cba123b2 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:365:17
    #32 0x7450cba0fbf1 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:606:16
    #33 0x7450cba1657a in mozilla::EventDispatcher::Dispatch(mozilla::dom::EventTarget*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/checkouts/gecko/dom/events/EventDispatcher.cpp:1221:11
    #34 0x7450cfcfcb1c in nsDocumentViewer::LoadComplete(nsresult) /builds/worker/checkouts/gecko/layout/base/nsDocumentViewer.cpp:1032:7
    #35 0x7450d05dda3a in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:6210:13
    #36 0x7450d05dc9ad in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:5604:7
    #37 0x7450d05dee56 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp
    #38 0x7450c6a65459 in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:1355:3
    #39 0x7450c6a64255 in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:961:14
    #40 0x7450c6a60ccf in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:783:9
    #41 0x7450c6a6353e in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/uriloader/base/nsDocLoader.cpp:666:5
    #42 0x7450d06262e4 in nsDocShell::OnStopRequest(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:13747:23
    #43 0x7450c52c5a83 in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:642:22
    #44 0x7450c52c8033 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /builds/worker/checkouts/gecko/netwerk/base/nsLoadGroup.cpp:536:10
    #45 0x7450c8d1938f in DoUnblockOnload /builds/worker/checkouts/gecko/dom/base/Document.cpp:12074:18
    #46 0x7450c8d1938f in mozilla::dom::Document::UnblockOnload(bool) /builds/worker/checkouts/gecko/dom/base/Document.cpp:12012:9
    #47 0x7450c8d4a2fb in mozilla::dom::Document::DispatchContentLoadedEvents() /builds/worker/checkouts/gecko/dom/base/Document.cpp:8436:3
    #48 0x7450c8e625ab in operator()<> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1085:18
    #49 0x7450c8e625ab in __invoke_impl<void, (lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1084:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:60:14
    #50 0x7450c8e625ab in __invoke<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1084:9)> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/invoke.h:95:14
    #51 0x7450c8e625ab in __apply_impl<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1084:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1678:14
    #52 0x7450c8e625ab in apply<(lambda at /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1084:9), std::tuple<> &> /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:1687:14
    #53 0x7450c8e625ab in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1083:12
    #54 0x7450c8e625ab 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:1134:13
    #55 0x7450c4f7e8ea in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:618:16
    #56 0x7450c4f6ab9e in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:945:26
    #57 0x7450c4f683b8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:768:15
    #58 0x7450c4f689d6 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:554:36
    #59 0x7450c4f85bc1 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:268:37
    #60 0x7450c4f85bc1 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
    #61 0x7450c4fa60df in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1155:16
    #62 0x7450c4fb0e38 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
    #63 0x7450c65726ee in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
    #64 0x7450c64583b4 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:369:10
    #65 0x7450c64583b4 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
    #66 0x7450c64583b4 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
    #67 0x7450cf4d9e99 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
    #68 0x7450cf67a46a in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:469:33
    #69 0x7450d1318fbd in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:651:20
    #70 0x7450c64583b4 in RunInternal /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:369:10
    #71 0x7450c64583b4 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
    #72 0x7450c64583b4 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
    #73 0x7450d131746c in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:586:34
    #74 0x629271d418f9 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:397:22

Comment 1

[Mayank Bansal]

Got a crash from the testcase: https://crash-stats.mozilla.org/report/index/17837ea5-9ceb-4faa-a73c-e0a410241102

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: f593f07c97724141b65e9eb1b9aa4a3bfdb47b2a (20240117145030)
End: 504ffc20d2ebb35be48357830d614ec96765c802 (20240117093935)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f593f07c97724141b65e9eb1b9aa4a3bfdb47b2a&tochange=504ffc20d2ebb35be48357830d614ec96765c802

Comment 3

[Jonathan Kew [:jfkthame]]

The crash appears to be a null-dereference here. This must mean that this (supposed to be an nsFloatManager) is actually null. The class has no virtual functions (hence no vtable) and mLineLeft is its first data member, so it makes sense that attempting to read mLineLeft would read from 0x000000000000.

For aReflowInput.mFloatManager to be null here, we must have gotten false from the call to BlockNeedsFloatManager earlier, so that we didn't create one (and there wasn't one already in the ReflowInput passed by the parent).

If we determined that we don't need a float manager, presumably we also don't need to call PushState/PopState on it, so we can make all those calls conditional on the needFloatManager flag. I'll push something to tryserver and see how it looks.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: slightly modified testcase

The original testcase crashes only intermittently for me, so here is a slightly modified version that reliably gives me a crash as soon as I load it.

Comment 5

[Jonathan Kew [:jfkthame]]

Sometimes there isn't any float-manager in the ReflowInput passed to nsBlockFrame, in which case we have to instantiate one to keep BlockReflowState happy, even if this particular frame doesn't "need" one. This happens when the reflow is started at the block-frame itself, rather than from the root of the frame tree. We can see this in the crash reports (e.g. comment 1), where nsBlockFrame::Reflow is called directly from the PresShell that is initiating this (partial) reflow.

So we should ensure here that we create a float manager if the caller didn't pass one at all, or if this block explicitly needs its own.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1928724 - Ensure we always have a float manager during block-frame reflow. r=#layout

Comment 7

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

Based on comment #2, this bug contains a bisection range found by bugmon. However, the Regressed by field is still not filled.

:jfkthame, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit BugBot documentation.

Comment 8

[Jonathan Kew [:jfkthame]]

From the given regression range, I'm pretty sure this comes from bug 1765615.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/421d7bf24007
Ensure we always have a float manager during block-frame reflow. r=layout-reviewers,emilio

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49167 for changes under testing/web-platform/tests

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/421d7bf24007

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 13

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20241114211619-0191fbfc9115.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.