Closed Bug 930993 Opened 6 years ago Closed 6 years ago

(jit) Dangerous Crash on Heap with Typed Arrays and GC

Categories

(Core :: JavaScript Engine: JIT, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox26 --- unaffected
firefox27 + fixed
firefox28 + verified
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- ?
b2g-v1.2 --- ?

People

(Reporter: decoder, Assigned: bbouvier)

References

(Blocks 1 open bug)

Details

(Keywords: crash, sec-critical, testcase, Whiteboard: [jsbugmon:update,ignore])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 5a9ac6fed6ff (run with --fuzzing-safe --ion-eager):


x = {}; 
y = x;
x.toString = (function(heap) {
  Int8ArrayView = Int8Array(heap);
  Float32ArrayView = Float32Array(heap);
  function f() {
      Int8ArrayView[0] = Float32ArrayView[0]
  }
  return f;
})(ArrayBuffer);
gczeal(4);
function compareSAndSet() {
    return x << y;
}
evaluate("compareSAndSet()");
The reduced crash is at 0x0:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6bb0ff4 in ?? ()
#0  0x00007ffff6bb0ff4 in ?? ()
#1  0x00007ffff6bc0534 in ?? ()
#2  0x0000000000000183 in ?? ()
#3  0x00007ffff694c780 in ?? ()
#4  0x0000000000000000 in ?? ()
rax     0x0     0
rcx	0x0	0
=> 0x7ffff6bb0ff4:      mov    %cl,(%rax)


However, the original crash looked like this (same instruction, dangerous address):

Program terminated with signal 11, Segmentation fault.
#0  0x00007f79914c31c8 in ?? ()
#0  0x00007f79914c31c8 in ?? ()
#1  0x00007f79933e8574 in ?? ()
#2  0x0000000000000183 in ?? ()
#3  0x00007f797f103c40 in ?? ()
#4  0x0000000000000000 in ?? ()
rax	0xf586eda7	4602678818996940199
rcx	0x0	0
=> 0x7f79914c31c8:	mov    %cl,(%rax)


Marking s-s and sec-critical based on that.
Keywords: sec-critical
Whiteboard: [jsbugmon:update,bisect]
Adding Bouvier, since he recently landed the Float32 patches. Looking at Float32Array it could be related to that.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/77b72ea510d0
user:        Benjamin Bouvier
date:        Thu Jul 18 16:45:16 2013 -0700
summary:     Bug 913282: More Float32 operators: TruncateToInt32; p=dougc,bbouvier, r=jonco,jandem

This iteration took 421.576 seconds to run.
Benjamin can you take a look at this? :)
Flags: needinfo?(benj)
I will try to see what's going on.
Flags: needinfo?(benj)
Attached patch bug930993.patchSplinter Review
Very nice catch!

This occurs because stack order isn't preserved in the OOL patch of truncate to int32. The fix just consists in changing the order of stack operations (one line move).
Assignee: general → benj
Status: NEW → ASSIGNED
Attachment #823372 - Flags: review?(jdemooij)
Comment on attachment 823372 [details] [diff] [review]
bug930993.patch

Review of attachment 823372 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks and nice find!
Attachment #823372 - Flags: review?(jdemooij) → review+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 35b73bb96ca0).
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(In reply to Benjamin Bouvier [:bbouvier] from comment #9)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/101106bdce31

Can you please help request uplift approval on aurora to resolve this on FX 27. Looks like this is goon on central :)
Flags: needinfo?(benj)
Comment on attachment 823372 [details] [diff] [review]
bug930993.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 888109 (Float32 optimizations)
User impact if declined: correctness bugs + possible exploits on x86 and x64 machines
Testing completed (on m-c, etc.): testing completed on m-i, merged to and completed on m-c, all tests pass.
Risk to taking this patch (and alternatives if risky): no risk
String or IDL/UUID changes made by this patch: N/A
Attachment #823372 - Flags: approval-mozilla-aurora?
Flags: needinfo?(benj)
Attachment #823372 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Per comment 4, this bug was introduced post b2g18, though the timeline means b2g1.1+ may be affected.
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Marking verified for Firefox 28 as per comment 16.
Group: core-security
You need to log in before you can comment on or make changes to this bug.