Assertion failure: JSVAL_IS_DOUBLE_IMPL(l), at js/Value.h:406 or Assertion failure: l.asBits <= JSVAL_SHIFTED_TAG_MAX_DOUBLE, at js/Value.h:636 or Crash [@ JSObject::getGeneric] or Crash [@ getClass] with SIMD

VERIFIED FIXED in Firefox 29

Status

()

defect
--
critical
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: decoder, Assigned: nmatsakis)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla29
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox27 unaffected, firefox28 unaffected, firefox29 fixed, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.2 unaffected, b2g-v1.3 unaffected)

Details

(Whiteboard: [jsbugmon:update], crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

The following testcase asserts on mozilla-central revision 451f47a70238 (run with --fuzzing-safe):


var int32x4 = SIMD.int32x4;
var a = int32x4((4294967295), 200, 300, 400);
var c = SIMD.int32x4.bitsToFloat32x4(a);
assertEq(c.x, 1.401298464324817e-43);
The crash address is partially controllable, here's a 64 bit trace:


Program received signal SIGSEGV, Segmentation fault.
getClass (this=0x7fffe0000000) at js/src/gc/Barrier.cpp:60
60      } // namespace js
#0  getClass (this=0x7fffe0000000) at js/src/gc/Barrier.cpp:60
#1  getOps (this=0x7fffe0000000) at js/src/vm/ObjectImpl.h:1286
#2  JSObject::getGeneric (cx=0x13c08f0, obj=..., receiver=..., id=..., vp=...) at js/src/jsobj.h:1002
#3  0x00000000006e0f6a in getProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=0x13c08f0) at js/src/jsobj.h:1026
#4  js::ValueToSource (cx=0x13c08f0, v=...) at js/src/jsstr.cpp:4059
#5  0x00000000006539d0 in JS_ValueToSource (cx=<optimized out>, valueArg=...) at js/src/jsapi.cpp:419
#6  0x000000000040cdac in ToSource (bytes=<synthetic pointer>, vp=..., cx=0x13c08f0) at js/src/shell/js.cpp:1370
#7  AssertEq (cx=0x13c08f0, argc=2, vp=0x1461520) at js/src/shell/js.cpp:1400
rax     0xe0000000      140736951484416
=> 0x47bd57 <JSObject::getGeneric(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)+7>: mov    0x8(%rax),%rax


The 0xe0 byte in 0x7fffe0000000 is controllable by the first argument of int32x4. It might be possible to control it even further. I assume there's some integer overflow/truncation going on somewhere. If one manages to control this address, one controls an object, so I assume sec-critical.
Crash Signature: [@ JSObject::getGeneric] [@ getClass]
Keywords: crash, sec-critical
Whiteboard: [jsbugmon:update,bisect]
Flags: needinfo?(ivan)
Crash Signature: [@ JSObject::getGeneric] [@ getClass] → [@ JSObject::getGeneric] [@ getClass]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, failed due to error (try manually).
No idea why this failed, but here's the bisection range:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/abdeb55e8a16
user:        Ivan Jibaja
date:        Wed Dec 18 16:28:32 2013 -0500
summary:     Bug 946042 - Add all SIMD functions to the interpreter. r=till
Blocks: 946042
Crash Signature: [@ JSObject::getGeneric] [@ getClass] → [@ JSObject::getGeneric] [@ getClass]
Also needinfo from Till to get this bug going :)
Flags: needinfo?(till)
Till, can you also answer if this feature is enabled in the browser nightly builds already now? I can see a lot of changes have been made to code that are not ifdef'ed but some stuff is behind an ifdef (for which I don't know if it's enabled in Nightly or not). Thanks!
Forwarding to Niko, as he knows way more about this than I do.

(In reply to Christian Holler (:decoder) from comment #6)
> Till, can you also answer if this feature is enabled in the browser nightly
> builds already now? I can see a lot of changes have been made to code that
> are not ifdef'ed but some stuff is behind an ifdef (for which I don't know
> if it's enabled in Nightly or not). Thanks!

SIMD is enabled in Nightlies, yes. So having this marked as sec-critical until proven to be harmless makes sense.
Flags: needinfo?(till)
Flags: needinfo?(nmatsakis)
Posted patch Bug953270.diff (obsolete) — Splinter Review
Canonicalize NaN as part of the lane accessor.
Attachment #8357050 - Flags: review?(till)
Flags: needinfo?(nmatsakis)
Assignee: nobody → nmatsakis
Comment on attachment 8357050 [details] [diff] [review]
Bug953270.diff

Ideally, this would actually be a patch ;)
Attachment #8357050 - Flags: review?(till)
Flags: needinfo?(ivan)
This time with actual patch.
Attachment #8357050 - Attachment is obsolete: true
Attachment #8359305 - Flags: review?(till)
Comment on attachment 8359305 [details] [diff] [review]
Bug953270.diff

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

r=me with the builtin/SIMD.h include reverted to its old position, and the commit message's format fixed.

::: js/src/builtin/SIMD.cpp
@@ +14,4 @@
>  #include "jsapi.h"
>  #include "jsfriendapi.h"
> +
> +#include "builtin/SIMD.h"

The cpp file's own header is always first, to prevent it from accidentally depending on not-included other headers.
Attachment #8359305 - Flags: review?(till) → review+
> The cpp file's own header is always first, to prevent it from accidentally
> depending on not-included other headers.

Oh, right, forgot. Here I was trying to be a good boy and enforcing SM rules. ;)
https://hg.mozilla.org/mozilla-central/rev/4d982bf3158d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Status: RESOLVED → VERIFIED
Crash Signature: [@ JSObject::getGeneric] [@ getClass] → [@ JSObject::getGeneric] [@ getClass]
JSBugMon: This bug has been automatically verified fixed.
Crash Signature: [@ JSObject::getGeneric] [@ getClass] → [@ JSObject::getGeneric] [@ getClass]
Keywords: regression
Group: core-security
You need to log in before you can comment on or make changes to this bug.