Closed Bug 624198 Opened 13 years ago Closed 13 years ago

Crash [@ _cairo_clip_path_destroy] [@ nsMathMLChar::PaintForeground] on old versions of Windows

Categories

(Core :: Graphics, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla2.0b10
Tracking Status
blocking2.0 --- betaN+
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: jruderman, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: crash, sec-critical, testcase, Whiteboard: [sg:critical])

Crash Data

Attachments

(7 files)

I think this testcase causes crashes on older versions of Windows (XP? 2003?).
Summary: Crash [@ _cairo_clip_path_destroy] on old versions of Windows → Crash [@ _cairo_clip_path_destroy] [@ nsMathMLChar::PaintForeground] on old versions of Windows
http://hg.mozilla.org/mozilla-central/annotate/4a3866321a14/gfx/cairo/cairo/src/cairo-gstate.c#l1848
_cairo_scaled_font_glyph_path on line 1883 fails so 'clip' is never
assigned and contains random stack data when '_cairo_clip_fini'
(_cairo_clip_reset) is executed on line 1901.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: pp
Attached patch crash fixSplinter Review
Assignee: nobody → matspal
Attachment #502390 - Flags: review?(vladimir)
There's also an unrelated bug in '_cairo_win32_print_gdi_error'
which fails to report the gdi error.

It's the 'GetGlyphOutlineW' that fails on line 1712 and 
'_cairo_win32_print_gdi_error' is called on line 1717 but the error
message never appears for me (on XP).
http://hg.mozilla.org/mozilla-central/annotate/4a3866321a14/gfx/cairo/cairo/src/cairo-win32-font.c#l1677

This patch makes _cairo_win32_print_gdi_error work for me.

The error message is "_cairo_win32_scaled_font_glyph_path: Cannot complete this function."
Attachment #502391 - Flags: review?(vladimir)
Jesse on IRC: "is it likely that lots of cairo bugs are like that?"

I don't read cairo code that much so I don't want to speculate on that.
Maybe we could override some of those Windows functions and inject errors...
For this testcase though, I expect that valgrind would have caught it.
Comment on attachment 502390 [details] [diff] [review]
crash fix

Make sure to coordinate with jeff on getting this upstream, and when you check in do the additional patched-cairo dance.
Attachment #502390 - Flags: review?(vladimir) → review+
Comment on attachment 502391 [details] [diff] [review]
print_gdi_error fix

Same thing with this patch
Attachment #502391 - Flags: review?(vladimir) → review+
Would we ever write back to 'clip'? So far this looks like a bogus read, and as long as it's only reading we might be OK. Unless of course we use 'clip' to define the memory extent of something else we're writing to.
Looks like it tries to do some fairly complex cleanup and dest of memory structures per https://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-surface.c#570  It might result in memory corruption similar to a double-free scenario.
The crash fix here should also fix the crash in bug 595423 (which is
sg:critical).
Blocks: 595423
Keywords: pp
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [sg:critical]
Attachment #503879 - Flags: approval2.0?
I haven't filed bugs on https://bugs.freedesktop.org/ yet because I can't
find a way to report a security sensitive bug there...
Given that this is sg:critical and we have patches, I'm going to block 2.0 on this. Feel free to land the patches ASAP!
blocking2.0: --- → betaN+
Attachment #503879 - Flags: approval2.0? → approval2.0+
Attachment #503880 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/6db090a3aaa0
http://hg.mozilla.org/mozilla-central/rev/ba49fff91d82

Crash test intentionally not landed yet.
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
This seemed to cause a:

Regression :( Trace Malloc Leaks increase 14.5% on MacOSX 10.5.2 Firefox
------------------------------------------------------------------------
   Previous: avg 1815027.333 stddev 138039.291 of 30 runs up to revision 8d9465a318f5
   New     : avg 2078330.000 stddev 3397.793 of 5 runs since revision ba49fff91d82

Perhaps it's worth backing out to see what if it gets better.
For the record, the reported leak is an expected regression from bug 627498.
Talos automatic leak detection blamed the wrong changeset, bug 627860 filed.
See discussion in mozilla.dev.tree-management for details.
Nothing to worry about here.
Crash Signature: [@ _cairo_clip_path_destroy] [@ nsMathMLChar::PaintForeground]
I'd like to unhide this bug so I can land the crash test.  It's been fixed
since Firefox 4 so I think that's safe to do now, but this was a bug in
upstream Cairo so I thought I'd ask.  Chris Wilson said 2012-02-09 on
https://bugs.freedesktop.org/show_bug.cgi?id=33318
"That code is now obsolete."
Crash Signature: [@ _cairo_clip_path_destroy] [@ nsMathMLChar::PaintForeground] → [@ _cairo_clip_path_destroy] [@ nsMathMLChar::PaintForeground]
Flags: needinfo?(abillings)
I'm opening this up after talking to Dveditz.
Group: core-security
Keywords: sec-critical
Landed the crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/848697281a2f
Flags: needinfo?(abillings)
Flags: in-testsuite?
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.