Closed Bug 582649 Opened 14 years ago Closed 14 years ago

Too-much-recursion crash with setUserData [@ * | XPCConvert::JSArray2Native]

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
status1.9.2 --- .13-fixed
status1.9.1 --- .16-fixed

People

(Reporter: martijn.martijn, Assigned: peterv)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, Whiteboard: [sg:dos])

Crash Data

Attachments

(2 files, 1 obsolete file)

Testcase (crashes browser)
162 bytes, text/html
Details
v1 (with crashtest)
2.98 KB, patch
peterv
: review+
benjamin
: approval2.0+
dveditz
: approval1.9.2.11-
dveditz
: approval1.9.2.13+
dveditz
: approval1.9.1.14-
dveditz
: approval1.9.1.16+
Details | Diff | Splinter Review 
See unminimized testcase, you need to unzip it, then open the file named 'parentframe.htm'. And you need the script grant enhanced privileges (this is to force gc).
This is a typical crash, that I get:
http://crash-stats.mozilla.com/report/index/4c1f136e-d445-4630-ad33-1863f2100728
0  	xul.dll  	nsXPConnect::WrapJS  	 js/src/xpconnect/src/nsXPConnect.cpp:1348
1 	xul.dll 	XPCVariant::InitializeData 	js/src/xpconnect/src/xpcvariant.cpp:392
2 	xul.dll 	XPCVariant::newVariant 	js/src/xpconnect/src/xpcvariant.cpp:148
3 	xul.dll 	XPCConvert::JSData2Native 	js/src/xpconnect/src/xpcconvert.cpp:977
4 	xul.dll 	XPCConvert::JSArray2Native 	
5 	xul.dll 	XPCVariant::InitializeData 	
6 	xul.dll 	XPCConvert::JSData2Native 	js/src/xpconnect/src/xpcconvert.cpp:977
7 	xul.dll 	XPCConvert::JSArray2Native 	
8 	xul.dll 	XPCVariant::InitializeData 


Other crashes that I got:
http://crash-stats.mozilla.com/report/index/bp-202dd796-19f0-4e83-8359-165612100728
0  	xul.dll  	nsXPCWrappedJSClass::DelegatedQueryInterface  	 js/src/xpconnect/src/xpcwrappedjsclass.cpp:645
1 	xul.dll 	nsXPCWrappedJS::QueryInterface 	js/src/xpconnect/src/xpcwrappedjs.cpp:185
2 	xul.dll 	XPCConvert::JSObject2NativeInterface 	
3 	xul.dll 	xul.dll@0xa5eabb

http://crash-stats.mozilla.com/report/index/bp-148387f3-57ca-4d81-bd43-85f592100728
0  	xul.dll  	nsXPCWrappedJSClass::DelegatedQueryInterface  	 js/src/xpconnect/src/xpcwrappedjsclass.cpp:645
1 	xul.dll 	nsXPCWrappedJS::QueryInterface 	js/src/xpconnect/src/xpcwrappedjs.cpp:185
2 	xul.dll 	XPCConvert::JSObject2NativeInterface 	
3 	xul.dll 	xul.dll@0xa5eabb

Unfortunately, I wasn't able to minimize the testcase further, thus far.
Is this 4.0-only, or also 3.6.x?
The unminimized testcase only crashes on trunk, but that might perhaps only be because of the oddness of it.
For me this crashed in 3.6.x on OS X in nsCycleCollectingAutoRefCnt::get.
Assignee: nobody → peterv
Status: NEW → ASSIGNED
This looks like it's caused by a recursive reference in an array. We recurse to death trying to wrap the array in XPConnect.
Attached patch v1 (obsolete) — — Splinter Review 
Would it be ok to use the SpiderMonkey recursion checks in XPConnect?
Attachment #462151 - Flags: feedback?
Attachment #462151 - Flags: feedback? → feedback?(brendan)
Comment on attachment 462151 [details] [diff] [review]
v1

Brendan said "sure" on irc.
Attachment #462151 - Flags: feedback?(brendan) → review?(mrbkap)
Group: core-security
Keywords: testcase
Summary: Crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash [@ nsXPConnect::WrapJS]
Whiteboard: [sg:dos]
Attachment #462151 - Flags: review?(mrbkap) → review+
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] → Too-much-recursion crash with setUserData
Summary: Too-much-recursion crash with setUserData → Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData
Summary: Too-much-recursion crash [@ nsXPConnect::WrapJS] with setUserData → Too-much-recursion crash with setUserData [@ * | XPCConvert::JSArray2Native]
Should the patch get approval?
Fixes crash.
Attachment #462151 - Attachment is obsolete: true
Attachment #467752 - Flags: review+
Attachment #467752 - Flags: approval2.0?
Attachment #467752 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/f0f25f2693cd
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

Simple fix to catch recursion crash.
Attachment #467752 - Flags: approval1.9.2.10?
Attachment #467752 - Flags: approval1.9.1.13?
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Attachment #467752 - Flags: approval1.9.2.11?
Attachment #467752 - Flags: approval1.9.2.11+
Attachment #467752 - Flags: approval1.9.1.14?
Attachment #467752 - Flags: approval1.9.1.14+
Comment on attachment 467752 [details] [diff] [review]
v1 (with crashtest)

missed the 1.9.2.11/1.9.1.14 releases, go for next time.
Attachment #467752 - Flags: approval1.9.2.12+
Attachment #467752 - Flags: approval1.9.2.11-
Attachment #467752 - Flags: approval1.9.2.11+
Attachment #467752 - Flags: approval1.9.1.15+
Attachment #467752 - Flags: approval1.9.1.14-
Attachment #467752 - Flags: approval1.9.1.14+
Crash Signature: [@ * | XPCConvert::JSArray2Native]
Depends on: 731334
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: