18316 - [CRASH] SEGV adjusting frame widths, while -> for

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[Mo DeJong]

I was surfing around with a CVS build from Tue Nov 9 when I got
this SIGSEV. My system is a RedHat 5.2 Linux box.


layout/html/document/src/nsFrameSetFrame.cpp line 292


287       if ((aNumIndicies > 0) && (aDesired != actual)) {
288         PRInt32 unit = (aDesired > actual) ? 1 : -1;
289         i = 0;
290         while (aDesired != actual) {
291           j = aIndicies[i];
292           aItems[j] += unit;
293           actual += unit;
294           i++;
295         }
296       }


(gdb) print i
$3 = 1
(gdb) print j
$4 = 1076802600

So it looks like the statement

aItems[j] += unit;

creates a SIGSEV because the j value (1076802600) is way off the
end of the pointer aItems.



#0  0x40e4ca59 in nsHTMLFramesetFrame::Scale (this=0x98b84a8,
    aDesired=1073740594, aNumIndicies=1, aIndicies=0x9a03a38, aItems=0x92bef18)
    at ../../../../../layout/html/document/src/nsFrameSetFrame.cpp:292
#1  0x40e4ce6e in nsHTMLFramesetFrame::CalculateRowCol (this=0x98b84a8,
    aPresContext=0x9951cd0, aSize=1073741824, aNumSpecs=2, aSpecs=0x92beef0,
    aValues=0x92bef18)
    at ../../../../../layout/html/document/src/nsFrameSetFrame.cpp:381
#2  0x40e4ebda in nsHTMLFramesetFrame::Reflow (this=0x98b84a8,
    aPresContext=@0x9951cd0, aDesiredSize=@0xbfffdd44,
    aReflowState=@0xbfffdd74, aStatus=@0xbfffde88)
    at ../../../../../layout/html/document/src/nsFrameSetFrame.cpp:963
#3  0x40d82e49 in nsLineLayout::ReflowFrame (this=0xbfffdf20,
    aFrame=0x98b84a8, aNextRCFrame=0xbfffea9c, aReflowStatus=@0xbfffde88,
    aMetrics=0x0) at ../../../../../layout/html/base/src/nsLineLayout.cpp:951
#4  0x40d54727 in nsBlockFrame::ReflowInlineFrame (this=0x928bc28,
    aState=@0xbfffea0c, aLineLayout=@0xbfffdf20, aLine=0x98b8580,
    aFrame=0x98b84a8, aLineReflowStatus=0xbfffded7 "")
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:3653
#5  0x40d54442 in nsBlockFrame::DoReflowInlineFrames (this=0x928bc28,
    aState=@0xbfffea0c, aLineLayout=@0xbfffdf20, aLine=0x98b8580,
    aKeepReflowGoing=0xbfffe7f0, aLineReflowStatus=0xbfffe6f3 "\002")
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:3544
#6  0x40d54277 in nsBlockFrame::DoReflowInlineFramesAuto (this=0x928bc28,
    aState=@0xbfffea0c, aLine=0x98b8580, aKeepReflowGoing=0xbfffe7f0,
    aLineReflowStatus=0xbfffe6f3 "\002")
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:3489
#7  0x40d54072 in nsBlockFrame::ReflowInlineFrames (this=0x928bc28,
    aState=@0xbfffea0c, aLine=0x98b8580, aKeepReflowGoing=0xbfffe7f0)
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:3438
#8  0x40d528d1 in nsBlockFrame::ReflowLine (this=0x928bc28,
    aState=@0xbfffea0c, aLine=0x98b8580, aKeepReflowGoing=0xbfffe7f0,
    aDamageDirtyArea=0)
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:2665
#9  0x40d51f86 in nsBlockFrame::ReflowDirtyLines (this=0x928bc28,
    aState=@0xbfffea0c)
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:2425
#10 0x40d504fc in nsBlockFrame::Reflow (this=0x928bc28,
    aPresContext=@0x9951cd0, aMetrics=@0xbfffed60, aReflowState=@0xbfffecc0,
    aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsBlockFrame.cpp:1489
