693720 - crash [@ FramePropertyTable::DeleteEnumerator ]

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

There's ~1000 crashes [@ FramePropertyTable::DeleteEnumerator ] over the
past four week period.  It occurs in all Firefox versions back to 4.0.
All crashes are on Windows so far.
The crash reason/address indicates it might be exploitable.
The crash occurs on this line across all versions:
http://hg.mozilla.org/mozilla-central/annotate/c3a50afc2243/layout/base/nsPresShell.cpp#l1253

Example crash:
bp-a3d96c28-a4ef-456d-aac4-43f3b2111008

mozilla::FramePropertyTable::DeleteEnumerator	layout/base/FramePropertyTable.cpp:248
nsTHashtable<CategoryLeaf>::s_EnumStub	obj-firefox/dist/include/nsTHashtable.h:420
PL_DHashTableEnumerate	obj-firefox/xpcom/build/pldhash.cpp:754
PresShell::Destroy	layout/base/nsPresShell.cpp:1253
DocumentViewerImpl::DestroyPresShell	layout/base/nsDocumentViewer.cpp:4352
DocumentViewerImpl::Hide	
nsDocShell::SetVisibility	
nsSubDocumentFrame::HideViewer	layout/generic/nsSubDocumentFrame.cpp:813
nsContainerFrame::DestroyFrom	layout/generic/nsContainerFrame.cpp:270
nsBoxFrame::DestroyFrom	layout/xul/base/src/nsBoxFrame.cpp:950
nsBoxFrame::RemoveFrame	layout/xul/base/src/nsBoxFrame.cpp:1013
nsCSSFrameConstructor::ContentRemoved	layout/base/nsCSSFrameConstructor.cpp:7507
nsGenericElement::RemoveChildAt	content/base/src/nsGenericElement.cpp:3656 
...

Comment 1

[Daniel Veditz [:dveditz]]

A fix for Fx8 seems unrealistic since we don't know the cause. Any hope of figuring it out for Fx9?

Comment 2

[Johnny Stenback (:jst)]

Mats, any progress here, or any idea who should own this?

Comment 3

[Mats Palmgren (inactive)]

FWIW, the number of incidents is now ~1700 for the past four weeks (up from ~1000).

Looks hard to make progress here without a way to reproduce the crash.
I suppose we could do code review of all handling of frame properties...
It would help if we knew which frame property we're trying to destroy -
could we figure that out from a binary dump file?
(from the static FramePropertyDescriptor address)

Hmm, could it be caused by some property value destructor causing changes to the
hash table while it's being enumerated?

Comment 4

[Jet Villegas (inactive)]

Picking this up to dig up some more info with Mats next week.

Comment 5

[Jet Villegas (inactive)]

Spoke to Mats about this. He really needs an URL to repro here. It looks like we're destroying a frame property and leaving a dangling reference in this hash table. The crash dumps make it very hard to tell which property is getting trashed here, or even if it's one property or several possibles. In any case, a crashing URL will be the ticket.

Comment 6

[Marcia Knous [:marcia]]

Some URLs from the last 2 days for FramePropertyTable..DeleteEnumerator - note that there may be questionable content in some of the URLs:

2 http://www.facebook.com/logout.php
      1 http://www.youtube.com/watch?v=lt7GqO47vaw&feature=related
      1 http://www.youtube.com/watch?v=Kk_5VN_EGfE
      1 http://www.youtube.com/watch?v=g4wtii4Ikjs&feature=related
      1 http://www.youtube.com.br/
      1 http://www.visit-x.net/CAMS/GB/listing/girls_1.html?track=BoxBreadCrumble
      1 http://www.telcel.com/bat
      1 http://www.tbs.co.jp/baseball/game/20111117DH01d.htm
      1 http://www.suceursdesang.com/studi/?p=coffre
      1 http://www.slideshare.net/shatheeshl/management-by-objectives-mbo
      1 http://www.photofacefun.com/photoframes/&section_id=0&p=8
      1 http://www.nownews.com/
      1 http://www.kooora.com/default.aspx?c=7618
      1 http://www.jappy.de/
      1 http://www.heybingo.com/gamefiles/gamewin.php?Ratio=1&Room=62&BannerHeight=70&ts=1321526887610
      1 http://www.google.com/
      1 http://www.google.co.in/imghp?hl=en&tab=ii
      1 http://www.gmanews.tv/story/238934/nation/arroyos-may-now-go-abroad-as-sc-junks-doj-appeal
      1 http://www.girlsgogames.com/games/cooking_games/cooking_games.html
      1 http://www.fotka.pl/wiadomosci/talk/chester1990
      1 http://www.facebook.com/shomshomwang?sk=photos
      1 http://www.facebook.com/mohammadfayez.koush
      1 http://www.facebook.com/group.php?gid=90698944973
      1 http://www.facebook.com/bdy.a7b?sk=photos
      1 http://www.club-hd.com/2011/11/hangover-part-ii-2011-m-720p-ingles-sub.html#more
      1 http://www.climatempo.com.br/previsao-do-tempo/cidade/546/suzano-sp
      1 http://www.a7ln.com/vb/search.php?do=getnew
      1 http://wallpapers.net/nature-desktop-wallpapers/page/6
      1 http://videomb.com/index.php?mod=news&act=show&id=72
      1 http://ttvnol.com/quansu
      1 http://television.telerama.fr/tele/grillefree.php
      1 https://www.rezervuoti.lt/calendar/calendar/2011-11-21/
      1 https://www.facebook.com/zaman.kalwar?sk=friends
      1 https://www.facebook.com/logout.php
      1 http://store.ovi.com/content/67625?clickSource=browse&categoryId=16&contentArea=applications&pos=3
      1 https://mail.bayareanewsgroup.com/exchange/
      1 https://images-na.ssl-images-amazon.com/images/G/01/browser-scripts/de-site-wide-1.2.6/site-wide-7108178688.js._V164587659_.js
      1 https://easylist-downloads.adblockplus.org/easylist.txt
      1 https://apps.facebook.com/treboldelasuerte/?ref=nf
      1 http://phim.xixam.com/movie/find/phim_moi.html
      1 http://nk.pl/profile/18834757
      1 http://modareb.maktoob.com/League/TransferList
      1 http://ieeexplore.ieee.org/xpl/bkBrowse.jsp
      1 http://hard.rozetka.com.ua/cases/c80090/page=2;producer=coolermaster/
      1 http://globoesporte.globo.com/futebol/brasileirao-serie-a/
      1 http://forum.anunturi-galati.ro/index.php?/forum/116-sisteme-desktop/
      1 http://exaltasamba.uol.com.br/
      1 http://commons.wikimedia.org/wiki/File:Huguenot_canterbury.jpg
      1 http://armorgames.com/category/arcade
      2 https://www.facebook.com/?ref=tn_tnmn
      1 http://www.yugiohrpgonline.com/?perfilEdit
      1 http://www.youtube.com/watch?v=Vctuq5NuYdQ&feature=related
      1 http://www.youtube.com/watch?v=d3UsGdmj1_0
      1 http://www.youtube.com/watch?v=4jyrpdztKIg&feature=related
      1 http://www.youtube.com/results?search_query=kool+savas+aura+&oq=koll+savas+&aq=0s&aqi=g-s2g1g-s1&aql=&gs_sm=c&gs_upl=106338l108146l0l110798l11l11l0l1l1l0l225l1508l1.7.2l10l0
      1 http://www.youtube.com/results?search_query=kollegah+halt+die+fresse&oq=kollegah+halt+&aq=0&aqi=g6&aql=&gs_sm=c&gs_upl=45l5481l0l7784l11l11l2l1l1l0l296l1634l0.4.4l8l0
      1 http://www.youtube.com/results?search_query=%D9%81%D9%8A%D8%AF%D9%8A%D9%88+%D8%AA%D9%8A%D8%AA%D8%A7%D9%86%D9%8A%D9%83&oq=%D9%81%D9%8A%D8%AF%D9%8A%D9%88+%D8%AA%D9%8A&aq=0&aqi=g1&aql=&gs_sm=c&gs_upl=12283l28595l0l31307l12l12l4l2l2l0l307l1416l0.1.4.1l6l0
      1 http://www.youtube.com/my_videos?feature=mhee
      1 http://www.yahoo.com/
      1 http://www.uol.com.br/
      1 http://www.repubblica.it/politica/2011/11/16/dirette/nasce_il_governo_monti_via_libera_dalle_parti_sociali-25078460/?ref=HREA-1
      1 http://www.reddit.com/r/AskReddit/
      1 http://www.plotek.pl/plotek/56,79592,10643958,Katarzyna_Zielinska,,13.html
      1 http://www.orkut.com.br/GLogin.aspx?cmd=logout
      1 http://www.orkut.com.br/FriendsList?uid=13510241199380434838&pno=4
      1 http://www.orkut.com.br/Community?cmm=113796993
      1 http://www.orkut.com.br/Album?uid=12188488852117302012&aid=1319380180
      1 http://www.odnoklassniki.ru/cdk/st.cmd/main/tkn/641
      1 http://www.jerusalemshots.com/Jerusalem_en130-14756.html
      1 http://www.in.gr/
      1 http://www.google.it/search?q=matrox+g400+dual+head+driver+&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:it:official&client=firefox-a
      1 http://www.google.com/search?q=Finanzamt+Dauer+R%C3%BCckerstattung&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:de:official&client=firefox-a
      1 http://www.fotka.pl/konto_dane.php
      1 http://www.faces.eu/?x=fTgTeF4bAzt4FD4eE0V7flFPLPYGE4S
      1 http://www.facebook.com/profile.php?id=100000937405822
      1 http://www.facebook.com/messages/?action=read&tid=id.243048422416875
      1 http://www.facebook.com/media/set/?set=a.152445444853615.29558.100002644811312&type=3
      1 http://www.facebook.com/lduartelima1?ref=tn_tnmn
      1 http://www.facebook.com/
      1 http://www.asiandating.com/PhotoProcessingSystem/Photo_Management.cfm?
      1 http://www47.os-community.de/User/mc_der_king?show_gb=1
      1 http://winrar.softonic.it/
      1 http://webmail.clix.pt/mail/showmail.pl
      1 http://vkontakte.ru/al_apps.php?__query=apps&al=-1&al_id=18036456&_rndVer=96541
      1 http://ustadzrofii.wordpress.com/kolom-hikmah/
      1 https://www.facebook.com/?ref=logo
      1 https://www.facebook.com/profile.php?id=100000196170579&sk=wall
      1 https://www.facebook.com/EMAD.3OMDA
      1 http://sports.maktoob.com/news-442595-%D8%A8%D8%A7%D9%84%D9%81%D9%8A%D8%AF%D9%8A%D9%88_%D8%A8%D9%88%D9%85%D8%A9_%D8%AA%D8%B5%D8%B7%D8%A7%D8%AF_%D9%81%D8%A3%D8%B1%D8%A7%D9%8B_%D9%81%D9%88%D9%82_%D8%B9%D8%A7%D8%B1%D8%B6%D8%A9_%D9%85%D8%B1%D9%85%D9%89_%D9%83
      1 http://sn117w.snt117.mail.live.com/mail/InboxLight.aspx?n=899448417
      1 http://series40.kiev.ua/java_games/simulator/page/23/
      1 http://pointblank.ru/?pid=5113304
      1 http://napiszar.org/page/2/
      1 http://l.yimg.com/zz/combo?mk/ura/0.8.18/rls-min.js&mk/ura/0.8.18/ura-config-min.js&mk/ura/0.8.18/init-min.js
      1 http://lauxanh.us/diendan/showthread.php?t=607999
      1 http://jang.com.pk/jang/nov2011-daily/17-11-2011/dillagi.htm
      1 http://ja.justin.tv/directory/gaming
      1 http://im2.vkontakte.ru/im_frame.php#251
      1 http://dl.google.com/googletalk/googletalk-setup.exe
      1 http://digg.com/story/r/the_elder_scrolls_v_skyrim_review_5
      1 http://apps.facebook.com/ninja-warz/
      1 http://actu-people.staragora.com/mode-starlettes-rihanna-vanessa-hudgens.html
      1 http://192.168.1.2/kafashan/administrator/index.php

