Closed
Bug 987935
Opened 10 years ago
Closed 10 years ago
OOM: inlineScriptedCall() forgets to check TypeSet::clone() return
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: sstangl, Assigned: sstangl)
References
(Blocks 1 open bug)
Details
(Keywords: sec-other, Whiteboard: [adv-main31-])
Attachments
(1 file)
1.16 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
Simple thinko. Testcase from decoder (js__jit__MTypeBarrier__MTypeBarrier.txt). Probably not exploitable.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8396642 -
Flags: review?(nicolas.b.pierron)
Comment 3•10 years ago
|
||
Comment on attachment 8396642 [details] [diff] [review] patch Review of attachment 8396642 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonBuilder.cpp @@ +3919,5 @@ > types::StackTypeSet *types = types::TypeScript::ThisTypes(calleeScript); > if (!types->unknown()) { > + types::TemporaryTypeSet *clonedTypes = types->clone(alloc_->lifoAlloc()); > + if (!clonedTypes) > + return false; The default behaviour after returning false is preventing any re-compilation, in such case we might want to recompile and not disable future compilations. I think we want to define a function named bool IonBuilder::oom() { abortReason_ = AbortReason_Alloc return false; } and use this function instead. Otherwise we can open a follow-up bug later.
Attachment #8396642 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cb79b338122
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2cb79b338122
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox31:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 6•10 years ago
|
||
Do we need to worry about this on older releases at all?
Comment 7•10 years ago
|
||
Do we need to uplift this to ESR24?
status-firefox-esr24:
--- → ?
Flags: needinfo?(sstangl)
Comment 8•10 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] from comment #7) > Do we need to uplift this to ESR24? There is nothing to indicate that we need to here and non one has come forward to say otherwise. :-)
Updated•10 years ago
|
Flags: needinfo?(sstangl)
Updated•10 years ago
|
Assignee: nobody → sstangl
Comment 9•10 years ago
|
||
Sean, is there anything QA can do here to test this? Comment #0 is pretty vague.
Flags: needinfo?(sstangl)
Updated•10 years ago
|
Whiteboard: [adv-main31-]
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Kamil Jozwiak [:kjozwiak] from comment #9) > Sean, is there anything QA can do here to test this? Comment #0 is pretty > vague. The bug was a missing nullptr test. Doesn't need followup.
Flags: needinfo?(sstangl)
Updated•10 years ago
|
Flags: qe-verify-
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•