108347 - Crash (linux/unix only) after NPP_SetWindow() call into flash plugin with window->width[height] <= 0

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Sakari Lehtonen]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.5) Gecko/20011014
BuildID:    2001101400

When entering http://www.moontv.fi you are to be redirected to the actual pages.
Mozilla crashes shortly after the redirect. I am using a prepackaged mozilla
0.9.5 from www.linuxmafia.org for Slackware, and this build doesn't seem to have
talkback enabled so I am unable to post any additional information. Maybe
someone can confirm this.

I have javascript enabled, and the flash plugin for linux installed.

Reproducible: Always
Steps to Reproduce:
1. Go to http://www.moontv.fi
2. Wait a few seconds
3.

Actual Results:  Mozilla crashed.

Expected Results:  Redirected to the actual pages.

Comment 1

[Matthias Versen [:Matti]]

Reporter:
Can you please use a nightly build with talkback.
Please poste the Talkback ID if you crash with this nightly and talkback
submitted the crash.
(run "install-dir\mozilla\bin\components\talkback")

Comment 2

[Adam Hauner]

Page loaded okay with 20011027-trunk.

Comment 3

[R.K.Aa.]

There is flash on the page it redirects to. Tested with flash installed: WFM.
Linux 2001102910

Comment 4

[Sakari Lehtonen]

I tested this again with a talkback enabled nighly build (build ID 2001110121)
and now Mozilla seems to just freeze, I had to kill it manually.

The talkback submissions are queued at the moment.

Comment 5

[Diego Biurrun]

I crash with 0.9.5 and 2001102909 ---> NEW

Comment 6

[Diego Biurrun]

2001111521 crashes with flash plugin but not without ---> Plug-ins

Comment 7

[Diego Biurrun]

TB38101679Z
TB38101836Q

both with 2001111606

Stephen, it appears to the custom to CC you now and ask you to post the talkback
data here. Thanks.

Comment 8

[[e-mail] (gone - use [e-mail] instead)]

Stack Signature  libc.so.6 + 0x4f21d (0x404d221d) 4db8284a
Bug ID
Trigger Time 2001-11-16 07:11:09
Email Address [e-mail]
URL visited www.moontv.fi
User Comments Tried to confirm bug 108347. I had success as it seems.
Build ID 2001111606
Product ID MozillaTrunk
Platform
Operating System LinuxIntel
Module
Trigger Reason SIGSEGV: Segmentation Fault: (signal 11)
Stack Trace
libc.so.6 + 0x4f21d (0x404d221d)
libc.so.6 + 0x4f0ce (0x404d20ce)
JS_malloc()
XPCStringConvert::ReadableToJSString()
XPCConvert::NativeData2JS()
XPCWrappedNative::CallMethod()
XPC_WN_CallMethod()
js_Invoke()
js_Interpret()
js_Invoke()
js_InternalInvoke()
js_GetProperty()
js_Interpret()
js_Invoke()
js_InternalInvoke()
js_SetProperty()
js_Interpret()
js_Invoke()
nsXPCWrappedJSClass::CallMethod()
nsXPCWrappedJS::CallMethod()
PrepareAndDispatch()
nsXPTCStubBase::Stub3()
nsDocLoaderImpl::FireOnStateChange()
nsDocLoaderImpl::FireOnStateChange()
nsDocLoaderImpl::doStopURLLoad()
nsDocLoaderImpl::OnStopRequest()
nsLoadGroup::RemoveRequest()
PresShell::RemoveDummyLayoutRequest()
PresShell::DoneRemovingReflowCommands()
PresShell::ProcessReflowCommands()
HandlePLEvent()
PL_HandleEvent()
PL_ProcessEventsBeforeID()
processQueue()
nsVoidArray::EnumerateForwards()
nsAppShell::ProcessBeforeID()
handle_gdk_event()
libgdk-1.2.so.0 + 0x17027 (0x40332027)
libglib-1.2.so.0 + 0x102f9 (0x403622f9)
libglib-1.2.so.0 + 0x10903 (0x40362903)
libglib-1.2.so.0 + 0x10a9c (0x40362a9c)
libgtk-1.2.so.0 + 0x8e457 (0x40284457)
nsAppShell::Run()
nsAppShellService::Run()
main1()
main()
libc.so.6 + 0x18a42 (0x4049ba42) 

