Closed
Bug 541790
Opened 14 years ago
Closed 14 years ago
Too early check for rt->gcKeepAtoms in js_GC
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: igor, Assigned: igor)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
5.06 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
This is a spin-off of the bug 538275 comment 34. js_GC queries rt->gcKeepAtoms before it suspends all running requests. As such it may query the flag before another thread set it and then suspend its current requests, perhaps via calling the last-ditch GC or via calling js_InvokeOperationCallback. In theory this could be exploited with thread-workers to circumvent the GC safety. The worker can, for example, constantly run Object.prototype.toSource or other code that relies rt->gcKeepAtoms for GC safety and that periodically calls js_InvokeOperationCallback. In the mean time the main thread can try to trigger the last-ditch GC. But it is highly non-trivial to build a reliable exploit based on that.
Assignee | ||
Comment 1•14 years ago
|
||
The fix moves keepAtoms initialization after we know that all other requests are suspended. The patch also moved purge-related calls before the restart label. The purpose of the restart is to collect more garbage released during js_RemoveRoot and friends called from the finalizers, it cannot affect js_PurgeThreads or rt->builtinFunctions.
Attachment #423224 -
Flags: review?(jorendorff)
Assignee | ||
Comment 2•14 years ago
|
||
There is another problem related to keepAtoms. If the last ditch GC waits for a full GC running on another thread, that full GC would collect atoms violating the assumptions about the last-ditch GC. The new patch fixes that via calling JS_(KEEP|UNKEEP)_ATOMS around the waiting code.
Attachment #423224 -
Attachment is obsolete: true
Attachment #423231 -
Flags: review?(jorendorff)
Attachment #423224 -
Flags: review?(jorendorff)
Comment 3•14 years ago
|
||
Comment on attachment 423231 [details] [diff] [review] fix v2 Thanks for fixing this.
Attachment #423231 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 4•14 years ago
|
||
https://hg.mozilla.org/tracemonkey/rev/00b8d5937a94
Whiteboard: fixed-in-tracemonkey
Updated•14 years ago
|
blocking2.0: ? → alpha1
Looks like this landed on mozilla-central yesterday: https://hg.mozilla.org/mozilla-central/rev/00b8d5937a94
Marking this as fixed based on above comment to get it off the alpha blocker list.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•