#11 0x40d4cee6 in nsAreaFrame::Reflow (this=0x928bc28,
    aPresContext=@0x9951cd0, aDesiredSize=@0xbfffed60,
    aReflowState=@0xbfffecc0, aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsAreaFrame.cpp:289
#12 0x40d5d683 in nsContainerFrame::ReflowChild (this=0x8f7cdd0,
    aKidFrame=0x928bc28, aPresContext=@0x9951cd0, aDesiredSize=@0xbfffed60,
    aReflowState=@0xbfffecc0, aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsContainerFrame.cpp:405
#13 0x40d73322 in RootFrame::Reflow (this=0x8f7cdd0, aPresContext=@0x9951cd0,
    aDesiredSize=@0xbfffeeb4, aReflowState=@0xbfffee0c, aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsHTMLFrame.cpp:327
#14 0x40d5d683 in nsContainerFrame::ReflowChild (this=0x903c528,
    aKidFrame=0x8f7cdd0, aPresContext=@0x9951cd0, aDesiredSize=@0xbfffeeb4,
    aReflowState=@0xbfffee0c, aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsContainerFrame.cpp:405
#15 0x40da3223 in ViewportFrame::Reflow (this=0x903c528,
    aPresContext=@0x9951cd0, aDesiredSize=@0xbffff024,
    aReflowState=@0xbfffef80, aStatus=@0xbffff054)
    at ../../../../../layout/html/base/src/nsViewportFrame.cpp:512
#16 0x40d8dfc2 in PresShell::ResizeReflow (this=0x99520b0, aWidth=1073741824,
    aHeight=1073741824)
    at ../../../../../layout/html/base/src/nsPresShell.cpp:1046
#17 0x408e8f9b in nsWebShell::SizeToContent (this=0x9870178)
    at ../../../webshell/src/nsWebShell.cpp:1256
#18 0x40335aaf in nsWebShellWindow::OnEndDocumentLoad (this=0x917b630,
    loader=0x91b6978, channel=0x8767ca0, aStatus=0, aDocObserver=0x956c744)
    at ../../../../xpfe/appshell/src/nsWebShellWindow.cpp:2019
#19 0x408eee2f in nsWebShell::OnEndDocumentLoad (this=0x956c730,
    loader=0x91b6978, channel=0x8767ca0, aStatus=0, aWebShell=0x956c744)
    at ../../../webshell/src/nsWebShell.cpp:3400
#20 0x408e4cb3 in nsDocLoaderImpl::FireOnEndDocumentLoad (this=0x91b6978,
    aLoadInitiator=0x91b6978, aDocChannel=0x8767ca0, aStatus=0)
    at ../../../webshell/src/nsDocLoader.cpp:866
#21 0x408e484f in nsDocLoaderImpl::OnStopRequest (this=0x91b6978,
    channel=0x8767ca0, ctxt=0x0, status=0, errorMsg=0x0)
    at ../../../webshell/src/nsDocLoader.cpp:748
#22 0x4088f861 in nsLoadGroup::RemoveChannel (this=0x91b69c0,
    channel=0x8767ca0, ctxt=0x0, status=0, errorMsg=0x0)
    at ../../../../netwerk/base/src/nsLoadGroup.cpp:600
#23 0x4129efda in nsHTTPChannel::ResponseCompleted (this=0x8767ca0,
    aTransport=0x8774520, aStatus=0, aMsg=0x0)
    at ../../../../../netwerk/protocol/http/src/nsHTTPChannel.cpp:804
#24 0x412a1f09 in nsHTTPResponseListener::OnStopRequest (this=0x9bc3be0,
    channel=0x8774520, i_pContext=0x8767ca0, i_Status=0, i_pMsg=0x0)
    at ../../../../../netwerk/protocol/http/src/nsHTTPResponseListener.cpp:239
