Last Comment Bug 678440 - Firefox 8.0a1 Crash [@ nsNSSSocketInfo::EnsureDocShellDependentStuffKnown()]
: Firefox 8.0a1 Crash [@ nsNSSSocketInfo::EnsureDocShellDependentStuffKnown()]
Status: VERIFIED FIXED
[verified-beta] [qa!]
: crash, regression
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: 8 Branch
: x86 All
: -- critical with 1 vote (vote)
: ---
Assigned To: Luke Wagner [:luke]
:
Mentors:
: 678547 678706 678710 678987 679030 679035 (view as bug list)
Depends on: 679036 679140 679144
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-11 20:53 PDT by Ekanan Ketunuti
Modified: 2011-10-10 01:10 PDT (History)
18 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
unaffected


Attachments
fix (4.60 KB, patch)
2011-08-12 17:33 PDT, Luke Wagner [:luke]
no flags Details | Diff | Splinter Review
patch (4.90 KB, patch)
2011-08-12 19:13 PDT, Luke Wagner [:luke]
no flags Details | Diff | Splinter Review
revert (51.57 KB, patch)
2011-08-16 15:37 PDT, Luke Wagner [:luke]
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Ekanan Ketunuti 2011-08-11 20:53:09 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110811 Firefox/8.0a1
Build ID: 20110811152156

Steps to reproduce:

STR
1. Downloaded https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1313101316/firefox-8.0a1.en-US.win32.zip
2. Extract and ran firefox -P with new profile
3. Installed http://www.echofon.com/twitter/firefox/Echofon.xpi and restart
4. Right click on Echofon toolbar, preferences, advanced, check Use SSL for all requests (This is important), and authorize Echofon
5. when authroized, firefox crash

Note: firefox won't crash if uncheck "Use SSL for all requests".



Actual results:

Crash [@ xul.dll@0x9cd449 ]

0 	xul.dll 	xul.dll@0x9cd449 	
1 	xul.dll 	xul.dll@0x9d830c 	
2 	ssl3.dll 	ssl3.dll@0x6d4f 	
3 	ssl3.dll 	ssl3.dll@0x9563 	
4 	ssl3.dll 	ssl3.dll@0x96cd 	
5 	ssl3.dll 	ssl3.dll@0x9b79 	
6 	ssl3.dll 	ssl3.dll@0x9eb5 	
7 	ssl3.dll 	ssl3.dll@0xb022 	
8 	ssl3.dll 	ssl3.dll@0xfb0d 	
9 	ssl3.dll 	ssl3.dll@0x10920 	
10 	ssl3.dll 	ssl3.dll@0x109ad 	
11 	ssl3.dll 	ssl3.dll@0x135f9 	
12 	xul.dll 	xul.dll@0x42f545 	
13 	nspr4.dll 	nspr4.dll@0x220f 	
14 	nspr4.dll 	nspr4.dll@0x1d2c 	
15 	mozcrt19.dll 	mozcrt19.dll@0x2c27 	
16 	mozcrt19.dll 	mozcrt19.dll@0xd297 	
17 	mozcrt19.dll 	mozcrt19.dll@0x2cb5 	
18 	kernel32.dll 	BaseThreadInitThunk 	
19 	ntdll.dll 	__RtlUserThreadStart 	
20 	kernel32.dll 	WerpReportFaultInternal 	
21 	kernel32.dll 	WerpReportFaultInternal

bp-29258595-2ce0-4bf0-9760-b35632110811
bp-ab4a6d16-8704-4666-99e5-286312110811
bp-d8ffd816-a90e-475f-b6c1-676852110811

Regression
Good http://hg.mozilla.org/mozilla-central/rev/42f7ed136034
Bad http://hg.mozilla.org/mozilla-central/rev/10915aa17365 Mounir Lamouri — Merging mozilla-inbound into mozilla-central. 

m-i
Good http://hg.mozilla.org/integration/mozilla-inbound/rev/dd0630c44b2d
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1312999277/
Bad http://hg.mozilla.org/integration/mozilla-inbound/rev/be91fb29d950
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1313002780/
Comment 1 Alice0775 White 2011-08-12 04:39:48 PDT
Confirmed on nightly Nightly linux build.
http://hg.mozilla.org/mozilla-central/rev/f262c389193e
Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110812 Firefox/8.0a1 ID:20110812030744

bp-7cb93135-30a5-4729-8de1-c2d5d2110812
bp-e8f478cf-a441-4574-821c-728772110812
Comment 2 Alice0775 White 2011-08-12 06:15:05 PDT
And also crashes on
http://hg.mozilla.org/mozilla-central/rev/f262c389193e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110812 Firefox/8.0a1 ID:20110812030744

bp-986a672b-0872-4dd9-b048-5d1de2110812
bp-0e2e0952-9778-4369-843d-f4e9a2110812
Comment 3 Alice0775 White 2011-08-12 08:56:29 PDT
triggered by
be91fb29d950	Luke Wagner — Bug 674597 - abort if attempting to create an xpcom proxy for wrapped JS (r=bsmedberg)
Comment 4 earlpiggot 2011-08-12 13:13:32 PDT
me too