Comment 9

[Diego Biurrun]

reassign to default owners

Comment 10

[Sakari Lehtonen]

Tested again with build ID 2001111906, crashes. This time submitting the
talkback succeeded:

TB38234922K

Comment 11

[Diego Biurrun]

Still a problem with 0.9.6

TB38411971Q

Stephen, could you please retrieve this? Thanks.

Comment 12

[Peter Lubczynski]

--->XPConnect based on stack? Feel free to bounce back....

Comment 13

[David Bradley]

This looks more like heap corruption than anything else, given it's dying in
malloc. Is there anything like Purify on Linux that can detect such heap
corruption when it happens, or at least sooner?

Comment 14

[David Bradley]

Per my previous comment I'm pretty sure this isn't an xpconnect issue. Who's
problem it is, is hard to say. I think this calls for a Linux heap corruption
sluth to track the origin of this problem

Comment 15

[Peter Lubczynski]

-->serge

Comment 16

[Phil Schwartau]

Setting default QA -

Comment 17

[Sergi]

Was found during investigation of 114047.

Solaris platform:
1. Run 'openwin -dev /dev/fb defdepth 24'
2. Run mozilla (solaris build) with flash plugin installed
Site works fine.
3. Run linux build of mozilla with flash plugin installed, send
   output to 24bit solaris Xserver
Site works fine.

Looks like reason of fault are the same as for 114047: bad visual.
Need to check different depthes on linux Xserver.

Going to investigate more. You can assign bug to me.

Comment 18

[Sergey Lunegov]

Attachment: workaround

This is actually not path but a workaround.

Comment 19

[Sergey Lunegov]

I think the problem is the various implementation of X system on
various platforms. And maybe some issues in GTK. I noted that
browser crashes if flash plugin is located inside some frame: <frameset>
or <table>. In these cases the window for plugin at the creating time
has width equal to 0. It causes the crash. I think that the root reason
is in GTK implementation. If both widht and height of window plugin
are non zero then browser doesn't crash. Reporter, would you please
to try to apply this patch and test www.moontv.fi.
The proposed path also may fix problem in bug #110248.

Comment 20

[Sakari Lehtonen]

Sorry, I cannot test this patch. 

I do not want to download the mozilla source (with my ISDN) and then fail
miserably when trying to actually build it. I am now using Galeon 1.0 with
Mozilla 0.9.6 (not a nightly) and sometimes the site works, some times it does
not. Some times it crashes when leaving the site. With Mozilla it always crashes.

Comment 21

[Diego Biurrun]

Correction of my last comment:

Now this is funky: I crash if I load the URL in a window, however I do _not_
crash if I load the URL in a tab.

Steps to reproduce:

a)
1. Load the URL in a window on its own.
2. Mozilla crashes.

b)
1. Load any page.
2. Open a new tab and load the URL.
3. Browser does not crash. 

Comment 22

[serge (gone)]

Sergey Lunegov, your patch proposal looks workable to me, 
the problem here is with flash plugin on linux (probably on any unix flavor) 
which does not handle width=0 or height=0 properly, at least I'm seeing random 
memory corruption after several NPP_SetWindow() calls.
Here is a test case:
---
<html>
<body>
<EMBED src="dummy.swf"
  WIDTH=0
  HEIGHT=0
  TYPE="application/x-shockwave-flash"
> 
</body>
</html>
---
just keep pressing reload button an wait for the crash.
I'm just thinking, maybe there is no reason at all to call NPP_SetWindow() with 
bad width/height
so in this case the patch will look like:
======
RCS file: /cvsroot/mozilla/modules/plugin/base/src/ns4xPluginInstance.cpp,v
retrieving revision 1.70
diff -u -r1.70 ns4xPluginInstance.cpp
--- ns4xPluginInstance.cpp      2001/12/08 01:13:05     1.70
+++ ns4xPluginInstance.cpp      2001/12/11 15:32:13
@@ -843,6 +843,9 @@
     return NS_OK;
   
   NPError error;
