crash near null [@ full_row] in glitter_scan_converter_render
Categories
(Core :: Printing: Output, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr115 | --- | unaffected |
| firefox-esr128 | --- | wontfix |
| firefox133 | --- | wontfix |
| firefox134 | --- | fixed |
| firefox135 | --- | verified |
People
(Reporter: tsmith, Assigned: jfkthame)
References
(Blocks 1 open bug, Regression)
Details
(4 keywords, Whiteboard: [bugmon:bisected,confirmed], [wptsync upstream])
Attachments
(3 files, 1 obsolete file)
Found while fuzzing m-c 20241027-00dc3a0cb565 (--enable-debug --enable-fuzzing)
To reproduce via Grizzly Replay:
$ pip install fuzzfetch grizzly-framework --upgrade
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>
==197248==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x00000000001c (pc 0x70991befdf25 bp 0x7ffd77ce6d70 sp 0x7ffd77ce6c30 T197248)
==197248==The signal is caused by a READ memory access.
==197248==Hint: address points to the zero page.
#0 0x70991befdf25 in full_row /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-tor-scan-converter.c:1338:48
#1 0x70991befdf25 in glitter_scan_converter_render /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-tor-scan-converter.c:1767:6
#2 0x70991befdf25 in _cairo_tor_scan_converter_generate /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-tor-scan-converter.c:1857:5
#3 0x70991bf5cbe3 in composite_polygon /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-spans-compositor.c:801:11
#4 0x70991bf5c88a in clip_and_composite_polygon /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-spans-compositor.c:967:12
#5 0x70991bf5172c in _cairo_spans_compositor_fill /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-spans-compositor.c:1174:15
#6 0x70991bf1c7b6 in _cairo_compositor_fill /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-compositor.c:245:11
#7 0x70991bf2b6e4 in _cairo_image_surface_fill /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-image-surface.c:1018:12
#8 0x70991bf54a46 in _cairo_surface_fill /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-surface.c:2502:14
#9 0x70991bf21dbd in _cairo_gstate_fill /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo-gstate.c:1352:15
#10 0x70991bf617a8 in _moz_cairo_fill_preserve /builds/worker/checkouts/gecko/gfx/cairo/cairo/src/cairo.c:2454:14
#11 0x70991c037a4e in mozilla::gfx::DrawTargetCairo::DrawPattern(mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::DrawTargetCairo::DrawPatternType, bool) /builds/worker/checkouts/gecko/gfx/2d/DrawTargetCairo.cpp:1052:7
#12 0x70991c039b02 in mozilla::gfx::DrawTargetCairo::Fill(mozilla::gfx::Path const*, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&) /builds/worker/checkouts/gecko/gfx/2d/DrawTargetCairo.cpp:1277:3
#13 0x70991c00eff4 in mozilla::gfx::RecordedFill::PlayEvent(mozilla::gfx::Translator*) const /builds/worker/checkouts/gecko/gfx/2d/RecordedEventImpl.h:2657:7
#14 0x70991c088e52 in operator() /builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/std_function.h:687:14
#15 0x70991c088e52 in bool mozilla::gfx::RecordedEvent::DoWithEvent<mozilla::gfx::EventStream>(mozilla::gfx::EventStream&, mozilla::gfx::RecordedEvent::EventType, std::function<bool (mozilla::gfx::RecordedEvent*)> const&) /builds/worker/checkouts/gecko/gfx/2d/RecordedEventImpl.h:4577:5
#16 0x709920f5f457 in mozilla::layout::PrintTranslator::TranslateRecording(mozilla::layout::PRFileDescStream&) /builds/worker/checkouts/gecko/layout/printing/PrintTranslator.cpp:54:20
#17 0x709920f61ccc in mozilla::layout::RemotePrintJobParent::PrintPage(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layout::PRFileDescStream&, nsRefCountedHashtable<nsIntegralHashKey<unsigned long, 0>, RefPtr<mozilla::gfx::RecordedDependentSurface>>*) /builds/worker/checkouts/gecko/layout/printing/ipc/RemotePrintJobParent.cpp:179:26
#18 0x709920f61b05 in mozilla::layout::RemotePrintJobParent::FinishProcessingPage(mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, nsRefCountedHashtable<nsIntegralHashKey<unsigned long, 0>, RefPtr<mozilla::gfx::RecordedDependentSurface>>*) /builds/worker/checkouts/gecko/layout/printing/ipc/RemotePrintJobParent.cpp:158:17
#19 0x709920f6185d in mozilla::layout::RemotePrintJobParent::RecvProcessPage(int const&, int const&, nsTArray<unsigned long>&&) /builds/worker/checkouts/gecko/layout/printing/ipc/RemotePrintJobParent.cpp:132:5
#20 0x709920a2bf87 in mozilla::layout::PRemotePrintJobParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PRemotePrintJobParent.cpp:387:52
#21 0x709920137075 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:7236:32
#22 0x70991ba9377f in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1726:25
#23 0x70991ba90702 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1653:9
#24 0x70991ba91382 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1444:3
#25 0x70991ba924cf in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1544:14
#26 0x70991aef3767 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:618:16
#27 0x70991aee8fc9 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:945:26
#28 0x70991aee7a07 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:768:15
#29 0x70991aee7e85 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:554:36
#30 0x70991aef7146 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:268:37
#31 0x70991aef7146 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#32 0x70991af0a9fb in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1155:16
#33 0x70991af116df in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#34 0x70991ba99305 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#35 0x70991b9eb701 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:362:3
#36 0x70991b9eb701 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:344:3
#37 0x709920759238 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#38 0x70992080b238 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:469:33
#39 0x7099215bb964 in nsAppStartup::Run() /builds/worker/checkouts/gecko/toolkit/components/startup/nsAppStartup.cpp:295:30
#40 0x7099216dff8a in XREMain::XRE_mainRun() /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5789:22
#41 0x7099216e1754 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:6024:8
#42 0x7099216e25a8 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:6097:21
#43 0x5f9db419a126 in do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:232:22
#44 0x5f9db419a126 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:464:16
|
Reporter | |
Comment 1•11 months ago
|
||
Maybe this is just a duplicate of bug 1725070.
|
||
Updated•11 months ago
|
|
||
Comment 2•11 months ago
|
||
Verified bug as reproducible on mozilla-central 20241112214908-aef84d293121.
Unable to bisect testcase (Testcase reproduces on start build!):
Start: 0083ca2f4709e599083520545d538d1ee1b8356d (20231115052415)
End: 00dc3a0cb565582c175dff58141a141ca732d62c (20241027210505)
BuildFlags: BuildFlags(asan=False, tsan=False, debug=True, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False)
Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.
|
||
Comment 4•11 months ago
|
||
The severity field is not set for this bug.
:emilio, could you have a look please?
For more information, please visit BugBot documentation.
|
||
Comment 5•11 months ago
|
||
This looks like a null deref in cairo when trying to deal with thousands of pages... So probably S3? The huge scale is what seems to be triggering this.
|
Assignee | |
Comment 6•11 months ago
|
||
The huge scaling (resulting from * { transform: scale(54); } with multiple levels of nested elements) is leading to FT_Set_Char_Size here returning an error (originating from here when it is attempting to scale metrics, I think).
This results in an error status being set on the font, which bubbles back up to eventually set an error status on the cairo context here. In theory, cairo APIs are supposed to be safe to call even if the context is in an error state (they should just return immediately without doing anything), but I suspect we're not properly handling such a "bad" context in all cases.
|
|
Assignee | |
Comment 7•11 months ago
|
||
The DrawTarget may become invalid at some point while processing a recording,
e.g. if FillGlyphs encounters a FreeType error because it tries to scale a font
beyond what FT can support. In such a case, we should bail out rather than
continuing to try and work with the invalid DT.
|
|
Assignee | |
Comment 8•11 months ago
|
||
This patch ^ is untested (I haven't reproduced the crash locally as yet) but I'm hoping it might avoid the issue; Tyson, if you have time to try applying this and confirm whether it helps, that'd be great - thanks!
|
||
Updated•11 months ago
|
|
|
Assignee | |
Comment 9•11 months ago
|
||
Some local testing suggests that the Translator::GetCurrentDrawTarget() patch didn't help here, so I'm abandoning that.
(FWIW, I do think this is basically the same underlying issue as bug 1725070, as Tyson suggested in comment 1.)
|
|
Assignee | |
Comment 10•11 months ago
|
||
After a bit more poking at this: the Translator::GetCurrentDrawTarget() patch was along the right lines, but failed to avoid the problem because DrawTargetOffset fails to override the default (always-true) implementation of GetValid(), and so a DrawTargetOffset will always claim to be "valid" even if its underlying DT is in an error state.
|
|
Assignee | |
Comment 11•11 months ago
|
||
|
|
||
Updated•11 months ago
|
|
||
Comment 12•11 months ago
|
||
|
|
Reporter | |
Comment 13•11 months ago
|
||
(In reply to Jonathan Kew [:jfkthame] from comment #8)
This patch ^ is untested (I haven't reproduced the crash locally as yet) but I'm hoping it might avoid the issue; Tyson, if you have time to try applying this and confirm whether it helps, that'd be great - thanks!
I did attempt to verify the patch but I was having trouble reproducing the crash and not the assertion failure (bug 1725070). With the original patch applied I still saw the assertion failure but much less frequently.
|
|
Assignee | |
Comment 15•11 months ago
|
||
(In reply to Tyson Smith [:tsmith] from comment #13)
(In reply to Jonathan Kew [:jfkthame] from comment #8)
This patch ^ is untested (I haven't reproduced the crash locally as yet) but I'm hoping it might avoid the issue; Tyson, if you have time to try applying this and confirm whether it helps, that'd be great - thanks!
I did attempt to verify the patch but I was having trouble reproducing the crash and not the assertion failure (bug 1725070). With the original patch applied I still saw the assertion failure but much less frequently.
Thanks -- later I was able to get a local fuzzing build going and reproduce it locally, so I'm pretty sure the updated patch (just landed) now fixes this. (Confirmation is of course welcome!) It should also fix the assertion from bug 1725070.
|
||
Comment 16•11 months ago
|
||
Backed out for causing failures at huge-font-crash-print.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/b33c31737dfcf540274e4c8aae2f04775c20c01a
Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=E-a-4Y2xSY-CHSkinw-ZUw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=c8b91d4176a9c49d0a6fb1dd9bbce5a9b7184095
Failure log: https://treeherder.mozilla.org/logviewer?job_id=484644201&repo=autoland&lineNumber=4925
|
|
Assignee | |
Comment 18•11 months ago
|
||
Ugh.... that's a different crash than the original issue being fixed here. I guess the huge scale is causing some other kind of breakage in cairo.... will try to look into it. Interestingly, though, it doesn't crash like this for me locally. :-\
|
|
Assignee | |
Comment 19•10 months ago
|
||
The point of the included test is that it should not crash (or assert) due to a bad surface/drawtarget, as soon as it tries to generate the enormous print job. That seems to be working fine; but I'm disabling it on ASAN builds, as it frequently (though not consistently) ends up reporting leaks after it times out.
|
|
||
Comment 20•10 months ago
|
||
|
||
Comment 21•10 months ago
|
||
| bugherder | ||
|
|
||
Comment 23•10 months ago
|
||
Verified bug as fixed on rev mozilla-central 20241205213207-9dfed8478876.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.
|
|
||
Comment 24•10 months ago
|
||
The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
- If no, please set
status-firefox134towontfix.
For more information, please visit BugBot documentation.
|
|
Assignee | |
Comment 25•10 months ago
|
||
Original Revision: https://phabricator.services.mozilla.com/D230532
|
|
||
Updated•10 months ago
|
|
|
||
Comment 26•10 months ago
|
||
beta Uplift Approval Request
- User impact if declined: Potential null-deref crash triggered by extreme content sizes
- Code covered by automated testing: yes
- Fix verified in Nightly: yes
- Needs manual QE test: no
- Steps to reproduce for manual QE testing: n/a
- Risk associated with taking this patch: minimal
- Explanation of risk level: Just adds missing drawTarget validity checks
- String changes made/needed: none
- Is Android affected?: yes
|
|
||
Updated•10 months ago
|
|
|
||
Comment 27•10 months ago
|
||
| uplift | ||
|
||
Updated•10 months ago
|
|
||
Updated•10 months ago
|
Description
•