276261 - Crash: malformed embed causes strange behaviour [@ nsCommonWidget::Show]

Status: RESOLVED WONTFIX

(Core :: Widget: Gtk, defect)

Description

[Tavis Ormandy]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041227 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041227 Firefox/1.0+

testcase attached causes odd behaviour on linux (gtk2).

WARNING: x server *may* die with testcase.

Reproducible: Sometimes

Steps to Reproduce:
1. view testcase
2. it may crash or demonstrate the odd behaviour, if not, continue.
3. start firefox with -ProfileManager argument and create new profile, goto 1.

Actual Results:  
First time, firefox made some odd X Requests as the window jumped around, then
immediately died complaining about X returning BadAlloc. After that, the same
thing seems to happen inconsistently, sometimes 2 javascript consoles appear
with no text. 
Creating a new profile restores the Crash behaviour.

I asked a friend (linux, firefox 1.0 gtk2, mozilla.org build) to test the
testcase for me, he reports his X Server died the first time, then just the two
consoles appear.

Comment 1

[Tavis Ormandy]

Attachment: testcase

Comment 2

[Adam Hauner]

Taviso: Could you provide Talkback incident Id of your crash?

WFM 2004122906/Seamonkey-trunk/W2K, FF1.0/W2K

Comment 3

[Tavis Ormandy]

qfa isn't called, perhaps because it dies inside gtk.

I don't know the internals very well, but I've tried stepping through it inside 
gdb, I can see that a new window is requested with an insanely huge Width and 
Height, there's a sanity check before it's passed to the toolkit, but it only 
checks it's > 0 (perhaps the same thing happens on windows, but windows has a 
built in sanity check or size limits? it seems gtk honours the requested size 
resulting in craziness).

the requested size seems to be consistent (which, i suppose would suggest non-
initialised values), perhaps some kind of overflow?

Here are some of the steps I've seen in gdb (output trimmed for brevity):

Program received signal SIGINT, Interrupt.
(gdb) break nsCommonWidget.cpp:234
Breakpoint 1 at 0x40f125ab: file nsCommonWidget.cpp, line 234.
(gdb) condition 1 this->mBounds.width > 1000
(gdb) cont
<view testcase>
Breakpoint 1, nsCommonWidget::Show(int) (this=0x877fc68, aState=1) at 
nsCommonWidget.cpp:234
(gdb) print this->mBounds
$4 = {x = 0, y = 0, width = 71582793, height = 71582792}

That seems insane?

Then it runs a sanity check:
http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsCommonWidget.cpp#236

but the sanity check only checks the dimensions are positive:

http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsCommonWidget.cpp#454

which this obviously easily qualifies for :-)

then after some stepping
http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsWindow.cpp#2412

this line

looks like this:

(gdb) print aHeight
$17 = 71582792
(gdb) print aWidth
$18 = 71582793

I double checked this is supposed to be pixels, which it is

http://developer.gnome.org/doc/API/2.0/gtk/GtkWindow.html#id2840277

so this just doesnt seem right, here's a partial bt of where it is:

(gdb) where
#0  nsWindow::NativeResize(int, int, int, int, int) (this=0x882b190, aX=0, aY=0, 
aWidth=71582793, aHeight=71582792, aRepaint=0) at nsWindow.cpp:2412
#1  0x40f12665 in nsCommonWidget::Show(int) (this=0x882b190, aState=1) at 
nsCommonWidget.cpp:253
#2  0x41be0c9f in nsXULWindow::SetVisibility(int) (this=0x882b088, 
aVisibility=1) at nsXULWindow.cpp:742
#3  0x41be1841 in nsXULWindow::OnChromeLoaded() (this=0x882b088) at nsXULWindow.
cpp:966
#4  0x41bf33d7 in nsWebShellWindow::OnStateChange(nsIWebProgress*, nsIRequest*, 
unsigned, unsigned) (this=0x882b088, aProgress=0x8831d04, aRequest=0x87dd2e0, 
aStateFlags=786448, aStatus=2147549183) at nsWebShellWindow.cpp:1292
#5  0x41d67f36 in nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, 
int, unsigned) (this=0x8831cf0, aProgress=0x8831d04, aRequest=0x87dd2e0, 
aStateFlags=786448, aStatus=2147549183) at nsDocLoader.cpp:1203
#6  0x41d6746b in nsDocLoader::doStopDocumentLoad(nsIRequest*, unsigned) 
(this=0x8831cf0, request=0x87dd2e0, aStatus=2147549183) at nsDocLoader.cpp:822
#7  0x41d6717e in nsDocLoader::DocLoaderIsEmpty() (this=0x8831cf0) at 
nsDocLoader.cpp:722
#8  0x41d66f11 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, 
unsigned) (this=0x8831cf0, aRequest=0x87dd2e0, aCtxt=0x0, aStatus=2147549183) at 
nsDocLoader.cpp:645
#9  0x410cfafe in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, 
unsigned) (this=0x88344f0, request=0x87dd2e0, ctxt=0x882b204, 
aStatus=2147549183) at nsLoadGroup.cpp:701
#10 0x43d6c651 in nsJARChannel::OnStopRequest(nsIRequest*, nsISupports*, 
unsigned) (this=0x87dd2e0, req=0x87dd5e8, ctx=0x0, status=2147549183) at 
nsJARChannel.cpp:694
#11 0x410c6995 in nsInputStreamPump::OnStateStop() (this=0x87dd5e8) at 
nsInputStreamPump.cpp:504
#12 0x410c632f in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) 
(this=0x87dd5e8, stream=0x87dd64c) at nsInputStreamPump.cpp:341
#13 0x401886b9 in nsInputStreamReadyEvent::EventHandler(PLEvent*) (plevent=0x0) 
at nsStreamUtils.cpp:118
#14 0x401a7d63 in PL_HandleEvent (self=0x87dd4a4) at plevent.c:698
#15 0x401a7c47 in PL_ProcessPendingEvents (self=0x80db010) at plevent.c:633
#16 0x401aaab9 in nsEventQueueImpl::ProcessPendingEvents() (this=0x80f6010) at 
nsEventQueue.cpp:413
#17 0x40f0f555 in event_processor_callback (source=0x8259918, condition=G_IO_IN, 
data=0x882b204) at nsAppShell.cpp:67

and after removing the breakpoint and continuing, it exits at:

The program 'Gecko' received an X Window System error.
This probably reflects a bug in the program.
The error was 'BadAlloc (insufficient resources for operation)'.
  (Details: serial 41853 error_code 11 request_code 53 minor_code 0)
  (Note to programmers: normally, X errors are reported asynchronously;
   that is, you will receive the error a while after causing it.
   To debug your program, run it with the --sync command line
   option to change this behavior. You can then get a meaningful
   backtrace from your debugger if you break on the gdk_x_error() function.)

(despite what it says, breaking on gdk_x_error() isn't useful)

I hope someone finds this helpful, i wish i could be more helpful (I know very 
little about c++, i'd be more comfortable with c!).

I've seen this succeed while I was experimenting, and X creates a massive window 
and the whole system slows to a crawl, with massive amounts of memory being 
used. The server stopped responding and I had to login via ssh and kill moz! I 
can believe the x server can crash when this happens.

I hope this is useful.

Comment 4

[Tavis Ormandy]

this is with cvs, btw, checked out a couple of hours ago. it also happens with 
firefox 1.0 and mozilla (current stable and beta).

Comment 5

[Tavis Ormandy]

here's some backtrace before the sanity test

Breakpoint 1, nsCommonWidget::Show(int) (this=0x87b7fe8, aState=1) at 
nsCommonWidget.cpp:234
(gdb) bt
#0  nsCommonWidget::Show(int) (this=0x87b7fe8, aState=1) at nsCommonWidget.cpp:
234
#1  0x422fdc06 in DocumentViewerImpl::Show() (this=0x8818fd8) at 
nsDocumentViewer.cpp:1469
#2  0x41c3c0fd in nsDocShell::SetVisibility(int) (this=0x83b6a68, aVisibility=1) 
at nsDocShell.cpp:3419
#3  0x41abbc85 in nsXULWindow::SetVisibility(int) (this=0x867a638, 
aVisibility=1) at nsXULWindow.cpp:741
#4  0x41abc841 in nsXULWindow::OnChromeLoaded() (this=0x867a638) at nsXULWindow.
cpp:966
#5  0x41ace3d7 in nsWebShellWindow::OnStateChange(nsIWebProgress*, nsIRequest*, 
unsigned, unsigned) (this=0x867a638, aProgress=0x83b6a7c, aRequest=0x865eee8, 
aStateFlags=786448, aStatus=2147549183) at nsWebShellWindow.cpp:1292
#6  0x41c72f36 in nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, 
int, unsigned) (this=0x83b6a68, aProgress=0x83b6a7c, aRequest=0x865eee8, 
aStateFlags=786448, aStatus=2147549183) at nsDocLoader.cpp:1203
#7  0x41c7246b in nsDocLoader::doStopDocumentLoad(nsIRequest*, unsigned) 
(this=0x83b6a68, request=0x865eee8, aStatus=2147549183) at nsDocLoader.cpp:822
#8  0x41c7217e in nsDocLoader::DocLoaderIsEmpty() (this=0x83b6a68) at 
nsDocLoader.cpp:722
#9  0x41c71f11 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, 
unsigned) (this=0x83b6a68, aRequest=0x865eee8, aCtxt=0x0, aStatus=2147549183) at 
nsDocLoader.cpp:645
#10 0x410cfafe in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, 
unsigned) (this=0x87e9288, request=0x865eee8, ctxt=0x40f3a0e8, 
aStatus=2147549183) at nsLoadGroup.cpp:701