#25 0x4087c3f2 in nsOnStopRequestEvent::HandleEvent (this=0x41310800)
    at ../../../../netwerk/base/src/nsAsyncStreamListener.cpp:325
#26 0x4087bc52 in nsStreamListenerEvent::HandlePLEvent (aEvent=0x413125e0)
    at ../../../../netwerk/base/src/nsAsyncStreamListener.cpp:173
#27 0x4018a2fb in PL_HandleEvent (self=0x413125e0) at plevent.c:537
#28 0x4018a20d in PL_ProcessPendingEvents (self=0x80a0310) at plevent.c:498
#29 0x4014bc9d in nsEventQueueImpl::ProcessPendingEvents (this=0x80a02e8)
    at ../../../xpcom/threads/nsEventQueue.cpp:193
#30 0x404a211c in event_processor_callback (data=0x80a02e8, source=8,
    condition=GDK_INPUT_READ) at ../../../../widget/src/gtk/nsAppShell.cpp:232
#31 0x404a1a23 in our_gdk_io_invoke (source=0x81926a8, condition=G_IO_IN,
    data=0x81fd610) at ../../../../widget/src/gtk/nsAppShell.cpp:53
#32 0x4063272e in g_io_unix_dispatch (source_data=0x81926c0,
    current_time=0xbffff620, user_data=0x81fd610) at giounix.c:135
#33 0x40633c8f in g_main_dispatch (current_time=0xbffff620) at gmain.c:652
#34 0x40634277 in g_main_iterate (block=1, dispatch=1) at gmain.c:870
#35 0x406343f9 in g_main_run (loop=0x81b1228) at gmain.c:928
#36 0x40564edf in gtk_main () at gtkmain.c:475
#37 0x404a262f in nsAppShell::Run (this=0x80a2140)
    at ../../../../widget/src/gtk/nsAppShell.cpp:399
#38 0x4032d7a5 in nsAppShellService::Run (this=0x809ff98)
    at ../../../../xpfe/appshell/src/nsAppShellService.cpp:483
#39 0x804c850 in main1 (argc=1, argv=0xbffff834)
    at ../../../xpfe/bootstrap/nsAppRunner.cpp:580
#40 0x804cadd in main (argc=1, argv=0xbffff834)
    at ../../../xpfe/bootstrap/nsAppRunner.cpp:670

Comment 1

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

Do you have any idea what you did to cause the crash?  Without steps to
reproduce, this may be impossible to fix.

Moving to HTMLFrames anyway...

Comment 2

[karnaze (gone)]

Reassigning to Eric.

Comment 3

[leger]

Resetting QA contact from leger.

Comment 4

[Eric Pollmann]

This seems very odd...  We must have come in with some really wacky conditions
to cause this algorithm to fail.  I'll change the while into a for so that we
will never go beyond the end of the array just as a safety precaution.
Estimating M13

Comment 5

[ekrock's old account (dead)]

Marking [CRASH].

Comment 6

[ekrock's old account (dead)]

Marking [HAVE FIX].

Comment 7

[Eric Pollmann]

Just checked in two fixes.  These are code level fixes and can not be verified
by running the program (unless you were able to duplicate the original bug.)  My
two fixes were to ensure that:

1) The aIndicies array is not read beyond its bounds by using a for loop instead
of a while loop
2) The aItems array is not written beyond its bounds by checking the returned
value from aIndicies (this caused the original crash).

Fixes changes are here:
http://cvs-mirror.mozilla.org/webtools/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/html/document/src&command=DIFF_FRAMESET&file=nsFrameSetFrame.h&rev1=3.16&rev2=3.17&root=/cvsroot

Thanks!

Comment 8

[Chris Petersen]

Marking verified per last comments.