See testcase, which crashes current trunk build after 100ms. It doesn't crash in a 2009-05-01 build, it does crash in a 2009-05-02 build: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-05-01+03%3A00%3A00&enddate=2009-05-02+07%3A00%3A00 No idea what could have caused it. http://crash-stats.mozilla.com/report/index/33e81e1f-7fbb-458f-a555-e37df2090505?p=1 0 xul.dll IsPercentageAware layout/generic/nsLineLayout.cpp:672 1 xul.dll xul.dll@0x8fbb0b

My http://hg.mozilla.org/mozilla-central/rev/051f635a1061, which is the last changeset before that range, looks like a likely candidate. Can you check in about:buildconfig whether the build that doesn't crash includes that changeset?

Reverting http://hg.mozilla.org/mozilla-central/rev/051f635a1061 did not remove the crash. I get a series of assertions: ###!!! ASSERTION: prev sibling not in line list: 'Not Reached', file ...layout/generic/nsBlockFrame.cpp, line 4913 ###!!! ASSERTION: unknown out of flow frame type: 'disp->mDisplay == NS_STYLE_DISPLAY_POPUP', file ...layout/generic/nsHTMLReflowState.cpp, line 518 ###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file ...gfx/thebes/src/gfxSkipChars.cpp, line 92 ###!!! ASSERTION: Text run does not map enough text for our reflow: 'gfxSkipCharsIterator(iter).ConvertOriginalToSkipped(offset + length) <= mTextRun->GetLength()', file ...layout/generic/nsTextFrameThebes.cpp, line 6107 ###!!! ASSERTION: unknown out of flow frame type: 'disp->mDisplay == NS_STYLE_DISPLAY_POPUP', file ...layout/generic/nsHTMLReflowState.cpp, line 518 ###!!! ASSERTION: shouldn't use unconstrained widths anymore: '(mFrameType == NS_CSS_FRAME_TYPE_INLINE && !frame->IsFrameOfType(nsIFrame::eReplaced)) || frame->GetType() == nsGkAtoms::textFrame || mComputedWidth != NS_UNCONSTRAINEDSIZE', file ...layout/generic/nsHTMLReflowState.cpp, line 287 ###!!! ASSERTION: null frame is not allowed: 'aFrame', file ...layout/generic/nsLineLayout.cpp, line 670

I am seeing a different regression range: 2009-04-22-03 - 2009-04-23-03. Reverting the bidi-related checkin in that range, from bug 489517, prevents the crash with this testcase, but it still happens when changing direction manually, so this looks like an older bug that bug 489517 is exposing.

Ok, I guess my regression range didn't make sense then.

I found the regression range for the older bug, and it turns out to be regression from bug 332655 (which is actually what I suspected in the first place).

This bug interferes with fuzzing by triggering a bunch of assertions before crashing, sometimes including the dreaded "StealFrame failure" assertion. As a result, I can't look for other bugs that trigger these assertions.

!exploitable output with the current 3.5.2 debug nightly (aec.9bc): Access violation - code c0000005 (!!! second chance !!!) eax=00000000 ebx=7ffdb000 ecx=00000000 edx=00000026 esi=0012ee2c edi=0012ee34 eip=01ac6952 esp=0012e034 ebp=0012e058 iopl=0 nv up ei pl zr na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000246 gklayout!IsPercentageAware+0x32: 01ac6952 8b11 mov edx,dword ptr [ecx] ds:0023:00000000=???????? 0:000> cdb: Reading initial command '!load winext\msec.dll;.logappend;!exploitable;k;q' Exploitability Classification: PROBABLY_EXPLOITABLE Recommended Bug Title: Probably Exploitable - Data from Faulting Address controls Code Flow starting at gklayout!IsPercentageAware+0x0000000000000032 (Hash=0x1d0e112e.0x416e3159) The data from the faulting address is later used as the target for a branch. ChildEBP RetAddr 0012e058 01ac59c1 gklayout!IsPercentageAware+0x32 0012e1e0 01aba585 gklayout!nsLineLayout::ReflowFrame+0x81 0012e3c0 01ac5ceb gklayout!nsFirstLetterFrame::Reflow+0x235 0012e558 01a854fe gklayout!nsLineLayout::ReflowFrame+0x3ab 0012e600 01a84f10 gklayout!nsBlockFrame::ReflowInlineFrame+0x5e 0012e6a0 01a84ad2 gklayout!nsBlockFrame::DoReflowInlineFrames+0x210 0012e79c 01a82842 gklayout!nsBlockFrame::ReflowInlineFrames+0xf2 0012e89c 01a81491 gklayout!nsBlockFrame::ReflowLine+0x2c2 0012ea5c 01a7ed71 gklayout!nsBlockFrame::ReflowDirtyLines+0x561 0012ee2c 01a56ec2 gklayout!nsBlockFrame::Reflow+0x251 0012f02c 01a5627e gklayout!nsFrame::BoxReflow+0x522 0012f110 01a56860 gklayout!nsFrame::RefreshSizeCache+0x11e 0012f128 01bb3824 gklayout!nsFrame::GetBoxAscent+0x60 0012f158 01bc6972 gklayout!nsSprocketLayout::GetAscent+0x44 0012f170 01bb134b gklayout!nsBoxFrame::GetBoxAscent+0x72 0012f308 01bc6d55 gklayout!nsSprocketLayout::Layout+0xeb 0012f328 01bb499b gklayout!nsBoxFrame::DoLayout+0x55 0012f34c 01bb612d gklayout!nsIFrame::Layout+0x6b 0012f41c 01bc6d55 gklayout!nsStackLayout::Layout+0x17d 0012f43c 01bb499b gklayout!nsBoxFrame::DoLayout+0x55 quit:

I actually see this signature come in from users sometime. yesterday was a big day. IsPercentageAware http://apps.facebook.com/buffyvampire/?page=trivia http://crash-stats.mozilla.com/report/index/67a9b2cf-c059-4595- Firefox 3.0.13 2009073022 1.9 Windows NT 5.1.2600 Service Pack 3 x86 IsPercentageAware http://www.188games.com/ContentInterface/Bet188/ContentInterfaceHome.aspx?requestMenu=Statement&s21mc=thanhvinh http://crash-stats.mozilla.com/report/index/35a198d2-a3f0-45ec-9c0d-550ed2090818 Firefox 3.5.2 20090729225027 1.9.1 Windows NT 5.1.2600 Service Pack 2 x86 IsPercentageAware http://www.xfire.com/profile/fastzolor/ http://crash-stats.mozilla.com/report/index/fa77d8aa-e5fe-4c44-a51f-fc9fa2090818 Firefox 3.5.2 Windows NT 5.1.2600 Szervizcsomag IsPercentageAware http://www.douglas.de/douglas/index_c0600.html http://crash-stats.mozilla.com/report/index/4d6c21e5-a4ac-4b0c-894b-062a12090818 Firefox 3.5.2 IsPercentageAware about:blank http://crash-stats.mozilla.com/report/index/75b40a90-2a47-4994-866a-8e7682090818 IsPercentageAware about:blank http://crash-stats.mozilla.com/report/index/e00ad601-8b64-40d0-b8bd-1a2c12090818 IsPercentageAware http://www.doit24.de/fs_start2.jsp?start_page=/artdetails.jsp%3F%26ref%3D264110-tier-zoo%26affmt%3D2%26affmn%3D4%26Artikel%3D46483517 http://crash-stats.mozilla.com/report/index/5d17de0f-1f06-4d2e- IsPercentageAware http://crash-stats.mozilla.com/report/index/4f7edbbd-22f8-42e4-ae66-e191a2090818 IsPercentageAware about:blank http://crash-stats.mozilla.com/report/index/9753a39c-9aa8-4b89-9e4e-295902090818

Comment 8 shows a null deref DoS, consistent with the last assertion in comment 2. But this is blocking further fuzz testing of this area so that adds more importance to getting this fixed.

See bug 460389 comment 19. Note also that not all the crashes with this signature are this bug. IsPercentageAware is just the first place where nsLineLayout::ReflowFrame will crash when called with null aFrame.

This prevents the crash, but leaves a bundle of assertions, so I'm not sure if it adds enough value to be worth pursuing.

First-letter frames definitely should have children. Is this just the general "first-letter and bidi reordering don't work together"? What are we going to do about this problem? I wonder if bidi-reordering could just ignore first-letter frames?

When ltr the first letter frame should contain "&m&", but when rtl it should contain just "&" (correct me if I'm wrong, I don't know how bidi works, just basing this on how a static rendering works). When we dynamically change the direction that triggers a reflow, and when we ResolveBidi we create a non-fluid continuation of the first letter frame breaking between "&" and "m&". Leaving us with two first letter frames (NS_NewFirstLetterFrame is a good place to set a breakpoint to follow this). When reflowing that continuation the block frame creates a continuation (not totally sure why) of that first letter frame, and it stays empty and that is where we crash. I don't think we should be creating continuations of first letter frames at all (?). So either we can teach Bidi resolution about first letter frames, or let the frame constructor deal with it by causing a reconstruct on containing blocks that have first letter style and "bidi stuff" (direction on the block changes, but I also imagine maybe changing some character data could do the same by mixing ltr and rtl characters?) changes on the block or its contents.

