Closed
Bug 665360
Opened 13 years ago
Closed 13 years ago
Crash [@ gfxMixedFontFamily::ReplaceFontEntry(gfxFontEntry*, gfxFontEntry*) ]
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: bc, Assigned: jfkthame)
References
()
Details
(5 keywords, Whiteboard: [sg:critical?][qa-])
Crash Data
Attachments
(2 files)
1.51 KB,
patch
|
jtd
:
review+
jst
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.25 KB,
patch
|
christian
:
approval1.9.2.23+
|
Details | Diff | Splinter Review |
1. http://www.eastoncycling.com/en-us/mountain/products/bars/all-mountain/ea70-mb-281 in Nightly on Windows XP 2. Crash Ted's exploitable breakpad anaylyser gives this a high exploitability which actually matches the use of deleted memory. + this 0x097fc460 {mHdr=0xdddddddd } const nsTArray_base<nsTArrayDefaultAllocator> * const // @return The number of elements in the array. size_type Length() const { => return mHdr->mLength; } > xul.dll!nsTArray_base<nsTArrayDefaultAllocator>::Length() Line 140 + 0x5 bytes C++ xul.dll!gfxMixedFontFamily::ReplaceFontEntry(gfxFontEntry * aOldFontEntry=0x097fc028, gfxFontEntry * aNewFontEntry=0x097f60c8) Line 116 + 0xb bytes C++ xul.dll!nsUserFontSet::ReplaceFontEntry(gfxProxyFontEntry * aProxy=0x097fc028, gfxFontEntry * aFontEntry=0x097f60c8) Line 688 C++ xul.dll!gfxUserFontSet::OnLoadComplete(gfxProxyFontEntry * aProxy=0x097fc028, const unsigned char * aFontData=0x00000000, unsigned int aLength=21812, unsigned int aDownloadStatus=0) Line 560 C++ xul.dll!nsFontFaceLoader::OnStreamComplete(nsIStreamLoader * aLoader=0x098ae110, nsISupports * aContext=0x00000000, unsigned int aStatus=0, unsigned int aStringLen=21812, const unsigned char * aString=0x0a0c6c00) Line 220 + 0x20 bytes C++ xul.dll!nsStreamLoader::OnStopRequest(nsIRequest * request=0x098ad8cc, nsISupports * ctxt=0x00000000, unsigned int aStatus=0) Line 125 + 0x3e bytes C++ xul.dll!nsCORSListenerProxy::OnStopRequest(nsIRequest * aRequest=0x098ad8cc, nsISupports * aContext=0x00000000, unsigned int aStatusCode=0) Line 623 C++ I was able to reproduce the crash but not the deleted memory in debug Aurora. I was not able to reproduce the crash in debug Beta. I was not able to reproduce with nightly Nightly or Aurora builds. This was found from crash automation from bp-5279a5dd-12aa-4dcc-bb1d-471152110616 which matches the detected crash.
Comment 1•13 years ago
|
||
What about a debug nightly, or MallocScribble on mac?
Whiteboard: [sg:critical?]
Reporter | ||
Comment 2•13 years ago
|
||
Yes, the stack in comment 0 is from a debug windows Nightly build. I tried today's debug Nightly mac build with scribble enabled and still did not crash. I just reproduced the crash with today's debug Nightly on windows xp with the same deleted memory in mHdr. I have it in visual studio at the moment and will leave it there for a bit if you need me to be a debugging bot. Note when I say nightly I mean an opt build downloaded from ftp.m.o regardless of branch. When I say Nightly (cap N) it is a trunk build.
Comment 3•13 years ago
|
||
Maybe something specific to WinXP fonts? Or specifically the fonts installed on your test system?
Comment 4•13 years ago
|
||
John, can you look into this?
Assignee: nobody → jdaggett
status-firefox5:
--- → wontfix
status-firefox6:
--- → affected
status-firefox7:
--- → affected
tracking-firefox5:
--- → -
tracking-firefox6:
--- → +
tracking-firefox7:
--- → +
Reporter | ||
Comment 5•13 years ago
|
||
Maybe just XP. I don't see it in Windows 7 with the automation. These are pretty generic vms.
Comment 6•13 years ago
|
||
(In reply to comment #4) > John, can you look into this? I'm flying today, I won't be able to look at this until next week. The location of the crash indicates a downloadable font, it's not specific to fonts on XP. But I think this is something jkew already fixed, maybe it just needs to get pushed to a difference branch? Jonathan, can you comment?
Comment 7•13 years ago
|
||
Jonathan, sounds like this needs attention from you, reassigning.
Assignee: jdaggett → jfkthame
Assignee | ||
Comment 8•13 years ago
|
||
The recent fixes I'm aware of in this area are bug 655138 and bug 650639, but both of those are present in Aurora as well as trunk code at this point. Crash-stats indicates this is still happening on aurora (6.0a2) as well as trunk, plus a variety of earlier versions (4.0.1, even 3.x). Interestingly, the most recent trunk build that shows up is from 06/24 (a week ago, at this point), but I don't see anything in the pushlog from around that time that jumps out as a likely explanation for why the crashes might have stopped. Also, crash-stats doesn't list any occurrences on FF5.0. I don't have any explanation for that, when it's happening in both 4.0.1 and 6.0a2. My guess is that the crashes occur when the gfxFontFamily associated with a proxy font entry has been deleted prior to the font load completing, but I haven't figured out an exact scenario that would lead to this - let alone managed to reproduce it. Although we've made some changes in this area lately (especially bug 633299 and followup fixes), I don't think that accounts for the problem here, as we see this crash signature even in much older releases.
Assignee | ||
Comment 9•13 years ago
|
||
I have been running a current Aurora build on Windows for a while hoping to reproduce this, but haven't yet had any success. However, something we could try as a possible band-aid would be to make the nsFontFaceLoader hold a reference to the font family, as well as to the actual proxy on whose behalf it is loading. (Note that the proxy font entry does *not* have a strong reference to its family, only a weak pointer.) If there's some race condition or similar whereby the family can get deleted right before the load completes, this should ensure that it remains valid long enough to avoid this crash. Unless you have any better ideas, John, I'd suggest we try landing this - I don't see how it could do any harm, at least - and see whether it has any effect on crash-stats.
Attachment #543614 -
Flags: review?(jdaggett)
Comment 10•13 years ago
|
||
Seems unlikely that we'll take this for 6, or that we'll even know if it's fixed or not. Tracking for newer versions though.
status-firefox8:
--- → affected
tracking-firefox8:
--- → +
Comment 11•13 years ago
|
||
John, can you review here? We'd love to get this band-aid bug in to see if it helps, which based on what Marcia tells me we'd know whether it helped or not in a matter of days.
Reporter | ||
Comment 12•13 years ago
|
||
fwiw, I am still seeing related crashes in automation and could retest within a matter of hours once this lands.
Updated•13 years ago
|
Attachment #543614 -
Flags: review?(jdaggett) → review+
Assignee | ||
Comment 13•13 years ago
|
||
Pushed to mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/89842a2a40c6 Let's keep this bug open after this merges to m-c, at least until we have time to see whether this affects the crashiness at all.
Whiteboard: [sg:critical?] → [sg:critical?] [inbound]
Comment 14•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/89842a2a40c6
Whiteboard: [sg:critical?] [inbound] → [sg:critical?]
Target Milestone: --- → mozilla8
Comment 15•13 years ago
|
||
bc: please retest and see if the band-aide helped any.
Updated•13 years ago
|
status-firefox9:
--- → affected
tracking-firefox9:
--- → +
Comment 16•13 years ago
|
||
I don't see any crashes in 8.0 builds after the Aug 9 checkin. There were only a handful before, but enough in 8 and 7 that I feel pretty good that we've got a real fix here.
Comment 17•13 years ago
|
||
Comment on attachment 543614 [details] [diff] [review] possible band-aid This patch seems to have fixed the security hole in Firefox 8. It's safe and should be landed on Firefox 7 beta.
Attachment #543614 -
Flags: approval-mozilla-beta?
Reporter | ||
Comment 18•13 years ago
|
||
I only see this on beta now. Appeared fixed (papered over?) on Nightly and Aurora. It is already on Aurora isn't it? http://hg.mozilla.org/releases/mozilla-aurora/rev/89842a2a40c6
Comment 20•13 years ago
|
||
This is in the top fifty crashes for Firefox 6; the band-aid should IMO be considered if we do a patch for that release. And it should definitely be landed on beta ASAP. https://crash-stats.mozilla.com/report/list?range_value=7&range_unit=days&date=2011-08-18&signature=gfxMixedFontFamily%3A%3AReplaceFontEntry%28gfxFontEntry*%2C%20gfxFontEntry*%29&version=Firefox%3A6.0
Updated•13 years ago
|
Comment 21•13 years ago
|
||
Comment on attachment 543614 [details] [diff] [review] possible band-aid Approved for beta per today's triage meeting.
Attachment #543614 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 22•13 years ago
|
||
yeah, lets get this fixed quickly and in as many releases as possible. signature has jumped to over 180 crashes per day after the release of 6.0 gfxMixedFontFamily::ReplaceFontEntry.gfxFontEntry...gfxFontEntry.. date total breakdown by build crashes count build, count build, ... 20110818 187 174 6.02011081116, 4 7.0a22011081604, 3 3.5.12009071509, 2 7.02011081615, 2 6.02011080403, 1 8.0a12011072003, 1 3.6.132010120307, and is visible on wide variety of sites. domains of sites 16 file:// 13 \N// 5 http://www.ibahia.com 4 http://www.probikekit.com 4 http://www.blogger.com 3 javascript:''// 3 http://www.xcid.net 3 http://www.lggetconnected.com 3 http://www.eastoncycling.com 2 http://www.conceptclub.ru 2 http://veldev.com 2 http://tracking.traviangames.com 2 http://swetozar-online.ru 2 http://ilumalite.syntaxmarketing.com.au 2 http://fr.novopress.info 2 http://feedproxy.google.com 2 http://demos.foxymoron.co.in 2 http://conceptclub.ru 2 http://192.168.0.100 2 http://178.79.144.232 1 https://www.change.org 1 https://plus.google.com 1 https://opdagelse.telenor.dk 1 https://ib.nab.com.au 1 https://docs.google.com 1 http://yfrog.com 1 http://www.youtube.com 1 http://www.yellowpagesmyanmar.net urls for testing 13 \N 3 javascript:'' 2 http://www.xcid.net/contacto/survey.php?action=manage 2 http://swetozar-online.ru/catalog/result/ 2 http://demos.foxymoron.co.in/lastwilderness/ 2 http://192.168.0.100/sliced/inne/Crimson%20Oak/DW%20Studio/blog.html 2 file:///D:/portfolio/index.html 1 https://www.change.org/login 1 https://plus.google.com/u/0/_/notifications/frame?hl=en&origin=https%3A%2F%2Fmail.google.com&jsh=r%3Bgc%2F22954852-97da52d8#pid=23&id=gbsf&parent=https%3A%2F%2Fmail.google.com&rpctoken=82175096&_methods=onError%2ConInfo%2ChideNotificationWidget%2CpostShar 1 https://opdagelse.telenor.dk/Bliv-underholdt/Musik/Fa-WiMP/Aktiver-WiMP/ 1 https://ib.nab.com.au/nabib/payments_transferList.ctl 1 https://docs.google.com/a/captiway.com/?tab=oo#home 1 http://yfrog.com/h6athizj 1 http://www.youtube.com/watch?v=HyQqZh2Yx8I&feature=mr_meh&list=ULFHp2KgyQUFk&index=21&playnext=0 1 http://www.yellowpagesmyanmar.net/list_bycat.php?page=2&cid=133&hcid=&loc=1 1 http://www.xcid.net/ 1 http://www.web-web-11.info/unicalle//index.php?option=com_diplomas&view=diploma1&Itemid=8 1 http://www.vodafonefreezone.com/Article.aspx?PageID=108&PageName=senle-ben-paketleri 1 http://www.turkcell.de/internet 1 http://www.thenorthface.cl/catalogo/hombres/footwear/hiking/2016_m-syncline-gtx 1 http://www.thedeyjaycollection.com/shop/ 1 http://www.theatre-octogone.ch/11/index.php?page=saison-theatre 1 http://www.sputnik-centar.hr/ 1 http://www.sounddepo.com/2011/ 1 http://www.silent-guilde.fr/ 1 http://www.sanitarium.com.au/products/vegetarian 1 http://www.ray-ban.com/germany/store-locator
Assignee | ||
Comment 23•13 years ago
|
||
Pushed to mozilla-beta: http://hg.mozilla.org/releases/mozilla-beta/rev/b8d41ea97ce8
Reporter | ||
Comment 24•13 years ago
|
||
beta does not appear to crash on my previous list of urls. This should be FIXED and the stati set to fixed, right?
Assignee | ||
Comment 25•13 years ago
|
||
If you were able to reproduce it consistently before, and can confirm it's fixed, that's great; please mark accordingly. I was never able to reproduce the crash locally, so intended to wait and see what crash-stats showed...
Comment 26•13 years ago
|
||
Ok, marking this FIXED then!
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 27•13 years ago
|
||
Does this affect 1.9.2? It looks like it does as the crash signature shows up on 3.6.x. If so we'll need to take it for 1.9.2.21
Assignee | ||
Comment 28•13 years ago
|
||
(In reply to Christian Legnitto [:LegNeato] from comment #27) > Does this affect 1.9.2? It almost certainly does. The 1.9.2 crash reports look as though they're coming from the same code path. We've never really explained the underlying cause, but the precautionary patch here seems to have stopped the crashes, as far as I can tell. The patch needs minor adjustment to apply on 1.9.2; I'm just building locally to check it's OK, then will post it here.
Assignee | ||
Comment 29•13 years ago
|
||
Attachment #556625 -
Flags: approval1.9.2.21?
Comment 30•13 years ago
|
||
Comment on attachment 556625 [details] [diff] [review] patch for 1.9.2 branch Thanks! Approved for releases/mozilla-1.9.2. Please land on the default branch asap
Attachment #556625 -
Flags: approval1.9.2.23? → approval1.9.2.23+
Assignee | ||
Comment 31•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f5dd7005daa8
status1.9.2:
--- → .23-fixed
Comment 32•13 years ago
|
||
qa-, Bob can you please verify this is fixed? Thanks
Whiteboard: [sg:critical?] → [sg:critical?][qa-]
Reporter | ||
Comment 33•13 years ago
|
||
ashughes: i retested my urls and did not see the crash on 1.9.2, beta/7, aurora/8, nightly/9.
Comment 34•13 years ago
|
||
Thanks Bob. I'm calling this verified.
Status: RESOLVED → VERIFIED
Keywords: verified-aurora,
verified-beta
Updated•13 years ago
|
Group: core-security
Updated•9 years ago
|
Keywords: testcase-wanted
You need to log in
before you can comment on or make changes to this bug.
Description
•