Comment 6

[Tavis Ormandy]

Just searching bugzilla, I believe the Bob Vickers from bug 40931, comment 53 is 
seeing this problem...note that this bug isnt a dupe of 40931, that was a gdk 
bug, this is a bug in moz (as far as i can tell!).

Comment 7

[Tavis Ormandy]

sorry, that should have been bug 40931, comment 56

Comment 8

[Adam Hauner]

Moving to Core/Widget: Gtk

Comment 9

[Boris Zbarsky [:bzbarsky]]

Brian, any idea what's up here?

Comment 10

[Christopher Blizzard (:blizzard)]

You can pass those sizes to gtk2 widgets - they map 32 bit coordinates for
widget sizes and locations for inner windows.  But depending on the plugin, it
might crash the plugin if it's not ready for a size like that.  The X Window
system has 16-bit coordinates.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Chris, the attached testcase completely kills XFree86 4.3.0 with both GTK1 and
GTK2 builds here.  So perhaps we need to do something to guard against this sort
of bad input anyway?

Comment 12

[Christopher Blizzard (:blizzard)]

I can reproduce here.  We're trying to create a window that's large and it
causes X to grow in size, often beyond the limits of the machine.

I'll bet layout is trying to create a window that's of unbounded size.  Fixing
this down at the widget layer is probably papering over the real problem in
layout.  I would suggest figuring out what's happening down in layout.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I can try to do that if I can open the testcase without crashing....

If you know where the bogus call into widget is being made, could you point me
to it?  I can add a safety check there and break inside that, then debug....

Comment 14

[Christopher Blizzard (:blizzard)]

No, I'm not sure. But you can put a check in nsCommonWidget::Resize().  Note
that there are two methods with different signatures.

Comment 15

[Boris Zbarsky [:bzbarsky]]

OK.  What sort of check would be reasonable?  I really don't know enough about
the widget code to be debugging this by guess-and-check, since a mistake brings
down my OS (well, brings down X, which then freezes up everything to the point
that I can't even ssh into the machine).

Comment 16

[Christopher Blizzard (:blizzard)]

Just throw in a check that looks for really big windows ( > 2000 pixels is
probably enough.)

Comment 17

[Boris Zbarsky [:bzbarsky]]

Hmm... I managed to get the assert I added once (and I'd forgotten to set the
silly env var to drop into the debugger, so it didn't help).  But now I
consistently get the thing loading successfully (with 2 JS console windows
opening, "as expected").

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Closing because no crash reported since 12 weeks.

Comment 19

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

Reopening because crash bugs **with testcases** should not be resolved **as WONTFIX** based on queries of crash-stats.  Other resolutions may be appropriate for other reasons.

(Crash signatures are not the same as bug identity; they're merely a search aid to find and group similar crashes.  The bug may still be present, but the signature may have changed slightly, or the bug may even still be present with the same signature but there are simply no recent reports of crashes in that function.)

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:stransky, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 21

[Martin Stránský [:stransky] (ni? me)]

AFAIK outdated.