Create an ugly special case in bidi resolution for floating first letter frames. This also fixes bug 429865, bug 436982, bug 460389, and bug 477731. I discovered two other bugs while fixing this. The overflow rect of the the child of a floating first letter frame doesn't get updated properly, likely due to the half-init'ed nsLineLayout that is used to reflow it. You can see this by converting the testcase to html. Then changing the direction will create a horizontal scrollbar to handle the overflow. The other is that if you replace the "&m&m" in the testcase with a mixture of RTL and LTR characters, they won't get interleaved correctly when switching the direction from rtl to ltr and back.

Comment on attachment 408559 [details] [diff] [review] patch Not sure who needs/wants to look at this. Please adjust as necessary.

Comment on attachment 408559 [details] [diff] [review] patch Looks good to me! Simon should definitely look at it too. Thanks!!!

(In reply to comment #15) > The other is that if you replace the "&m&m" in the testcase with a mixture of > RTL and LTR characters, they won't get interleaved correctly when switching the > direction from rtl to ltr and back. That sounds bad. What does the frame tree look like in this case?

A quick look at the frame tree looks okay. But here is a testcase so you can have a look. After some investigation changing the direction dynamically wasn't needed. I think the "bc" get incorrectly placed with the "ef".

That looks as if the embedding levels of the frames aren't getting set properly.

All branches are affected.

I have a fix for attachment 409000 [details]: the embedding levels are set properly, but nsBidiPresUtils::GetFrameEmbeddingLevel is checking the embedding level of the placeholder instead of the placeholdee.

Opened bug 525740 for the reordering bug.

Not blocking, but we'll look at the patch when we get to approvals.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3cecb7409866 And the tests from the public bugs http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3f66579563a7

Comment on attachment 408559 [details] [diff] [review] patch Approved for 1.9.1.5 and 1.9.0.16, a=dveditz for release-drivers

I might be more conservative about making this bug public, although on 1.9.1+ we crash with a null deref, we are just getting lucky, the frame tree is messed up. On 1.9.0 there's even more room for things to go wrong as you have to flip the direction back and forth a few times before it crashes, so there is plenty of time to mess with a broken frame tree.

I forgot to paste this above: a push for a bustage fix http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4194a77250fb

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e58a4079e3d3 And the tests from the public bugs http://hg.mozilla.org/releases/mozilla-1.9.1/rev/cdd6547cb10b

Checking in nsBidiPresUtils.cpp; /cvsroot/mozilla/layout/base/nsBidiPresUtils.cpp,v <-- nsBidiPresUtils.cpp new revision: 1.117; previous revision: 1.116 Checking in layout/base/crashtests/503936-1.html; /cvsroot/mozilla/layout/base/crashtests/503936-1.html,v <-- 503936-1.html initial revision: 1.1 Checking in layout/base/crashtests/crashtests.list; /cvsroot/mozilla/layout/base/crashtests/crashtests.list,v <-- crashtests.list new revision: 1.103; previous revision: 1.102

Er, that second part should have been: Checking in layout/base/crashtests/429865-1.html; /cvsroot/mozilla/layout/base/crashtests/429865-1.html,v <-- 429865-1.html initial revision: 1.1 Checking in layout/base/crashtests/436982-1.html; /cvsroot/mozilla/layout/base/crashtests/436982-1.html,v <-- 436982-1.html initial revision: 1.1 Checking in layout/base/crashtests/477731-1.html; /cvsroot/mozilla/layout/base/crashtests/477731-1.html,v <-- 477731-1.html initial revision: 1.1 Checking in layout/base/crashtests/crashtests.list; /cvsroot/mozilla/layout/base/crashtests/crashtests.list,v <-- crashtests.list new revision: 1.104; previous revision: 1.103

Verified crash with attached testcase in 1.9.1.5 and lack of crash with 1.9.1.6 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.6pre) Gecko/20091119 Shiretoko/3.5.6pre (.NET CLR 3.5.30729)) so this is fixed for 1.9.1. On 1.9.0, the testcase does not cause a crash in 1.9.0.15 and I see no difference in behavior between 1.9.0.15 and .16. Does this really affect 1.9.0 directly or is this just defense in depth across all of the code lines?

I have a testcase that crashes 1.9.0 but I would prefer not to post it in a public bug. I checked that 1.9.0 nightlies no longer crash on that testcase.

That's good enough for me, Timothy. I'm marking this verified for 1.9.0.16.

Timothy, please be sure to attach it to this bug after we ship a release with this fix in it. (Alternatively, send it to someone in the security-group who can attach it as a private attachment.)

Landed the testcase of this bug and the testcase mentioned in comment 34 on mozilla-central http://hg.mozilla.org/mozilla-central/rev/365fbdfea8a2