455976 - Crash [@ argb32_image_mark] with border-image on very wide element

Status: VERIFIED FIXED

(Core :: Graphics, defect, P3)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Looks a bit scary: dies trying to derefernce the bogus address bdf7df78.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems more likely Gfx than Layout, though I could be wrong.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

10.5.4?

Comment 4

[Jesse Ruderman]

I can reproduce this on my MBP, which uses Mac OS X 10.5.5.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: workaround

I'm working on isolating the crash to a simple thing that I can send to apple, but here's a workaround for us.  For Quartz, we don't need PAD to get correct edge behaviour, and PAD makes us take a fairly bogus path.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That's getting kinda gross with platform #ifdefs in cross-platform code. Can we define a mode like gfxPattern::EXTEND_PAD_EDGE and map that to EXTEND_PAD or EXTEND_NONE depending on platform?

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: workaround, v2 [pushed]

Yeah, good point.. here's that.  I've got an additional patch in progress to clean up SetExtend usage in nsThebesImage (and SetFilter with the magic numbers), but that's not necessary for this.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 340001 [details] [diff] [review]
workaround, v2 [pushed]

+        // Our own private flag for setting either NONE or PAD,
+        // depending on what the platform does for NONE.

Say a little more about what this flag means and what platforms do with it.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in; this should resolve this crash.  I'm pretty sure given our current usage we can't hit the crashing case again, but I'm not sure how to mark this bug -- the immediate security impact was resolved, but we should still do more analysis on the underlying cause (ideally creating a minimized testcase binary, since the crash is in CGPattern code).  Going to mark this down to wanted/P3 -- can the security folks adjust security flags as appropriate?

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: restore xlib behavior [pushed]

For broken border-image reftests.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1222374248.1222378342.24174.gz

Comment 11

[Karl Tomlinson (:karlt)]

Comment on attachment 340445 [details] [diff] [review]
restore xlib behavior [pushed]

http://hg.mozilla.org/mozilla-central/rev/617e674f9eef

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 340001 [details] [diff] [review]
workaround, v2 [pushed]

with comment changes:
http://hg.mozilla.org/mozilla-central/rev/3c9f6c799fed

Comment 13

[Jesse Ruderman]

Vlad, I'd prefer if you filed a new bug on the remaining issues/analysis.  A security bug was fixed here and that needs to be tracked clearly.

Comment 14

[Jesse Ruderman]

A related testcase still crashes.  I filed bug 458653.

Comment 15

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081013 Minefield/3.1b2pre. I verified using the testcase in Comment 0.

Comment 16

[Brandon Sterne (:bsterne)]

This bug needs verified/fixed keywords for tracking.