1536892 - crash near null in [@ mozilla::SVGGeometryFrame::GetCanvasTM]

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Reduced with m-c:
BuildID=20190320112939
SourceStamp=25398e555020fef80c7b2a06a0d4c667e861cd6f

==117007==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000030 (pc 0x7f9900b50c7b bp 0x7ffcd1f17690 sp 0x7ffcd1f175a0 T0)
==117007==The signal is caused by a READ memory access.
==117007==Hint: address points to the zero page.
    #0 0x7f9900b50c7a in get src/obj-firefox/dist/include/nsCOMPtr.h:823:48
    #1 0x7f9900b50c7a in operator nsIContent * src/obj-firefox/dist/include/nsCOMPtr.h:831
    #2 0x7f9900b50c7a in GetContent src/layout/generic/nsIFrame.h:724
    #3 0x7f9900b50c7a in mozilla::SVGGeometryFrame::GetCanvasTM() src/layout/svg/SVGGeometryFrame.cpp:631
    #4 0x7f9900bdfab1 in nsSVGMarkerFrame::GetCanvasTM() src/layout/svg/nsSVGMarkerFrame.cpp:74:38
    #5 0x7f9900bf8c44 in nsSVGMarkerAnonChildFrame::GetCanvasTM() src/layout/svg/nsSVGMarkerFrame.h:154:57
    #6 0x7f9900bc58a7 in nsSVGForeignObjectFrame::GetCanvasTM() src/layout/svg/nsSVGForeignObjectFrame.cpp:468:62
    #7 0x7f9900bff08a in nsSVGUtils::GetCanvasTM(nsIFrame*) src/layout/svg/nsSVGUtils.cpp:344:59
    #8 0x7f9900ba1f3f in nsFilterInstance::GetPostFilterBounds(nsIFrame*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, double> const*, nsRect const*) src/layout/svg/nsFilterInstance.cpp:430:18
    #9 0x7f9900bfc603 in nsSVGUtils::GetPostFilterVisualOverflowRect(nsIFrame*, nsRect const&) src/layout/svg/nsSVGUtils.cpp:145:10
    #10 0x7f990072ad10 in ComputeEffectsRect src/layout/generic/nsFrame.cpp:7171:11
    #11 0x7f990072ad10 in nsIFrame::FinishAndStoreOverflow(nsOverflowAreas&, nsSize, nsSize*, nsStyleDisplay const*) src/layout/generic/nsFrame.cpp:9219
    #12 0x7f990072769e in nsIFrame::UpdateOverflow() src/layout/generic/nsFrame.cpp:7334:7
    #13 0x7f990034f229 in mozilla::OverflowChangedTracker::Flush() src/layout/base/OverflowChangedTracker.h:107:34
    #14 0x7f990036caaf in FlushOverflowChangedTracker src/obj-firefox/dist/include/mozilla/RestyleManager.h:217:64
    #15 0x7f990036caaf in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) src/layout/base/RestyleManager.cpp:3117
    #16 0x7f9900301bb9 in ProcessPendingRestyles src/layout/base/RestyleManager.cpp:3155:3
    #17 0x7f9900301bb9 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4122
    #18 0x7f990026bb85 in FlushPendingNotifications src/layout/base/nsIPresShell.h:580:5
    #19 0x7f990026bb85 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1887
    #20 0x7f9900280499 in TickDriver src/layout/base/nsRefreshDriver.cpp:342:13
    #21 0x7f9900280499 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:319
    #22 0x7f990027fd88 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:336:5
    #23 0x7f9900283fcf in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:777:5
    #24 0x7f9900283fcf in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:697
    #25 0x7f990028318a in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) src/layout/base/nsRefreshDriver.cpp:592:9
    #26 0x7f9900d77215 in mozilla::layout::VsyncChild::RecvNotify(mozilla::VsyncEvent const&) src/layout/ipc/VsyncChild.cpp:65:16
    #27 0x7f98f74755cb in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:168:54
    #28 0x7f98f6ff93d7 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2828:28
    #29 0x7f98f6861749 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2151:21
    #30 0x7f98f685d54a in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2078:9
    #31 0x7f98f685f787 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1937:3
    #32 0x7f98f6860517 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1968:13
    #33 0x7f98f5595c51 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1180:14
    #34 0x7f98f559e05d in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:482:10
    #35 0x7f98f686ab4f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
    #36 0x7f98f67408ce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #37 0x7f98f67408ce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #38 0x7f98f67408ce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #39 0x7f98ffb93903 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #40 0x7f990416eece in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:933:20
    #41 0x7f98f67408ce in RunInternal src/ipc/chromium/src/base/message_loop.cc:315:10
    #42 0x7f98f67408ce in RunHandler src/ipc/chromium/src/base/message_loop.cc:308
    #43 0x7f98f67408ce in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #44 0x7f990416e05c in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:771:34
    #45 0x55eff2ecb834 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
    #46 0x55eff2ecb834 in main src/browser/app/nsBrowserApp.cpp:263

Comment 1

[violet.bugreport]

Attachment: Bug 1536892 - Check a frame does maintain overflow before adding to OverflowChangedTracker

If a frame doesn't maintain overflow, do not add it to the OverflowChangedTracker

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/94b858d36603
Check a frame does maintain overflow before adding to OverflowChangedTracker. r=longsonr CLOSED TREE

Comment 4

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/94b858d36603

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Sean, since 67 is marked as affected, should we consider uplifting this to Beta67?

Comment 6

[Daniel Holbert [:dholbert]]

To help answer that decision, I tracked down the regression range.

Last good revision: d71374b4e528c667eaa83672bcefe9f1d415ce8c
First bad revision: 68592cefee573b075b68880e65828e820e2e16ed
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d71374b4e528c667eaa83672bcefe9f1d415ce8c&tochange=68592cefee573b075b68880e65828e820e2e16ed

So we've been living with this bug since Firefox 64, with no reports besides this one fuzzer bug.

Given that the crash doesn't seem easily exploitable and we're not aware of any real content hitting it (just this fuzzer testcase), I tend to think we should just let it ride the trains.

Comment 7

[Sean Voisen (:svoisen)]

Comment 6 sounds good to me.

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Thanks Daniel, Sean. That seems wise. I presume the fix has some risk associated which is why letting it ride the trains is better.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Ritu Kothari (:ritu) from comment #8)

I presume the fix has some risk associated which is why letting it ride the trains is better.

Yeah -- I don't know of any specific risk, but I don't feel confident enough to say it's zero-risk. So: uplift has a small risk without a clear reward, hence leaning towards riding the trains.