Found by the BlackBerry Security Automated Analysis Team's fuzzing framework ALF. #0 0x7f95d78eba06 (libmozalloc.so!mozalloc_abort(char const*)+0x66) Line 30 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc_abort.cpp" #1 0x7f95d78eb759 (libmozalloc.so!mozalloc_handle_oom(unsigned long)+0x1a9) Line 50 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc_oom.cpp" #2 0x7f95d78eb5a1 (libmozalloc.so!moz_xmalloc+0x21) Line 54 of "/builds/slave/m-in-l64-asan-0000000000000000/build/memory/mozalloc/mozalloc.cpp" #3 0x7f95d344ab43 (libxul.so!_ZNSt6vectorI18_cairo_path_data_tSaIS0_EE19_M_emplace_back_auxIJRKS0_EEEvDpOT_+0xa3) Line 201 of "../../dist/include/mozilla/mozalloc.h" #4 0x7f95d3441946 (libxul.so!mozilla::gfx::PathBuilderCairo::BezierTo(mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&)+0x3a6) Line 891 of "/tools/gcc-4.7.2-0moz1/lib/gcc/x86_64-unknown-linux-gnu/4.7.2/../../../../include/c++/4.7.2/bits/stl_vector.h" #5 0x7f95ceb2b782 (libxul.so!mozilla::dom::CanvasRenderingContext2D::BezierTo(mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&)+0x422) Line 463 of "../../dist/include/mozilla/dom/CanvasRenderingContext2D.h" #6 0x7f95d0a5fe1a (libxul.so!void mozilla::gfx::ArcToBezier<mozilla::dom::CanvasRenderingContext2D>(mozilla::dom::CanvasRenderingContext2D*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::SizeTyped<mozilla::gfx::UnknownUnits> const&, float, float, bool)+0xa6a) Line 79 of "../../../dist/include/mozilla/gfx/PathHelpers.h" #7 0x7f95d0a5f358 (libxul.so!mozilla::dom::CanvasRenderingContext2D::Arc(double, double, double, double, double, bool, mozilla::ErrorResult&)+0x1c8) Line 1908 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/canvas/src/CanvasRenderingContext2D.cpp" #8 0x7f95ceb29934 (libxul.so!mozilla::dom::CanvasRenderingContext2DBinding::arc(JSContext*, JS::Handle<JSObject*>, mozilla::dom::CanvasRenderingContext2D*, JSJitMethodCallArgs const&)+0x8f4) Line 4687 of "./CanvasRenderingContext2DBinding.cpp" #9 0x7f95ceb28ea7 (libxul.so!mozilla::dom::CanvasRenderingContext2DBinding::genericMethod(JSContext*, unsigned int, JS::Value*)+0x537) Line 4730 of "./CanvasRenderingContext2DBinding.cpp" #10 0x7f95d451c44b (libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)+0x3bb) Line 220 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/jscntxtinlines.h" #11 0x7f95d450e107 (libxul.so!Interpret(JSContext*, js::RunState&)+0x1a067) Line 2611 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/vm/Interpreter.cpp" #12 0x7f95d44f3fb3 (libxul.so!js::RunScript(JSContext*, js::RunState&)+0x3c3) Line 423 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/vm/Interpreter.cpp" #13 0x7f95d451ca21 (libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)+0x991) Line 485 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/vm/Interpreter.cpp" #14 0x7f95d44b72f6 (libxul.so!js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)+0x726) Line 522 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/vm/Interpreter.cpp" #15 0x7f95d42049cf (libxul.so!JS::Call(JSContext*, JS::Value, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)+0xdf) Line 5004 of "/builds/slave/m-in-l64-asan-0000000000000000/build/js/src/../../js/src/jsapi.cpp" #16 0x7f95ced8cc94 (libxul.so!mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, nsDOMEvent&, mozilla::ErrorResult&)+0x394) Line 36 of "./EventHandlerBinding.cpp" #17 0x7f95d02428d4 (libxul.so!JS::Value mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, nsDOMEvent&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling)+0x244) Line 63 of "../../dist/include/mozilla/dom/EventHandlerBinding.h" #18 0x7f95d0241580 (libxul.so!nsJSEventListener::HandleEvent(nsIDOMEvent*)+0xca0) Line 238 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsJSEventListener.cpp" #19 0x7f95d022c42e (libxul.so!nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEvent*, mozilla::dom::EventTarget*)+0x1ce) Line 948 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventListenerManager.cpp" #20 0x7f95d022d55f (libxul.so!nsEventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*)+0x92f) Line 1008 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventListenerManager.cpp" #21 0x7f95d021cd21 (libxul.so!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor&, ELMCreationDetector&)+0x4d1) Line 327 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventListenerManager.h" #22 0x7f95d021bc03 (libxul.so!nsEventTargetChainItem::HandleEventTargetChain(nsTArray<nsEventTargetChainItem>&, nsEventChainPostVisitor&, nsDispatchingCallback*, ELMCreationDetector&)+0x443) Line 284 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventDispatcher.cpp" #23 0x7f95d021fc0a (libxul.so!nsEventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, nsDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*)+0x28fa) Line 594 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsEventDispatcher.cpp" #24 0x7f95d15bddf1 (libxul.so!nsDocumentViewer::LoadComplete(tag_nsresult)+0xa01) Line 1002 of "/builds/slave/m-in-l64-asan-0000000000000000/build/layout/base/nsDocumentViewer.cpp" #25 0x7f95d22c44ea (libxul.so!nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, tag_nsresult)+0x5ca) Line 6869 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp" #26 0x7f95d22c1776 (libxul.so!nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult)+0x14b6) Line 6666 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp" #27 0x7f95d22c1cbc (libxul.so!non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult)+0xc) Line 6672 of "/builds/slave/m-in-l64-asan-0000000000000000/build/docshell/base/nsDocShell.cpp" #28 0x7f95ce480fdf (libxul.so!nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, tag_nsresult)+0x47f) Line 1331 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp" #29 0x7f95ce480333 (libxul.so!nsDocLoader::doStopDocumentLoad(nsIRequest*, tag_nsresult)+0x263) Line 865 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp" #30 0x7f95ce47dfcf (libxul.so!nsDocLoader::DocLoaderIsEmpty(bool)+0x7ef) Line 755 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp" #31 0x7f95ce47f548 (libxul.so!nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x5b8) Line 639 of "/builds/slave/m-in-l64-asan-0000000000000000/build/uriloader/base/nsDocLoader.cpp" #32 0x7f95ce47fde9 (libxul.so!non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x9) Line 642 of "Unified_cpp_uriloader_base0.cpp" #33 0x7f95cd23ce09 (libxul.so!nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, tag_nsresult)+0x799) Line 689 of "/builds/slave/m-in-l64-asan-0000000000000000/build/netwerk/base/src/nsLoadGroup.cpp" #34 0x7f95d075e7b6 (libxul.so!nsDocument::DoUnblockOnload()+0x226) Line 8073 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/base/src/nsDocument.cpp" #35 0x7f95d075e46b (libxul.so!nsDocument::UnblockOnload(bool)+0x55b) Line 8001 of "/builds/slave/m-in-l64-asan-0000000000000000/build/content/base/src/nsDocument.cpp" #36 0x7f95d018c811 (libxul.so!nsLoadBlockingAsyncDOMEvent::~nsLoadBlockingAsyncDOMEvent()+0x81) Line 78 of "/builds/slave/m-in-l64-asan-0000000000000000/build/dom/events/nsAsyncDOMEvent.cpp" #37 0x7f95ccfa8acd (libxul.so!nsRunnable::Release()+0x7d) Line 32 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp" #38 0x7f95cd0d5e41 (libxul.so!nsThread::ProcessNextEvent(bool, bool*)+0xb91) Line 433 of "../../dist/include/nsCOMPtr.h" #39 0x7f95ccfa9df1 (libxul.so!NS_ProcessNextEvent(nsIThread*, bool)+0xb1) Line 263 of "/builds/slave/m-in-l64-asan-0000000000000000/build/xpcom/glue/nsThreadUtils.cpp" #40 0x7f95cd907fd1 (libxul.so!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)+0x311) Line 95 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/glue/MessagePump.cpp" #41 0x7f95cd87cf03 (libxul.so!MessageLoop::Run()+0x1c3) Line 226 of "/builds/slave/m-in-l64-asan-0000000000000000/build/ipc/chromium/src/base/message_loop.cc" #42 0x7f95cfbb023c (libxul.so!nsBaseAppShell::Run()+0x5c) Line 161 of "/builds/slave/m-in-l64-asan-0000000000000000/build/widget/xpwidgets/nsBaseAppShell.cpp" #43 0x7f95d293b666 (libxul.so!nsAppStartup::Run()+0xc6) Line 276 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/components/startup/nsAppStartup.cpp" #44 0x7f95d2753cb5 (libxul.so!XREMain::XRE_mainRun()+0x1de5) Line 4090 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp" #45 0x7f95d2754bea (libxul.so!XREMain::XRE_main(int, char**, nsXREAppData const*)+0x4fa) Line 4158 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp" #46 0x7f95d2755b1b (libxul.so!XRE_main+0x3ab) Line 4368 of "/builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsAppRunner.cpp" #47 0x459dcd (firefox!main+0x94d) Line 280 of "/builds/slave/m-in-l64-asan-0000000000000000/build/browser/app/nsBrowserApp.cpp" #48 0x7f95dd98476c (libc.so.6!__libc_start_main+0xec) Line 226 of "libc-start.c" #49 0x45934c (firefox!_start+0x28)