Comment 7

[Jet Villegas (inactive)]

These may well be OOM crashes. Need to keep an eye on this after 734847 lands.

Comment 8

[Jet Villegas (inactive)]

Bug 734847 landed back in May. Did it put a dent on this critsmash bug's stats?

Comment 9

[Jet Villegas (inactive)]

(In reply to Jet Villegas (:jet) from comment #8)
> Bug 734847 landed back in May. Did it put a dent on this critsmash bug's
> stats?

It appears to still be in the crash-stats, so *bump*.

Comment 10

[Mats Palmgren (inactive)]

I'll put this on my list for Q2.  I'll do some improvements in bug 729519 for
frame list properties which might help.  I'll do a review of all other types
of frame properties as well.  If nothing comes out of that I'll write some
diagnostic code so we can see which frame property is the problem.

Comment 12

[Mats Palmgren (inactive)]

There are 73 reported crash incidents over the past 4 week period.
Only one of those is for the current release version (32.0.1),
the rest are from older versions.

The crash in 32.0.1 is on OSX: bp-488998cf-f5a2-4dfd-85e5-3fe232140924
(all others are on Windows).  This one seems to involve a plug-in.

Comment 13

[Mats Palmgren (inactive)]

There are only a handful of crashes in the past 28 days with this signature.
Older builds are over-represented.  Here are *all* the ones in v39 or newer:
bp-9817505b-5a47-477a-b1f2-ec2672151014
bp-046428ca-610a-4828-92c2-f43af2151013
bp-a435dfad-22e4-44bf-abd8-0e2872151007
bp-60ec4a5b-9c0d-403b-82b6-f80842150929

All (except maybe the last which has an incomplete stack) stem from
PresShell::sPaintSuppressionCallback.  The first three are on FennecAndroid.
I think these are a different problem from the original.