Closed Bug 477598 Opened 15 years ago Closed 12 years ago

Make XPCWrappedJSClass::CallMethod use public rooting APIs

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Version:
Other Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: mrbkap)

Details

(Keywords: sec-other, Whiteboard: [sg:nse]) 

There's this comment in XPCWrappedJSClass::CallMethod:

http://hg.mozilla.org/mozilla-central/file/5f349409c9d5/js/src/xpconnect/src/xpcwrappedjsclass.cpp#l1259

  // We use js_AllocStack, js_Invoke, and js_FreeStack so that the gcthings
  // we use as args will be rooted by the engine as we do conversions and
  // prepare to do the function call. This adds a fair amount of complexity,
  // but is a good optimization compared to calling JS_AddRoot for each item.

I don't think this is correct anymore.  In jsgc.cpp it seems like js_TraceContext only traces those parts of the stack that are pointed-to by JSStackFrames.  The right thing here, I think, would be a JSAutoTempValueRooter.

Incidentally, I think making that change would also fix bug 476643.
Group: core-security
Whiteboard: [sg:critical?]
I think we should do this, but there's no GC hazard here. js_AllocStack adds an entry to cx->stackHeaders, which is scanned on trace.
Group: core-security
Whiteboard: [sg:critical?] → [sg:nse]
By "on trace," I mean "on GC."
Summary: Possible GC hazard in XPCWrappedJSClass::CallMethod → Make XPCWrappedJSClass::CallMethod use public rooting APIs
This happened over a series of patches a while back.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.