Closed Bug 1130972 Opened 5 years ago Closed 5 years ago

EXC_BAD_ACCESS in Load<js::Float32x4, 4u> in SIMD.cpp when executing ecma_7/SIMD/load.js on ggc opt/debug build.

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox37 --- disabled
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: arai, Assigned: bbouvier)

Details

(Keywords: csectype-uaf, sec-high)

Attachments

(1 file)

Running jstests.py on try server hits TEST-UNEXPECTED-FAIL in ecma_7/SIMD/load.js on ggc build.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8cd0e99b4a17

configure parameter: --disabled-optimize --enable-debug --enable-stdcxx-compat --enable-ctypes --enable-trace-malloc --disable-shared-js --enable-exact-rooting --enable-gccompacting --enable-threadsafe --with-ccache --enable-nspr-build
environment variable: JS_GC_ZEAL=14

(lldb) env JS_GC_ZEAL=14
(lldb) run -f shell.js -f ecma_7/shell.js -f ecma_7/SIMD/shell.js -f ecma_7/SIMD/load.js
Process 73544 stopped
* thread #1: tid = 0xb9f5a0, 0x0000000100162ee9 js`bool Load<js::Int32x4, 4u>(cx=0x0000000102e09de0, argc=2, vp=0x00007fff5fbfb938) + 553 at SIMD.cpp:1082, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x10576daf0)
    frame #0: 0x0000000100162ee9 js`bool Load<js::Int32x4, 4u>(cx=0x0000000102e09de0, argc=2, vp=0x00007fff5fbfb938) + 553 at SIMD.cpp:1082
   1079	        return false;
   1080	
   1081	    Elem *dest = reinterpret_cast<Elem*>(result->typedMem());
-> 1082	    memcpy(dest, typedArrayData, sizeof(Elem) * NumElem);
   1083	
   1084	    args.rval().setObject(*result);
   1085	    return true;
(lldb) bt
* thread #1: tid = 0xb9f5a0, 0x0000000100162ee9 js`bool Load<js::Int32x4, 4u>(cx=0x0000000102e09de0, argc=2, vp=0x00007fff5fbfb938) + 553 at SIMD.cpp:1082, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x10576daf0)
  * frame #0: 0x0000000100162ee9 js`bool Load<js::Int32x4, 4u>(cx=0x0000000102e09de0, argc=2, vp=0x00007fff5fbfb938) + 553 at SIMD.cpp:1082
    frame #1: 0x0000000100162cb3 js`js::simd_int32x4_load(cx=0x0000000102e09de0, argc=2, vp=0x00007fff5fbfb938) + 35 at SIMD.cpp:1136
    frame #2: 0x000000010037ed14 js`js::CallJSNative(cx=0x0000000102e09de0, native=0x0000000100162c90, args=0x00007fff5fbfb7d0)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 164 at jscntxtinlines.h:226
    frame #3: 0x000000010030947d js`js::Invoke(cx=0x0000000102e09de0, args=CallArgs at 0x00007fff5fbfb7d0, construct=NO_CONSTRUCT) + 1261 at Interpreter.cpp:492
    frame #4: 0x00000001002f2594 js`js::Invoke(cx=0x0000000102e09de0, thisv=0x00007fff5fbfbbd0, fval=0x00007fff5fbfbc00, argc=2, argv=0x00007fff5fbfbd60, rval=JS::MutableHandleValue at 0x00007fff5fbfb8d0) + 900 at Interpreter.cpp:548
    frame #5: 0x000000010060c891 js`js::jit::DoCallFallback(cx=0x0000000102e09de0, frame=0x00007fff5fbfbdd8, stub_=0x0000000104209c18, argc=2, vp=0x00007fff5fbfbd50, res=JS::MutableHandleValue at 0x00007fff5fbfbcb8) + 1921 at BaselineIC.cpp:9572
    frame #6: 0x00000001041f5d0b

I guess it's similar issue as bug 1129416.

marking as security, just in case.
Wow, thanks for noticing and reporting, I'll try to find what the issue is.
Flags: needinfo?(benj)
In SIMD Load, we create the return value after getting the typed array address
from which we want to load the value.  This is wrong, as the address can
change, if there's a GC in between the two instructions.  This fixes it by
getting the address at the last minute (after creating the return value).
Unfortunately, it introduces some boilerplate in Store as well (which uses the
same function), but it doesn't seem to be worth having another function just
for retrieving Store arguments and getting the typed array address...
Attachment #8562741 - Flags: review?(jcoppeard)
Assignee: nobody → benj
Status: NEW → ASSIGNED
Flags: needinfo?(benj)
Comment on attachment 8562741 [details] [diff] [review]
Fix rooting hazards in SIMD load/store interpreter methods

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

Yes, looks good.
Attachment #8562741 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/mozilla-central/rev/4025bc064621
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.