This is a bug in ArcToBezier I think.

This behaves very badly on mac as well by the way (with CG canvas backend instead of Cairo)

Trying to use lldb on firefox with hilarious results. Waiting to rebuild my tree. I suspect that we don't sanitize the arguments to the arc -> bezier algorithm, which causes badness.

Comment on attachment 8399180 [details] [diff] [review] Normalize start and end angle in ArcToBezier Bas, jeff, can either of you please review and if r+ please land for me. Thanks!

Also, thanks to our friends at blackberry for reporting this!

Comment on attachment 8399180 [details] [diff] [review] Normalize start and end angle in ArcToBezier Review of attachment 8399180 [details] [diff] [review]: ----------------------------------------------------------------- Do you know what goes wrong to require the angle to be sanitized?

Check out the loop a few lines below or catch it in gdb. Basically we are i-looping.

(In reply to Andreas Gal :gal from comment #8) > Check out the loop a few lines below or catch it in gdb. Basically we are > i-looping. I'm not sure fixing this matters (i.e. is it worth doing the fmod on every arc operation?). A user could just as easily cause the same OOM by adding points to the path manually.

Comment on attachment 8399180 [details] [diff] [review] Normalize start and end angle in ArcToBezier Review of attachment 8399180 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/PathHelpers.h @@ +17,5 @@ > float aStartAngle, float aEndAngle, bool aAntiClockwise) > { > + aStartAngle = fmodf(aStartAngle, 2.0f * M_PI); > + aEndAngle = fmodf(aEndAngle, 2.0f * M_PI); > + I think this is a good change to make, but I don't think it can be this simple. We need to take into account and possibly adjust the aAntiClockwise flag when doing this. For example, (in degrees), start angle 350, end angle 370, which sweeps 20 degrees. With this change, end becomes 10, and things don't quite work after that. (although I'm not sure I understand what the correction code is trying to do in the first place.) Similarly, difference of more than 360deg used to be a full circle. So, start 10, end 370 would give us a full circle. Now, both start and end become 10, and we get nothing. Should be a small enough change to make.

Btw, my comments are non-withstanding the "should we be doing normalization in the first place" conversation.

(In reply to Milan Sreckovic [:milan] (TPE timezone) from comment #10) > Comment on attachment 8399180 [details] [diff] [review] > Normalize start and end angle in ArcToBezier > > Review of attachment 8399180 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/2d/PathHelpers.h > @@ +17,5 @@ > > float aStartAngle, float aEndAngle, bool aAntiClockwise) > > { > > + aStartAngle = fmodf(aStartAngle, 2.0f * M_PI); > > + aEndAngle = fmodf(aEndAngle, 2.0f * M_PI); > > + > > I think this is a good change to make, but I don't think it can be this > simple. We need to take into account and possibly adjust the aAntiClockwise > flag when doing this. For example, (in degrees), start angle 350, end angle > 370, which sweeps 20 degrees. With this change, end becomes 10, and things > don't quite work after that. (although I'm not sure I understand what the > correction code is trying to do in the first place.) > Similarly, difference of more than 360deg used to be a full circle. So, > start 10, end 370 would give us a full circle. Now, both start and end > become 10, and we get nothing. > Should be a small enough change to make. OK, I take away the first complaint, that should be OK. I think the full circle one still stands...

Comment on attachment 8399180 [details] [diff] [review] Normalize start and end angle in ArcToBezier This seems to only avoid an OOM that is just as easily caused by adding lots of segments manually, so I don't see much point in adding the extra code/runtime for that.

Any other suggestions how to fix this? This is a 1-liner that causes a crash and it clearly shouldn't.

(In reply to Andreas Gal :gal from comment #14) > Any other suggestions how to fix this? This is a 1-liner that causes a crash > and it clearly shouldn't. It's just an OOM though. You can cause the same thing with while (1) { ctx->lineTo(); }. I don't see what's special about this case.

Something in https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d3228c82badd&tochange=33dc8a83cfc0 made this testcase stop behaving badly. I would assume bug 1187210.