+
+  if (window->width == 0 || window->height == 0)
+    return NS_OK;
  
 #ifdef MOZ_WIDGET_GTK
   // Allocate and fill out the ws_info data
======
av, peterl what do you think?

Comment 23

[serge (gone)]

Attachment: a simple test case

the same as above

Comment 24

[Peter Lubczynski]

wonderufl! r=peterl

Comment 25

[av (gone)]

Hm... Why is (0,0) bad? I mean, if we don't send setwindow in such a case, is 
there a possibility that we can miss something? Say, initialization call, or 
changing handle call? The spec is not very friendly here. What is the reason we 
are trying to send setwindow with zero directions at all? Does it happen on Unix 
only?

Comment 26

[Diego Biurrun]

Just made a fresh build with the patch/workaround included and I do not crash
anymore :-)

Comment 27

[serge (gone)]

av, I do not have the answers on your questions:(
except the last one,
>Does it happen on Unix only?
I've debugged only linux (using test case attachment 61317 [details]), and yes, it does 
happen on linux, the memory is getting corrupt somehow, and mozilla crashes with 
random stack trace.

Comment 28

[Sergi]

It happens on Linux only. Moreover, not linux build of mozilla, but any
unix build of mozilla with displaying to Linux X.

Comment 29

[serge (gone)]

Attachment: slightly modified Sergey Lunegov's first patch

add check for negative values of window->width[height]

Comment 30

[serge (gone)]

Yes, I can suspect this is most likely because of window->width[height] < 0;
I was able to see negative window->width value on ebolt.hu only once in very 
first debug session:(
Tibor, would you try to apply  attachment 61965 [details] [diff] [review] from bug #108347, which does 
check for negatives, and it would be more useful if you post the crash stack 
trace here, instead of debug output.
Thanks.

Comment 31

[serge (gone)]

please, disregard my previous comment #30,
it belongs to bug# 110248, which is moat likely a dup of this one.

Comment 32

[Diego Biurrun]

Just pulled and made a build with the updated patch..  works fine over here.  No
crash on moontv.fi or on the testcase, not in a tab, not in a window by itself.

Comment 33

[Diego Biurrun]

Since the patch seems to fix the crash and is really small, is there any chance
this is going to be checked in soon?

Comment 34

[Peter Lubczynski]

Comment on attachment 61965 [details] [diff] [review]
slightly modified Sergey Lunegov's first patch

r=peterl

Comment 35

[serge (gone)]

Peter , thanks you for r=,
I'm nominating this for mozilla 0.9.8,
the patch itself is simple enough, and inside  #ifdef MOZ_WIDGET_GTK
I'm 100% positive it'll protect us from a bunch of random crashes, 
because when layout is doing reflow plugin's window size is not determine yet 
properly, and SetWindow could be called with (w<=0,h<=0), which is enough for 
flash plugin to corrupt the memory.
Attachment 61317 [details] is the simplest test case with (0,0).
Blizzard, would you please sr= for this? 
Thanks.

Comment 36

[Christopher Blizzard (:blizzard)]

Are you absolutely sure that ::SetWindow will be called again later when the
size is determined?

Comment 37

[serge (gone)]

100% sure, that is how it works.

Comment 38

[Christopher Blizzard (:blizzard)]

r=blizzard, a=blizzard on behalf of drivers for 0.9.8

Comment 39

[serge (gone)]

on the trunk.
changing summary.
thank you to all.

Comment 40

[Diego Biurrun]

Just pulled and made a fresh clean build.  No crash, not on the page, not on the
testcase.  Good work, guys.  Marking VERIFIED.

Comment 41

[Sakari Lehtonen]

Tested the page and the testcase with build ID 2002011808, no crash. Great.