Closed
Bug 440982
Opened 16 years ago
Closed 15 years ago
To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should not get the script hook unless it needs to and it is safe to call
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: kenorb, Assigned: timeless)
Details
(4 keywords, Whiteboard: [sg:investigate])
Attachments
(1 file, 1 obsolete file)
1.30 KB,
patch
|
timeless
:
review+
timeless
:
superreview+
shaver
:
approval1.9.1+
dveditz
:
approval1.9.0.12+
dveditz
:
approval1.8.1.next+
timeless
:
approval1.8.0.next?
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.8.1.14) Gecko/20080617 Firefox/2.0.0.14 Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.8.1.14) Gecko/20080617 Firefox/2.0.0.14 #0 JS_Assert (s=0x281582b2 "!rt->gcRunning", file=0x28158291 "jsgc.c", ln=1393) at jsutil.c:63 63 abort(); [New Thread 0x834b100 (LWP 100215)] [New Thread 0x9475500 (LWP 100213)] [New Thread 0x94f9300 (LWP 100220)] [New Thread 0x8dd2100 (LWP 100217)] [New Thread 0x89f3600 (LWP 100216)] [New Thread 0x8106800 (LWP 100211)] [New Thread 0x8106500 (LWP 100210)] [New Thread 0x8103100 (LWP 100159)] (gdb) bt #0 JS_Assert (s=0x281582b2 "!rt->gcRunning", file=0x28158291 "jsgc.c", ln=1393) at jsutil.c:63 #1 0x280e1008 in js_NewGCThing (cx=0x8ed2680, flags=1, nbytes=8) at jsgc.c:1393 rt = (JSRuntime *) 0x81fa000 flindex = 0 doGC = Variable "doGC" is not available. #2 0x28138baa in js_NewString (cx=0x8ed2680, chars=0x8608c20, length=42, gcflag=0) at jsstr.c:2442 str = Variable "str" is not available. #3 0x280b8de1 in js_AtomizeString (cx=0x8ed2680, str=0xbfbfdc80, flags=Variable "flags" is not available. ) at jsatom.c:660 gen = 168171 keyHash = 2150442760 key = -1077945212 state = (JSAtomState *) 0x81fa0e8 table = (JSHashTable *) 0x81a7160 he = Variable "he" is not available. #4 0x280b9044 in js_Atomize (cx=0x8ed2680, bytes=0x28eec0d8 "global_for_XPCJSContextStack_SafeJSContext", length=42, flags=0) at jsatom.c:735 chars = (jschar *) 0x8608c20 str = Variable "str" is not available. #5 0x2810393c in js_GetClassId (cx=0x8ed2680, clasp=0x28ef27c0, idp=0xbfbfdcf8) at jsobj.c:2379 #6 0x28109628 in js_NewObject (cx=0x8ed2680, clasp=0x28ef27c0, proto=0x0, parent=0x0) at jsobj.c:2401 #7 0x280a8449 in JS_NewObject (cx=0x8ed2680, clasp=0x28ef27c0, proto=0x0, parent=0x0) at jsapi.c:2393 #8 0x28eb9ba7 in XPCJSContextStack::GetSafeJSContext (this=0x81ac150, aSafeJSContext=0xbfbfde18) at xpcthreadcontext.cpp:180 #9 0x28e94327 in XPCCallContext (this=0xbfbfddfc, callerLanguage=XPCContext::LANG_NATIVE, cx=0x0, obj=0x0, funobj=0x0, name=0, argc=4294967295, argv=0x0, rval=0x0) at xpccallcontext.cpp:93 #10 0x28ec0ce5 in nsXPCWrappedJSClass::DelegatedQueryInterface (this=0xb123f40, self=0xb123fa0, aIID=@0x2909beb4, aInstancePtr=0xbfbfdf18) at xpcwrappedjsclass.cpp:544 #11 0x28ebb9a5 in nsXPCWrappedJS::QueryInterface (this=0xb123fa0, aIID=@0x2909beb4, aInstancePtr=0xbfbfdf18) at xpcwrappedjs.cpp:106 #12 0x28190731 in nsQueryInterface::operator() (this=Variable "this" is not available. ) at nsCOMPtr.cpp:47 #13 0x2909af0d in nsCOMPtr<jsdIScriptHook>::assign_from_qi (this=0xbfbfdf44, qi={mRawPtr = 0xb123fa0}, aIID=@0x2909beb4) at nsCOMPtr.h:1232 #14 0x29098676 in jsds_ScriptHookProc (jsdc=0x8188050, jsdscript=0xa07acc0, creating=0, callerdata=0x0) at nsCOMPtr.h:645 #15 0x2908dabb in jsd_DestroyScriptHookProc (cx=0x818b300, script=0x9e2f500, callerdata=0x8188050) at jsd_scpt.c:628 #16 0x2813499b in js_CallDestroyScriptHook (cx=0x818b300, script=0x9e2f500) at jsscript.c:1458 #17 0x281355c4 in js_DestroyScript (cx=0x818b300, script=0x9e2f500) at jsscript.c:1464 #18 0x280d9765 in fun_finalize (cx=0x818b300, obj=0xea04e38) at jsfun.c:1211 #19 0x2810887c in js_FinalizeObject (cx=0x818b300, obj=0xea04e38) at jsobj.c:2762 #20 0x280e021f in js_GC (cx=0x818b300, gckind=GC_NORMAL) at jsgc.c:3031 #21 0x280ac405 in JS_GC (cx=0x818b300) at jsapi.c:1873 #22 0x2995443c in nsJSContext::Notify (this=0x8652300, timer=0xac081c0) at nsJSEnvironment.cpp:2227 #23 0x282006e6 in nsTimerImpl::Fire (this=0xac081c0) at nsTimerImpl.cpp:397 #24 0x2820197f in handleTimerEvent (event=0x9d13a30) at nsTimerImpl.cpp:459 #25 0x281f94b4 in PL_HandleEvent (self=0x9d13a30) at plevent.c:688 #26 0x281f984c in PL_ProcessPendingEvents (self=0x81e5b80) at plevent.c:623 #27 0x281fcce6 in nsEventQueueImpl::ProcessPendingEvents (this=0x81e5b50) at nsEventQueue.cpp:448 #28 0x290ea845 in event_processor_callback (source=0x85effc0, condition=G_IO_IN, data=0x81e5b50) at nsAppShell.cpp:67 #29 0x28a8af29 in g_io_channel_unix_get_fd () from /usr/local/lib/libglib-2.0.so.0 #30 0x28a5968c in g_main_context_dispatch () from /usr/local/lib/libglib-2.0.so.0 #31 0x28a5c6d2 in g_main_context_check () from /usr/local/lib/libglib-2.0.so.0 #32 0x28a5c995 in g_main_loop_run () from /usr/local/lib/libglib-2.0.so.0 #33 0x2841d294 in gtk_main () from /usr/local/lib/libgtk-x11-2.0.so.0 #34 0x290eac05 in nsAppShell::Run (this=0x82b6c80) at nsAppShell.cpp:139 #35 0x291b24f8 in nsAppStartup::Run (this=0x82b7af0) at nsAppStartup.cpp:151 #36 0x08054476 in XRE_main (argc=1, argv=0xbfbfe794, aAppData=0x8069720) at nsAppRunner.cpp:2817 #37 0x0804aff1 in main (argc=100159, argv=0x0) at nsBrowserApp.cpp:61 Reproducible: Always
Component: General → JavaScript Debugging APIs
Keywords: assertion
Product: Firefox → Core
QA Contact: general → jsd
Summary: SEGV in JS_Assert on abort() → jsds_ScriptHookProc triggers JS under JS_GC
Version: unspecified → 1.8 Branch
right, so the js we're running isn't obvious, It's listed as coming from this: nsCOMPtr<jsdIScriptHook>::assign_from_qi, thankfully there's only one of those in this function. The script hook of course is written in JS. 702 jsds_ScriptHookProc (JSDContext* jsdc, JSDScript* jsdscript, JSBool creating, 703 void* callerdata) 704 { 710 nsCOMPtr<jsdIScriptHook> hook; 711 gJsds->GetScriptHook (getter_AddRefs(hook));
Assignee: nobody → timeless
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #380028 -
Flags: superreview?(brendan)
Attachment #380028 -
Flags: review?(brendan)
Updated•15 years ago
|
Attachment #380028 -
Flags: superreview?(brendan)
Attachment #380028 -
Flags: superreview+
Attachment #380028 -
Flags: review?(brendan)
Attachment #380028 -
Flags: review+
Comment 2•15 years ago
|
||
Comment on attachment 380028 [details] [diff] [review] only grab hook if we need it That was easy. Good fixing there! /be
Comment 3•15 years ago
|
||
Isn't this a problem on trunk and 1.9.1 branch? Please nominate if so. /be
A checkin message that describes the change ("only get script hook if we're going to call it, to avoid possible calls back into script under JS_GC's notification", perhaps) rather than the state that, post-patch, no longer exists would probably be more useful.
brendan: yes, it is, the bug is ancient and should go to all live branches. shaver: yes, thanks.
OS: FreeBSD → All
Hardware: x86 → All
Summary: jsds_ScriptHookProc triggers JS under JS_GC → To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should not get the script hook unless it needs to and is safe to call
Version: 1.8 Branch → Trunk
Summary: To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should not get the script hook unless it needs to and is safe to call → To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should not get the script hook unless it needs to and it is safe to call
asking for approval for all branches (and someone should push when approved)
Attachment #380028 -
Attachment is obsolete: true
Attachment #380799 -
Flags: superreview+
Attachment #380799 -
Flags: review+
Attachment #380799 -
Flags: approval1.9.1?
Attachment #380799 -
Flags: approval1.9.0.12?
Attachment #380799 -
Flags: approval1.9.0.11?
Attachment #380799 -
Flags: approval1.8.1.next?
Attachment #380799 -
Flags: approval1.8.0.next?
Updated•15 years ago
|
Attachment #380799 -
Flags: approval1.9.0.11?
Updated•15 years ago
|
Group: core-security
Flags: wanted1.9.1.x+
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Whiteboard: [sg:investigate]
Comment on attachment 380799 [details] [diff] [review] updated comment a191=shaver
Attachment #380799 -
Flags: approval1.9.1? → approval1.9.1+
Comment 8•15 years ago
|
||
This needs to land somewhere and bake before we'll take it for 1.9.0.
Whiteboard: [sg:investigate] → [sg:investigate][needs trunk/1.9.1 landing]
Comment 9•15 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/040cb204cd92
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [sg:investigate][needs trunk/1.9.1 landing] → [sg:investigate][needs 1.9.1 landing]
Comment 10•15 years ago
|
||
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9f212010c4d2
Keywords: fixed1.9.1
Whiteboard: [sg:investigate][needs 1.9.1 landing] → [sg:investigate]
Updated•15 years ago
|
Attachment #380799 -
Flags: approval1.9.0.12? → approval1.9.0.12+
Comment 11•15 years ago
|
||
Comment on attachment 380799 [details] [diff] [review] updated comment Approved for 1.9.0.12, a=dveditz for release-drivers
Comment 12•15 years ago
|
||
Comment on attachment 380799 [details] [diff] [review] updated comment Approved for 1.8.1.23, a=dveditz for release-drivers
Attachment #380799 -
Flags: approval1.8.1.next? → approval1.8.1.next+
Comment 13•15 years ago
|
||
Checking in js/jsd/jsd_xpc.cpp; /cvsroot/mozilla/js/jsd/jsd_xpc.cpp,v <-- jsd_xpc.cpp new revision: 1.91; previous revision: 1.90
Keywords: fixed1.9.0.12
Comment 14•15 years ago
|
||
I assume that there is no real method to verify this fix?
Updated•15 years ago
|
Group: core-security
Comment 15•14 years ago
|
||
Checked for 1.8.1.24: Checking in js/jsd/jsd_xpc.cpp; /cvsroot/mozilla/js/jsd/jsd_xpc.cpp,v <-- jsd_xpc.cpp new revision: 1.72.2.3; previous revision: 1.72.2.2 done
Keywords: fixed1.8.1.24
Comment 16•14 years ago
|
||
(In reply to comment #14) > I assume that there is no real method to verify this fix? And now, almost eight months later, I ask again. Mark?
Updated•13 years ago
|
Component: JavaScript Debugging/Profiling APIs → JavaScript Engine
You need to log in
before you can comment on or make changes to this bug.
Description
•