Mozilla/5.0 (Windows NT 5.1; rv:8.0a1) Gecko/20110812 Firefox/8.0a1 ID:20110812030744
Comment 5 Luke Wagner [:luke] 2011-08-12 16:12:09 PDT
Well, it looks like we have a binary component attempting to create an xpcom proxy for wrapped JS.  This is an invalid/unsafe operation.
Comment 6 Luke Wagner [:luke] 2011-08-12 16:13:09 PDT
To be clear: that's all bug 674597 does; check and abort.
Comment 7 Luke Wagner [:luke] 2011-08-12 16:45:07 PDT
Can reproduce, the offending NS_GetProxyForObject is http://hg.mozilla.org/mozilla-central/file/cec797d85529/security/manager/ssl/src/nsNSSIOLayer.cpp#l394; Echofon must be implementing a nsIDocShellTreeItem.
Comment 8 Luke Wagner [:luke] 2011-08-12 17:33:11 PDT
Created attachment 552804 [details] [diff] [review]
fix

This fixes the assert for me.  I just did a simple conversion to runnable like bug 674571.

Kai: before bug 674597 and with this patch, the "rootItem do_QI is null" assert fires (this is with Echofon installed); can I remove it?
Comment 9 Luke Wagner [:luke] 2011-08-12 17:46:02 PDT
Comment on attachment 552804 [details] [diff] [review]
fix

Wrong.
Comment 10 Luke Wagner [:luke] 2011-08-12 19:13:34 PDT
Created attachment 552822 [details] [diff] [review]
patch

Kai, there was a peculiar thing (we think a bug) where NS_ProxyRelease was being called in secureUI right before secureUI was used again.  I just wanted to call your attention to this so you could double check.
Comment 11 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-08-14 23:33:29 PDT
Luke, I found another problem similar to this in the same code path: we will sometimes try to create an XPCOM proxy for an nsIPrompt that is implemented by JS. I am working on a fix that this, that, and at least one other related bug with a single runnable. I would rather use that solution than this one.
Comment 12 Luke Wagner [:luke] 2011-08-15 08:53:44 PDT
Brian, thanks a lot for your help on this.  I have two patches converting two more sites where proxies are used into runnables (bug 678440 and bug 678547), one of which is an nsIPrompt, so hopefully this helps.  I'm now actively investigating this bug.
Comment 13 Luke Wagner [:luke] 2011-08-15 08:55:56 PDT
Oops, I have my bugs confused; this is bug 678440 and I am investigating bug 678706.  But bug 678547 does have the nsIPrompt patch.
Comment 14 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-08-15 11:14:59 PDT
EnsureDocShellDependentStuffKnown is called (through GetPreviousCert) on the critical path of certificate chain validation in the SSL handshake. AFAICT, adding a sync dispatch to it will cause certificate path validation--and all network I/O--to wait for the main thread's event queue to drain. I do not know how much of a performance impact that would have but I think I have an alternative solution with better performance characteristics. Will post soon.
Comment 15 Luke Wagner [:luke] 2011-08-15 11:39:12 PDT
IIUC, this wouldn't be *adding* a sync dispatch, but replacing the (slower) one inherent in proxies.  Actually, this patch replaces three proxy dispatches (each doing a round-trip sync).  You know the code better so I'm happy to let you do the patch though.
Comment 16 Luke Wagner [:luke] 2011-08-16 15:37:03 PDT
Created attachment 553609 [details] [diff] [review]
revert

bsmith has some patches to kill most proxies in ssl (bsmedberg also has a patch under review to remove *all* xpcom/proxies).  However, neither of these will be landing in the next day or two, so bsmith and I agreed to just back out the offending patch and dependent asserts.  This is just a backout, so no r=, just approval to land on just-branched aurora.
Comment 17 christian 2011-08-17 15:11:00 PDT
Comment on attachment 553609 [details] [diff] [review]
revert

Approved for mozilla-aurora
Comment 19 Luke Wagner [:luke] 2011-08-19 10:00:39 PDT
Crashes have dropped to 0, marking fixed.
Comment 20 Luke Wagner [:luke] 2011-08-19 10:04:19 PDT
*** Bug 678547 has been marked as a duplicate of this bug. ***
Comment 21 Luke Wagner [:luke] 2011-08-19 10:05:05 PDT
*** Bug 678987 has been marked as a duplicate of this bug. ***
Comment 22 Luke Wagner [:luke] 2011-08-19 10:05:29 PDT
*** Bug 678706 has been marked as a duplicate of this bug. ***
Comment 23 Luke Wagner [:luke] 2011-08-19 10:06:17 PDT
*** Bug 678710 has been marked as a duplicate of this bug. ***
Comment 24 Mihaela Velimiroviciu (:mihaelav) 2011-10-03 06:21:26 PDT
Build identifier: Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0

I tried to install the plugin from the link in bug description but plugin can not be installed because it is incompatible with Firefox 8.0. Is there another version of plugin that is compatible with Firefox 8?

Thank you!
Comment 25 Ekanan Ketunuti 2011-10-06 22:23:59 PDT
Mihaela, I notice that Echofon 2.2.4 beta 1 compatible with Firefox 8 http://www.echofon.com/twitter/firefox/Echofon_beta.xpi
Comment 26 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-10-07 01:21:48 PDT
*** Bug 679035 has been marked as a duplicate of this bug. ***
Comment 27 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2011-10-07 01:22:29 PDT
*** Bug 679030 has been marked as a duplicate of this bug. ***
Comment 28 Mihaela Velimiroviciu (:mihaelav) 2011-10-07 02:51:41 PDT
Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1
Mozilla/5.0 (X11; Linux i686; rv:10.0a1) Gecko/20111006 Firefox/10.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111006 Firefox/10.0a1
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1

Mozilla/5.0 (X11; Linux i686; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0

Verified as fixed on the above builds: Firefox doesn't crash after authorizing or submiting tweet.

Still, no data is displayed on nightlies in either of the Echofon's tabs, but I will log another bug for that.

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