354595 - Crash setting flashEmbed[0] in Flash Player code called from nsJSObjWrapper::GetNewOrUsed

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jesse Ruderman]

This crashes if "emb" is a Flash movie:

  var emb = document.getElementById("emb")
  emb[0] = 3;

Stack trace (note that the symbols for Flash Player are bogus):

0   ...romedia.Flash Player.plugin 	0x06b63b38 Flash_EnforceLocalSecurity + 374272
1   ...romedia.Flash Player.plugin 	0x06b638a4 Flash_EnforceLocalSecurity + 373612
2   ...romedia.Flash Player.plugin 	0x06cce754 native_ShockwaveFlash_TCallFrame + 452328
3   ...romedia.Flash Player.plugin 	0x06cce6f4 native_ShockwaveFlash_TCallFrame + 452232
4   org.mozilla.firefox            	0x007f6e50 nsJSObjWrapper::GetNewOrUsed(_NPP*, JSContext*, JSObject*) + 2956
5   libmozjs.dylib                 	0x2304c2e0 js_LookupPropertyWithFlags + 684
6   libmozjs.dylib                 	0x23030b2c js_CheckRedeclaration + 84
7   libmozjs.dylib                 	0x23048ea8 js_HasOwnPropertyHelper + 1156
8   libmozjs.dylib                 	0x2302ff28 js_Invoke + 1828
9   libmozjs.dylib                 	0x2303a4d0 js_Interpret + 37512
...

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Raymond Walsh]

It appears possible for PL_DHashTableOperate to return null (when the table is on the verge of overflowing and the object exists), but this condition is not trapped.



dhcp145:~/cvsrepos/mozilla/modules/plugin/base/src apple$ cvs diff -u8p ./nsJSNPRuntime.cpp 
Index: nsJSNPRuntime.cpp
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/base/src/nsJSNPRuntime.cpp,v
retrieving revision 1.23
diff -u -8 -p -r1.23 nsJSNPRuntime.cpp
--- nsJSNPRuntime.cpp   27 Mar 2007 15:33:41 -0000      1.23
+++ nsJSNPRuntime.cpp   17 Apr 2007 18:39:22 -0000
@@ -948,16 +948,19 @@ nsJSObjWrapper::GetNewOrUsed(NPP npp, JS
   }
 
   nsJSObjWrapperKey key(obj, npp);
 
   JSObjWrapperHashEntry *entry =
     NS_STATIC_CAST(JSObjWrapperHashEntry *,
                    PL_DHashTableOperate(&sJSObjWrappers, &key, PL_DHASH_ADD));
 
+  if (!entry) {
+      return nsnull;
+  }
   if (PL_DHASH_ENTRY_IS_BUSY(entry) && entry->mJSObjWrapper) {
     // Found a live nsJSObjWrapper, return it.
 
     return _retainobject(entry->mJSObjWrapper);
   }
 
   // No existing nsJSObjWrapper, create one.
 

Comment 3

[Johnny Stenback (:jst)]

Attachment: Possible fix.

Comment 4

[Johnny Stenback (:jst)]

And that fix is of course based on Raymonds findings. I'm not sure yet whether it actually fixes this bug, but it's the right thing to do either way.

Comment 5

[Johnny Stenback (:jst)]

Fix checked in on trunk. Leaving bug open as I don't know if this actually fixes this bug. If it appears to, feel free to mark this bug fixed.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 261878 [details] [diff] [review]
Possible fix.

approved for 1.8.1.7, a=dveditz

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 261878 [details] [diff] [review]
Possible fix.

Requesting approval to land this on branches, I've gotten off-line reports of people seeing this crash on the branch, but not on the trunk since this landed there.

Comment 8

[Johnny Stenback (:jst)]

Marking bug fixed as there hasn't been any sign of this being a problem since this patch landed on the trunk.

Comment 9

[Johnny Stenback (:jst)]

Attachment: Branch version, for the record (same change as above w/o some style cleanup).

This is the same as the trunk version with the exception of the first hunk that only changes code formatting (and doesn't apply to the branch).

Comment 10

[Johnny Stenback (:jst)]

Fixed on branch.

Comment 11

[Carsten Book [:Tomcat]]

verified fixed 1.8.1.7 using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.7pre) Gecko/20070913 BonEcho/2.0.0.7pre ID:2007091303 

no crash on testcase - adding verified keyword

Comment 12

[Daniel Veditz [:dveditz]]

I don't know if this is the cause, but Mac Flash crashes have skyrocketed in 2.0.0.8 compared to 2.0.0.7.  Even though total crashes in a 10 day window favor the old version 45K to 27K, crashes in Flash on a Mac have gone from 223 to 844.

FF 2.0.0.7             FF 2.0.0.8
223 Flash crashes      844 Flash crashes
13.5% of Mac crashes   60% of Mac crashes
0.5% of all crashes    3% of all crashes

Comment 13

[Martijn Wargers (dead)]

Yeah, I noticed quite a few Mac people complaining about crashing after the update in the Mozilla feedback newsgroups.
Carsten mentioned that it could be related to bug 400554.
I don't have a Mac, so I can't really help with testing.

Comment 14

[Daniel Veditz [:dveditz]]

I filed bug 400714 to cover comment 12 -- this fix may or may not have been the cause so it should not be tracked here.