Last Comment Bug 750066 - (CVE-2012-1941) Out of bounds read in nsHTMLReflowState::CalculateHypotheticalBox, with nested multi-column, relative position, and absolute position
(CVE-2012-1941)
: Out of bounds read in nsHTMLReflowState::CalculateHypotheticalBox, with neste...
Status: VERIFIED FIXED
[asan][sg:critical][advisory-tracking...
: crash, sec-critical, testcase
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: unspecified
: All All
: -- critical (vote)
: mozilla15
Assigned To: Mats Palmgren (:mats)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-29 07:31 PDT by Abhishek Arya
Modified: 2014-06-27 14:51 PDT (History)
11 users (show)
abillings: sec‑bounty+
mats: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
fixed
+
fixed
-
fixed
13+
fixed


Attachments
Load testcase and resize browser window to reproduce (628 bytes, text/html)
2012-04-29 07:31 PDT, Abhishek Arya
no flags Details
fix for Fx13 and later (13.02 KB, patch)
2012-04-30 15:47 PDT, Mats Palmgren (:mats)
roc: review+
Details | Diff | Review
crashtest (2.31 KB, patch)
2012-04-30 15:48 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review
fix for Fx13 and later, v2 (12.90 KB, patch)
2012-04-30 17:21 PDT, Mats Palmgren (:mats)
roc: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review
for esr10, if wanted (12.37 KB, patch)
2012-04-30 18:13 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review

Description Abhishek Arya 2012-04-29 07:31:09 PDT
Created attachment 619397 [details]
Load testcase and resize browser window to reproduce

Reproduces on ASANified aurora, trunk. Load attached testcase and resize browser window to reproduce crash.


=================================================================
==27281== ERROR: AddressSanitizer heap-buffer-overflow on address 0x7f188f6581b4 at pc 0x7f18b95a4375 bp 0x7fffcea2e310 sp 0x7fffcea2e308
READ of size 4 at 0x7f188f6581b4 thread T0
    #0 0x7f18b95a4375 in nsLineBox::Contains(nsIFrame*) const firefox/aurora/layout/generic/nsLineBox.h:547
    #1 0x7f18b95cb325 in nsBlockInFlowLineIterator firefox/aurora/layout/generic/nsBlockFrame.cpp:5261
    #2 0x7f18b983b03f in nsHTMLReflowState::CalculateHypotheticalBox(nsPresContext*, nsIFrame*, nsIFrame*, int, int, nsHTMLReflowState const*, nsHypotheticalBox&, nsIAtom*) firefox/aurora/layout/generic/nsHTMLReflowState.cpp:987
    #3 0x7f18b984058e in nsHTMLReflowState::InitAbsoluteConstraints(nsPresContext*, nsHTMLReflowState const*, int, int, nsIAtom*) firefox/aurora/layout/generic/nsHTMLReflowState.cpp:1186
    #4 0x7f18b9831ec4 in nsHTMLReflowState::InitConstraints(nsPresContext*, int, int, nsMargin const*, nsMargin const*, nsIAtom*) firefox/aurora/layout/generic/nsHTMLReflowState.cpp:1878
    #5 0x7f18b98280af in nsHTMLReflowState::Init(nsPresContext*, int, int, nsMargin const*, nsMargin const*) firefox/aurora/layout/generic/nsHTMLReflowState.cpp:291
    #6 0x7f18b9829d49 in nsHTMLReflowState firefox/aurora/layout/generic/nsHTMLReflowState.cpp:180
    #7 0x7f18b954974b in nsAbsoluteContainingBlock::ReflowAbsoluteFrame(nsIFrame*, nsPresContext*, nsHTMLReflowState const&, int, int, bool, nsIFrame*, unsigned int&, nsOverflowAreas*) firefox/aurora/layout/generic/nsAbsoluteContainingBlock.cpp:424
    #8 0x7f18b9546006 in nsAbsoluteContainingBlock::Reflow(nsContainerFrame*, nsPresContext*, nsHTMLReflowState const&, unsigned int&, int, int, bool, bool, bool, nsOverflowAreas*) firefox/aurora/layout/generic/nsAbsoluteContainingBlock.cpp:158
    #9 0x7f18b95617bb in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1207
    #10 0x7f18b95f1b7b in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockReflowContext.cpp:295
    #11 0x7f18b9591a4f in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:3202
    #12 0x7f18b95872b6 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:2511
    #13 0x7f18b956c871 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockFrame.cpp:2022
    #14 0x7f18b955ffcf in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1071
    #15 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #16 0x7f18b9634445 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:710
    #17 0x7f18b963b6c7 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:965
    #18 0x7f18b95f1b7b in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockReflowContext.cpp:295
    #19 0x7f18b9591a4f in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:3202
    #20 0x7f18b95872b6 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:2511
    #21 0x7f18b956c871 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockFrame.cpp:2022
    #22 0x7f18b955ffcf in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1071
    #23 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #24 0x7f18b9634445 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:710
    #25 0x7f18b963b6c7 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:965
    #26 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #27 0x7f18b981e4ec in nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsCanvasFrame.cpp:461
    #28 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #29 0x7f18b979c723 in nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) firefox/aurora/layout/generic/nsGfxScrollFrame.cpp:559
    #30 0x7f18b97a21ba in nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) firefox/aurora/layout/generic/nsGfxScrollFrame.cpp:659
    #31 0x7f18b97a652f in nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsGfxScrollFrame.cpp:900
    #32 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #33 0x7f18b9b72706 in ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsViewportFrame.cpp:230
    #34 0x7f18b92dcb59 in PresShell::DoReflow(nsIFrame*, bool) firefox/aurora/layout/base/nsPresShell.cpp:7608
    #35 0x7f18b92d941f in PresShell::ResizeReflowIgnoreOverride(int, int) firefox/aurora/layout/base/nsPresShell.cpp:2036
    #36 0x7f18b92da50a in PresShell::ResizeReflow(int, int) firefox/aurora/layout/base/nsPresShell.cpp:1971
    #37 0x7f18bc8b1f4a in nsViewManager::DoSetWindowDimensions(int, int) firefox/aurora/view/src/nsViewManager.cpp:243
    #38 0x7f18bc8b28ed in nsViewManager::SetWindowDimensions(int, int) firefox/aurora/view/src/nsViewManager.cpp:263
    #39 0x7f18b912d7f1 in DocumentViewerImpl::SetBounds(nsIntRect const&) firefox/aurora/layout/base/nsDocumentViewer.cpp:1928
    #40 0x7f18c01e6cec in nsDocShell::SetPositionAndSize(int, int, int, int, bool) firefox/aurora/docshell/base/nsDocShell.cpp:4739
    #41 0x7f18c01e73a7 in non-virtual thunk to nsDocShell::SetPositionAndSize(int, int, int, int, bool) firefox/aurora/modules/zlib/src/gzlib.c:0
    #42 0x7f18bac83ba1 in nsFrameLoader::UpdateBaseWindowPositionAndSize(nsIFrame*) firefox/aurora/content/base/src/nsFrameLoader.cpp:1699
    #43 0x7f18bac8310e in nsFrameLoader::UpdatePositionAndSize(nsIFrame*) firefox/aurora/content/base/src/nsFrameLoader.cpp:1672
    #44 0x7f18b9a25d35 in nsSubDocumentFrame::ReflowFinished() firefox/aurora/layout/generic/nsSubDocumentFrame.cpp:648
    #45 0x7f18b9a25ebc in non-virtual thunk to nsSubDocumentFrame::ReflowFinished() firefox/aurora/modules/zlib/src/gzlib.c:0
    #46 0x7f18b9307109 in PresShell::HandlePostedReflowCallbacks(bool) firefox/aurora/layout/base/nsPresShell.cpp:3840
    #47 0x7f18b92de439 in PresShell::DidDoReflow(bool) firefox/aurora/layout/base/nsPresShell.cpp:7448
    #48 0x7f18b92d9452 in PresShell::ResizeReflowIgnoreOverride(int, int) firefox/aurora/layout/base/nsPresShell.cpp:2040
    #49 0x7f18b92da50a in PresShell::ResizeReflow(int, int) firefox/aurora/layout/base/nsPresShell.cpp:1971
    #50 0x7f18bc8b1f4a in nsViewManager::DoSetWindowDimensions(int, int) firefox/aurora/view/src/nsViewManager.cpp:243
    #51 0x7f18bc8b28ed in nsViewManager::SetWindowDimensions(int, int) firefox/aurora/view/src/nsViewManager.cpp:263
    #52 0x7f18bc8bb81d in nsViewManager::DispatchEvent(nsGUIEvent*, nsIView*, nsEventStatus*) firefox/aurora/view/src/nsViewManager.cpp:680
    #53 0x7f18bc8a5164 in HandleEvent(nsGUIEvent*) firefox/aurora/view/src/nsView.cpp:159
    #54 0x7f18c19d195a in nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) firefox/aurora/widget/gtk2/nsWindow.cpp:523
    #55 0x7f18c19d0cda in nsWindow::DispatchResizeEvent(nsIntRect&, nsEventStatus&) firefox/aurora/widget/gtk2/nsWindow.cpp:481
    #56 0x7f18c19e30c3 in nsWindow::Resize(int, int, int, int, bool) firefox/aurora/widget/gtk2/nsWindow.cpp:1178
    #57 0x7f18b912d58b in DocumentViewerImpl::SetBounds(nsIntRect const&) firefox/aurora/layout/base/nsDocumentViewer.cpp:1921
    #58 0x7f18c01e6cec in nsDocShell::SetPositionAndSize(int, int, int, int, bool) firefox/aurora/docshell/base/nsDocShell.cpp:4739
    #59 0x7f18c01e73a7 in non-virtual thunk to nsDocShell::SetPositionAndSize(int, int, int, int, bool) firefox/aurora/modules/zlib/src/gzlib.c:0
    #60 0x7f18c0703e4b in nsWebShellWindow::HandleEvent(nsGUIEvent*) firefox/aurora/xpfe/appshell/src/nsWebShellWindow.cpp:347
    #61 0x7f18c19d195a in nsWindow::DispatchEvent(nsGUIEvent*, nsEventStatus&) firefox/aurora/widget/gtk2/nsWindow.cpp:523
    #62 0x7f18c19d0cda in nsWindow::DispatchResizeEvent(nsIntRect&, nsEventStatus&) firefox/aurora/widget/gtk2/nsWindow.cpp:481
    #63 0x7f18c19fdd4b in nsWindow::OnSizeAllocate(_GtkWidget*, _GdkRectangle*) firefox/aurora/widget/gtk2/nsWindow.cpp:2423
0x7f188f6581b4 is located 20 bytes to the right of 32-byte region [0x7f188f658180,0x7f188f6581a0)
allocated by thread T0 here:
    #0 0x4a4332 in malloc ??:0
    #1 0x7f18cf2747c7 in moz_xmalloc firefox/aurora/memory/mozalloc/mozalloc.cpp:87
    #2 0x7f18b95a26fb in nsBlockFrame::PushLines(nsBlockReflowState&, nsLineList_iterator) firefox/aurora/layout/generic/nsBlockFrame.cpp:4410
    #3 0x7f18b95b1d2c in nsBlockFrame::PlaceLine(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFloatManager::SavedState*, nsRect&, int&, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:4321
    #4 0x7f18b95a837e in nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) firefox/aurora/layout/generic/nsBlockFrame.cpp:3760
    #5 0x7f18b9598f47 in nsBlockFrame::ReflowInlineFrames(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:3478
    #6 0x7f18b958775c in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:2567
    #7 0x7f18b956fbea in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockFrame.cpp:2307
    #8 0x7f18b955ffcf in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1071
    #9 0x7f18b95f1b7b in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockReflowContext.cpp:295
    #10 0x7f18b9591a4f in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:3202
    #11 0x7f18b95872b6 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:2511
    #12 0x7f18b956c871 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockFrame.cpp:2022
    #13 0x7f18b955ffcf in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1071
    #14 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
    #15 0x7f18b9634445 in nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, nsColumnSetFrame::ReflowConfig const&, bool, nsCollapsingMargin*, nsColumnSetFrame::ColumnBalanceData&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:710
    #16 0x7f18b963b6c7 in nsColumnSetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsColumnSetFrame.cpp:965
    #17 0x7f18b95f1b7b in nsBlockReflowContext::ReflowBlock(nsRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockReflowContext.cpp:295
    #18 0x7f18b9591a4f in nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:3202
    #19 0x7f18b95872b6 in nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) firefox/aurora/layout/generic/nsBlockFrame.cpp:2511
    #20 0x7f18b956c871 in nsBlockFrame::ReflowDirtyLines(nsBlockReflowState&) firefox/aurora/layout/generic/nsBlockFrame.cpp:2022
    #21 0x7f18b955ffcf in nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) firefox/aurora/layout/generic/nsBlockFrame.cpp:1071
    #22 0x7f18b9652fa7 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) firefox/aurora/layout/generic/nsContainerFrame.cpp:940
==27281== ABORTING
Stats: 140M malloced (154M for red zones) by 338742 calls
Stats: 42M realloced by 19009 calls
Stats: 118M freed by 232563 calls
Stats: 0M really freed by 0 calls
Stats: 328M (84015 full pages) mmaped in 82 calls
  mmaps   by size class: 8:278511; 9:49146; 10:16380; 11:16376; 12:3072; 13:2048; 14:1536; 15:384; 16:576; 17:128; 18:112; 19:56; 20:16;
  mallocs by size class: 8:255876; 9:45163; 10:15510; 11:15570; 12:2378; 13:1737; 14:1416; 15:299; 16:521; 17:108; 18:100; 19:50; 20:14;
  frees   by size class: 8:164067; 9:36673; 10:13283; 11:13080; 12:1703; 13:1568; 14:1236; 15:256; 16:456; 17:95; 18:89; 19:46; 20:11;
  rfrees  by size class:
Stats: malloc large: 272 small slow: 1772
Shadow byte and word:
  0x1fe311ecb036: fb
  0x1fe311ecb030: 00 00 00 00 fb fb fb fb
More shadow bytes:
  0x1fe311ecb010: 00 00 00 00 00 00 00 fb
  0x1fe311ecb018: fb fb fb fb fb fb fb fb
  0x1fe311ecb020: fa fa fa fa fa fa fa fa
  0x1fe311ecb028: fa fa fa fa fa fa fa fa
=>0x1fe311ecb030: 00 00 00 00 fb fb fb fb
  0x1fe311ecb038: fb fb fb fb fb fb fb fb
  0x1fe311ecb040: fa fa fa fa fa fa fa fa
  0x1fe311ecb048: fa fa fa fa fa fa fa fa
  0x1fe311ecb050: fa fa fa fa fa fa fa fa
Comment 1 Mats Palmgren (:mats) 2012-04-30 09:00:34 PDT
A frame has a stale LineCursorProperty after that line was pulled up...
Comment 2 Mats Palmgren (:mats) 2012-04-30 15:47:44 PDT
Created attachment 619727 [details] [diff] [review]
fix for Fx13 and later

(Fx12 and older has nsBlockReflowState::FreeLineBox which we later moved
to nsBlockFrame, so I'll prepare a separate patch for those branches.)

Notes:
This patch adds ClearLineCursor() calls when we delete a line or steal
a line from a different block.  I'm leaving the current ClearLineCursor()
calls as is for now, but some of them can probably be removed, e.g. the
unconditional ClearLineCursor() call at the start of DoRemoveFrame() is
just in case a line *might* be deleted in that method. We can remove
those which we think are redundant on trunk in a follow-up  patch, but
I think it's less risky to just leave them as is on branches.

There's a bug in (DEBUG-only) nsBlockFrame::VerifyOverflowSituation().
It verifies the OverflowLines (if any) of the first-in-flow only, not on
all the in-flow frames as it should.  Also, the comment "its never possible
for multiple frames to have overflow lists" is bogus -- adding an assert
for it failed horribly (due to interruptable reflow perhaps?).  Anyway,
I'm pretty sure all code already deals with this properly.

https://tbpl.mozilla.org/?tree=Try&rev=74c744268372
Comment 3 Mats Palmgren (:mats) 2012-04-30 15:48:16 PDT
Created attachment 619728 [details] [diff] [review]
crashtest
Comment 4 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-04-30 16:37:10 PDT
Comment on attachment 619727 [details] [diff] [review]
fix for Fx13 and later

Review of attachment 619727 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsBlockFrame.cpp
@@ +5233,5 @@
>      return;
>  
>    // Try to use the cursor if it exists, otherwise fall back to the first line
>    nsLineBox* cursor = static_cast<nsLineBox*>
> +    (aFrame->Properties().Get(nsBlockFrame::LineCursorProperty()));

Why aren't we calling aFrame->GetLineCursor() here?
Comment 5 Mats Palmgren (:mats) 2012-04-30 17:21:19 PDT
Created attachment 619780 [details] [diff] [review]
fix for Fx13 and later, v2

Use the GetLineCursor() accessor in three more places.
Comment 6 Mats Palmgren (:mats) 2012-04-30 18:13:40 PDT
Created attachment 619802 [details] [diff] [review]
for esr10, if wanted

This patch requires part 1 in bug 730769.
Comment 7 Mats Palmgren (:mats) 2012-04-30 18:19:35 PDT
I'm guessing [sg:critical] since use-after-free of nsLineBox* have shown signs
of being exploitable in the past.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-04-30 19:22:27 PDT
Comment on attachment 619780 [details] [diff] [review]
fix for Fx13 and later, v2

Review of attachment 619780 [details] [diff] [review]:
-----------------------------------------------------------------

Can we given nsLineBoxes their own presshell arena freelist and poison them? I think we should.
Comment 9 Mats Palmgren (:mats) 2012-05-01 08:50:31 PDT
Yeah, we can easily do that by using the same underlying code as frame allocation use.
I filed bug 750745 for it.
Comment 10 Alex Keybl [:akeybl] 2012-05-03 16:43:39 PDT
(In reply to Mats Palmgren [:mats] from comment #2)
> There's a bug in (DEBUG-only) nsBlockFrame::VerifyOverflowSituation().
> It verifies the OverflowLines (if any) of the first-in-flow only, not on
> all the in-flow frames as it should.  Also, the comment "its never possible
> for multiple frames to have overflow lists" is bogus -- adding an assert
> for it failed horribly (due to interruptable reflow perhaps?).  Anyway,
> I'm pretty sure all code already deals with this properly.

Is this an sg:crit bug only on debug builds?
Comment 11 Mats Palmgren (:mats) 2012-05-03 16:50:27 PDT
No, I just wanted to explain the changes I did in VerifyOverflowSituation()
which is DEBUG-only.  Those changes are unrelated to the crash-fix, but fixing/
extending the checks in VerifyOverflowSituation() might help detect similar
badness in the future.
Comment 13 Ed Morley [:emorley] 2012-05-04 10:55:20 PDT
https://hg.mozilla.org/mozilla-central/rev/38200378d87e
Comment 14 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-07 10:44:28 PDT
It's been on central a couple of days now and no one's reported any issues so please go ahead and nominate for Aurora/Beta.
Comment 15 Mats Palmgren (:mats) 2012-05-07 11:29:50 PDT
Comment on attachment 619780 [details] [diff] [review]
fix for Fx13 and later, v2

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: sg:crit crash
Testing completed (on m-c, etc.): on m-c since 2012-05-04
Risk to taking this patch (and alternatives if risky): low risk - the line
cursor is used for improving performance; if there is an error in the patch,
clearing the cursor could make some operations slower (the patch aims to
only clear the cursor if the line it points to is being removed or
moved to a different frame).  For normal sites the line cursor isn't an
important performance factor though, it only matters for degenerated cases
like dynamically adding/removing lines in a block with many thousands of lines.
String changes made by this patch: none
Comment 16 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-09 15:41:52 PDT
Comment on attachment 619780 [details] [diff] [review]
fix for Fx13 and later, v2

low risk and sg:crit - let's get this on beta/aurora.
Comment 18 Lukas Blakk [:lsblakk] use ?needinfo 2012-05-10 16:14:55 PDT
Approved, also tracked and approved the part 1 patch in bug 730769
Comment 20 Al Billings [:abillings] 2012-05-14 14:00:54 PDT
Verified fixed in 5/10 ASAN OS X build with testcase.
Comment 21 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-05-23 21:02:04 PDT
Is it possible to verify this for Beta/ESR without an ASAN build? I'm a little worried that we seem to have a lot of these ASAN bugs lately which cannot be verified on builds we are releasing.
Comment 22 Huzaifa Sidhpurwala 2012-05-31 22:02:29 PDT
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #21)
> Is it possible to verify this for Beta/ESR without an ASAN build? I'm a
> little worried that we seem to have a lot of these ASAN bugs lately which
> cannot be verified on builds we are releasing.

+1, Same for distros who ship mozilla after running their own QE, ASAN builds make QE difficult.
Comment 23 Al Billings [:abillings] 2012-05-31 23:03:01 PDT
Building on Linux is pretty easy though. It is the preferred means.

https://developer.mozilla.org/en/Building_Firefox_with_Address_Sanitizer
Comment 24 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-03 10:50:20 PDT
Given we can't make Beta/ESR ASAN builds at this time, QA is untracking. Please re-add [qa+] if this changes at some point in the future.
Comment 25 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-03 10:51:30 PDT
Follow-up to comment 24. QA is putting faith in the verification of mozilla-central ASAN builds due to technical constraints.
Comment 26 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-21 15:01:43 PDT
qa- as per comment 25
Comment 27 Al Billings [:abillings] 2013-01-14 15:09:47 PST
Created attachment 702017 [details]
inferno@chromium.org,3000,2012-04-29,2013-03-11,2012-05-04,true
Comment 28 Mats Palmgren (:mats) 2013-05-14 06:58:49 PDT
Crash test:
https://hg.mozilla.org/integration/mozilla-inbound/rev/adc89f858730
Comment 29 Ryan VanderMeulen [:RyanVM] 2013-05-14 13:29:37 PDT
https://hg.mozilla.org/mozilla-central/rev/adc89f858730

Note You need to log in before you can comment on or make changes to this bug.