Assertion failure: data >> 28 != 0xf (The instruction does not have condition code), at js/src/jit/arm/Assembler-arm.h:1967

VERIFIED FIXED in Firefox 43

Status

()

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

People

(Reporter: decoder, Assigned: nbp)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla43
ARM
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox42 unaffected, firefox43 verified, firefox45 verified, firefox-esr38 unaffected, b2g-master fixed)

Details

(Whiteboard: [jsbugmon:update,bisect][fuzzblocker][b2g-adv-main2.5-][adv-main45+])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
The following testcase crashes on mozilla-central revision f61c3cc0eb8b (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-simulator=arm --enable-debug, run with --ion-eager --arm-hwcap=vfp --ion-offthread-compile=off --arm-asm-nop-fill=1 min.js):

gczeal(2);
for (lfLocal in this) {}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x086dd45a in js::jit::Instruction::extractCond (this=0xf7c40ed8) at js/src/jit/arm/Assembler-arm.h:1967
#0  0x086dd45a in js::jit::Instruction::extractCond (this=0xf7c40ed8) at js/src/jit/arm/Assembler-arm.h:1967
#1  0x086f6776 in extractCond (this=0xf7c40ed8) at js/src/jit/arm/Assembler-arm.cpp:3209
#2  InstIsGuard (ph=<synthetic pointer>, inst=0xf7c40ed8) at js/src/jit/arm/Assembler-arm.cpp:3112
#3  js::jit::Instruction::next (this=0xf7c40ed8) at js/src/jit/arm/Assembler-arm.cpp:3205
#4  0x08745a9b in next (this=0xffffbbd0) at js/src/jit/arm/Assembler-arm.h:2219
#5  js::jit::Assembler::GetCF32Target<js::jit::InstructionIterator> (iter=iter@entry=0xffffbbd0) at js/src/jit/arm/Assembler-arm.cpp:716
#6  0x086fba9e in CodeFromJump (jump=0xffffbbd0) at js/src/jit/arm/Assembler-arm.cpp:855
#7  js::jit::Assembler::TraceJumpRelocations (trc=trc@entry=0xf7a26ee4, code=code@entry=0xf4d5e8d0, reader=...) at js/src/jit/arm/Assembler-arm.cpp:865
#8  0x085a131e in js::jit::JitCode::traceChildren (this=this@entry=0xf4d5e8d0, trc=trc@entry=0xf7a26ee4) at js/src/jit/Ion.cpp:809
#9  0x084c49ff in js::GCMarker::processMarkStackTop (this=this@entry=0xf7a26ee4, budget=...) at js/src/gc/Marking.cpp:1349
#10 0x084a657c in js::GCMarker::drainMarkStack (this=this@entry=0xf7a26ee4, budget=...) at js/src/gc/Marking.cpp:1247
#11 0x087d81de in js::gc::GCRuntime::drainMarkStack (this=this@entry=0xf7a25210, sliceBudget=..., phase=phase@entry=js::gcstats::PHASE_MARK) at js/src/jsgc.cpp:5208
#12 0x088355b3 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0xf7a25210, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:5889
#13 0x0883659f in js::gc::GCRuntime::gcCycle (this=this@entry=0xf7a25210, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6110
#14 0x0883691b in js::gc::GCRuntime::collect (this=this@entry=0xf7a25210, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6239
#15 0x08836ca2 in js::gc::GCRuntime::gc (this=0xf7a25210, gckind=GC_NORMAL, reason=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6300
#16 0x088377b9 in js::gc::GCRuntime::runDebugGC (this=this@entry=0xf7a25210) at js/src/jsgc.cpp:6735
#17 0x0825e11b in js::gc::GCRuntime::gcIfNeededPerAllocation (this=0xf7a25210, cx=cx@entry=0xf7a03240) at js/src/gc/Allocator.cpp:28
#18 0x082a3016 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0xf7a25210, cx=0xf7a03240, kind=js::gc::FIRST) at js/src/gc/Allocator.cpp:55
#19 0x082ac802 in js::Allocate<JSObject, (js::AllowGC)1> (cx=cx@entry=0xf7a03240, kind=kind@entry=js::gc::FIRST, nDynamicSlots=0, heap=heap@entry=js::gc::TenuredHeap, clasp=clasp@entry=0x97cb060 <JSFunction::class_>) at js/src/gc/Allocator.cpp:121
#20 0x083bf7fc in JSObject::create (cx=0xf7a03240, kind=js::gc::FIRST, heap=js::gc::TenuredHeap, shape=..., group=...) at js/src/jsobjinlines.h:315
#21 0x087ec6f5 in NewObject (cx=0xf7a03240, group=..., kind=js::gc::FIRST, newKind=js::SingletonObject, initialShapeFlags=0) at js/src/jsobj.cpp:684
#22 0x087ed7a9 in js::NewObjectWithClassProtoCommon (cxArg=cxArg@entry=0xf7a03240, clasp=clasp@entry=0x97cb060 <JSFunction::class_>, protoArg=protoArg@entry=..., allocKind=allocKind@entry=js::gc::FIRST, newKind=newKind@entry=js::SingletonObject) at js/src/jsobj.cpp:817
#23 0x08841f77 in NewObjectWithClassProto (newKind=js::SingletonObject, allocKind=js::gc::FIRST, proto=..., clasp=0x97cb060 <JSFunction::class_>, cx=0xf7a03240) at js/src/jsobjinlines.h:707
#24 js::NewFunctionWithProto (cx=0xf7a03240, native=0x819d360 <dateTimeFormat_toSource(JSContext*, unsigned int, JS::Value*)>, nargs=0, flags=JSFunction::NATIVE_FUN, enclosingDynamicScope=..., atom=..., proto=..., allocKind=js::gc::FIRST, newKind=js::SingletonObject) at js/src/jsfun.cpp:2014
#25 0x0884458b in NewNativeFunction (newKind=js::GenericObject, allocKind=js::gc::FIRST, atom=..., nargs=0, native=0x819d360 <dateTimeFormat_toSource(JSContext*, unsigned int, JS::Value*)>, cx=0xf7a03240) at js/src/jsfun.cpp:1956
#26 js::DefineFunction (cx=cx@entry=0xf7a03240, obj=..., id=id@entry=..., native=0x819d360 <dateTimeFormat_toSource(JSContext*, unsigned int, JS::Value*)>, nargs=0, flags=0, flags@entry=512, allocKind=allocKind@entry=js::gc::FIRST, newKind=newKind@entry=js::GenericObject) at js/src/jsfun.cpp:2267
#27 0x087adfda in JS_DefineFunctions (cx=cx@entry=0xf7a03240, obj=obj@entry=..., fs=0x97a43f4 <dateTimeFormat_methods+20>, fs@entry=0x97a43e0 <dateTimeFormat_methods>, behavior=behavior@entry=DefineAllProperties) at js/src/jsapi.cpp:3640
#28 0x082211c7 in InitDateTimeFormatClass (global=..., Intl=..., cx=0xf7a03240) at js/src/builtin/Intl.cpp:1672
#29 js::InitIntlClass (cx=cx@entry=0xf7a03240, obj=obj@entry=...) at js/src/builtin/Intl.cpp:2089
#30 0x082fb33b in js::GlobalObject::resolveConstructor (cx=0xf7a03240, global=..., key=JSProto_Intl) at js/src/vm/GlobalObject.cpp:133
#31 0x082fb3da in js::GlobalObject::ensureConstructor (cx=cx@entry=0xf7a03240, global=..., global@entry=..., key=key@entry=JSProto_Intl) at js/src/vm/GlobalObject.cpp:100
#32 0x082fbe65 in js::GlobalObject::initStandardClasses (cx=cx@entry=0xf7a03240, global=global@entry=...) at js/src/vm/GlobalObject.cpp:340
#33 0x08791c1a in JS_EnumerateStandardClasses (cx=cx@entry=0xf7a03240, obj=obj@entry=...) at js/src/jsapi.cpp:1363
#34 0x080d23c4 in global_enumerate (cx=0xf7a03240, obj=...) at js/src/shell/js.cpp:5138
#35 0x0880a59f in Snapshot (cx=cx@entry=0xf7a03240, pobj_=..., pobj_@entry=..., flags=flags@entry=1, props=props@entry=0xffffca00) at js/src/jsiter.cpp:314
#36 0x0884389e in js::GetIterator (cx=cx@entry=0xf7a03240, obj=obj@entry=..., flags=flags@entry=1, objp=objp@entry=...) at js/src/jsiter.cpp:814
#37 0x0884d980 in js::ValueToIterator (cx=0xf7a03240, flags=1, vp=...) at js/src/jsiter.cpp:1089
#38 0x087aa764 in js::jit::Simulator::softwareInterrupt (this=0xf7a7e000, instr=0xf7a02e54) at js/src/jit/arm/Simulator-arm.cpp:2174
#39 0x087aae96 in js::jit::Simulator::decodeType7 (this=0xf7a7e000, instr=0xf7a02e54) at js/src/jit/arm/Simulator-arm.cpp:3280
#40 0x087a8f85 in js::jit::Simulator::instructionDecode (this=this@entry=0xf7a7e000, instr=instr@entry=0xf7a02e54) at js/src/jit/arm/Simulator-arm.cpp:4199
#41 0x087aca0c in execute<false> (this=0xf7a7e000) at js/src/jit/arm/Simulator-arm.cpp:4254
#42 js::jit::Simulator::callInternal (this=this@entry=0xf7a7e000, entry=entry@entry=0xf7c89308 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352\r\200\240\341\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>) at js/src/jit/arm/Simulator-arm.cpp:4342
#43 0x087ace91 in js::jit::Simulator::call (this=<optimized out>, entry=entry@entry=0xf7c89308 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352\r\200\240\341\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>, argument_count=<optimized out>, argument_count@entry=8) at js/src/jit/arm/Simulator-arm.cpp:4425
#44 0x084db459 in EnterBaseline (cx=cx@entry=0xf7a03240, data=...) at js/src/jit/BaselineJIT.cpp:125
#45 0x0850adcd in js::jit::EnterBaselineMethod (cx=cx@entry=0xf7a03240, state=...) at js/src/jit/BaselineJIT.cpp:157
#46 0x0831a140 in js::RunScript (cx=cx@entry=0xf7a03240, state=...) at js/src/vm/Interpreter.cpp:694
#47 0x0832cb4a in js::ExecuteKernel (cx=cx@entry=0xf7a03240, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:978
#48 0x0832cf74 in js::Execute (cx=cx@entry=0xf7a03240, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:1012
#49 0x087a5b27 in ExecuteScript (cx=cx@entry=0xf7a03240, scope=..., script=script@entry=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4353
#50 0x087a5d26 in JS_ExecuteScript (cx=cx@entry=0xf7a03240, scriptArg=scriptArg@entry=...) at js/src/jsapi.cpp:4384
#51 0x0806b611 in RunFile (compileOnly=false, file=0xf7aea9e0, filename=0xffffdaa2 "min.js", cx=0xf7a03240) at js/src/shell/js.cpp:460
#52 Process (cx=cx@entry=0xf7a03240, filename=0xffffdaa2 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:578
#53 0x080cdb50 in ProcessArgs (op=0xffffd770, cx=<optimized out>) at js/src/shell/js.cpp:5831
#54 Shell (envp=<optimized out>, op=0xffffd770, cx=<optimized out>) at js/src/shell/js.cpp:6109
#55 main (argc=6, argv=0xffffd8c4, envp=0xffffd8e0) at js/src/shell/js.cpp:6455
eax	0x0	0
ebx	0x9794434	158942260
ecx	0xf7e4388c	-136038260
edx	0x0	0
esi	0xf	15
edi	0xf7c40eac	-138146132
ebp	0xffffbb18	4294949656
esp	0xffffbb00	4294949632
eip	0x86dd45a <js::jit::Instruction::extractCond()+42>
=> 0x86dd45a <js::jit::Instruction::extractCond()+42>:	movl   $0x7af,0x0
   0x86dd464 <js::jit::Instruction::extractCond()+52>:	call   0x80f2560 <abort()>


Happens quite frequently, so marking as fuzzblocker. Also involves GC and assertions about instructions generally sound like they could be security-related, so marking s-s as well.
(Assignee)

Comment 1

4 years ago
Hurray \o/

We trade an undefined behaviour with a critical SEGV!
Flags: needinfo?(nicolas.b.pierron)
(Assignee)

Comment 2

4 years ago
This issue is likely caused by Bug 1195263.
(Assignee)

Updated

4 years ago
Assignee: nobody → nicolas.b.pierron
(Assignee)

Comment 3

4 years ago
I was unable to reproduce the signature reported in comment 0, but I managed to find one unsupported case of Bug 986680, with the previous test case.

Decoder, can you still reproduce this issue?
Flags: needinfo?(choller)
Keywords: sec-high

Updated

4 years ago
Group: core-security → javascript-core-security
(Assignee)

Comment 4

4 years ago
This patch is needed to mute a lot of error caused by

    movPtr(..., scratch);

which relies on ma_alu.  This patch verify that none of the sources alias
the scratch register, and try to take the ownership if the destination does
not register either.
Attachment #8656535 - Flags: review?(sstangl)
Comment on attachment 8656535 [details] [diff] [review]
part 0 - ScratchRegisterScope: ma_alu expect to have scratch register owned by the caller.

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

::: js/src/jit/arm/MacroAssembler-arm.cpp
@@ +413,5 @@
> +    if (dest != scratch) {
> +        // If the destination register is not the scratch register, double check
> +        // that the current function does not erase the content of the scratch
> +        // register.
> +        ScratchRegisterScope assertScratch(asMasm());

It doesn't look like there is any value gained from having a Scope here. I would just ignore this part and hardcode scratch = ScratchRegister, keeping the comment above.

This type of situation in the future can be resolved by that "hand-off" scope we talked about on IRC a week ago.
Attachment #8656535 - Flags: review?(sstangl) → review+
(Assignee)

Comment 6

4 years ago
(In reply to Sean Stangl [:sstangl] from comment #5)
> ::: js/src/jit/arm/MacroAssembler-arm.cpp
> @@ +413,5 @@
> > +    if (dest != scratch) {
> > +        // If the destination register is not the scratch register, double check
> > +        // that the current function does not erase the content of the scratch
> > +        // register.
> > +        ScratchRegisterScope assertScratch(asMasm());
> 
> It doesn't look like there is any value gained from having a Scope here. I
> would just ignore this part and hardcode scratch = ScratchRegister, keeping
> the comment above.
> 
> This type of situation in the future can be resolved by that "hand-off"
> scope we talked about on IRC a week ago.

There is value to keep this assertion for the following reasons:

  ScratchRegisterScope scratch;
  masm.movPtr(…, scratch);  // We expect to have a valid value in scratch, dest == scratch
                            // so we know that earsing scratch is expected by the caller anyway.
  masm.movPtr(…, r1);  // The following instruction implies that we expect the scratch register
                       // to survive, but this instruction erases it to compute the r1 register.
                       //
                       // Having a ScratchRegisterScope if dest != scratch should catch this issue.
  masm.andPtr(scratch, r1);
Flags: needinfo?(nicolas.b.pierron)
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1201793
(Assignee)

Comment 8

4 years ago
Comment on attachment 8656535 [details] [diff] [review]
part 0 - ScratchRegisterScope: ma_alu expect to have scratch register owned by the caller.

I will re-attach this patch on Bug 1201793, and push it, as it fits the topic better, and I still want to leave the assertion of the current bug under investigation.
(Assignee)

Updated

4 years ago
Attachment #8656535 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Depends on: 1201793
(Assignee)

Comment 9

4 years ago
I checked based on revision f61c3cc0eb8b, but I am still unable to reproduce it.
Do you happen to have other test cases?
(Reporter)

Comment 10

4 years ago
This is an automated crash issue comment:

Summary: Assertion failure: data >> 28 != 0xf (The instruction does not have condition code), at js/src/jit/arm/Assembler-arm.h:1989
Build version: mozilla-central revision 7f987c38bd3e
Build flags: --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-simulator=arm --enable-debug
Runtime options: --arm-asm-nop-fill=1 --ion-check-range-analysis

Testcase:

var msPerDay = 86400000;
function DaysInYear(y){
  if(y%4 != 0) return 365;
  if(y%100 == 0 && y%400 != 0) return 365;
  if(y%400 == 0) return 366;
}
    for ( var time = 0, year = 1969; year >= 0; year-- ) {
      time -= TimeInYear(year);
    }
function TimeInYear( y ) {
  return ( DaysInYear(y) * msPerDay );
}
        gczeal(2);
lfcode.push(`
`);


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x086df29a in js::jit::Instruction::extractCond (this=0xf7c82db4) at js/src/jit/arm/Assembler-arm.h:1989
#0  0x086df29a in js::jit::Instruction::extractCond (this=0xf7c82db4) at js/src/jit/arm/Assembler-arm.h:1989
#1  0x086fca86 in extractCond (this=0xf7c82db4) at js/src/jit/arm/Assembler-arm.cpp:3209
#2  InstIsGuard (ph=<synthetic pointer>, inst=0xf7c82db4) at js/src/jit/arm/Assembler-arm.cpp:3112
#3  js::jit::Instruction::next (this=0xf7c82db4) at js/src/jit/arm/Assembler-arm.cpp:3205
#4  0x087385eb in next (this=0xffffb830) at js/src/jit/arm/Assembler-arm.h:2241
#5  js::jit::Assembler::GetCF32Target<js::jit::InstructionIterator> (iter=iter@entry=0xffffb830) at js/src/jit/arm/Assembler-arm.cpp:716
#6  0x08703b6e in CodeFromJump (jump=0xffffb830) at js/src/jit/arm/Assembler-arm.cpp:855
#7  js::jit::Assembler::TraceJumpRelocations (trc=trc@entry=0xf7a26f2c, code=code@entry=0xf4d5e510, reader=...) at js/src/jit/arm/Assembler-arm.cpp:865
#8  0x085a5de7 in js::jit::JitCode::traceChildren (this=this@entry=0xf4d5e510, trc=trc@entry=0xf7a26f2c) at js/src/jit/Ion.cpp:820
#9  0x084ca84f in js::GCMarker::processMarkStackTop (this=this@entry=0xf7a26f2c, budget=...) at js/src/gc/Marking.cpp:1368
#10 0x084aba5c in js::GCMarker::drainMarkStack (this=this@entry=0xf7a26f2c, budget=...) at js/src/gc/Marking.cpp:1266
#11 0x087def0e in js::gc::GCRuntime::drainMarkStack (this=this@entry=0xf7a25210, sliceBudget=..., phase=phase@entry=js::gcstats::PHASE_MARK) at js/src/jsgc.cpp:5207
#12 0x08840b73 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0xf7a25210, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:5888
#13 0x08841bb2 in js::gc::GCRuntime::gcCycle (this=this@entry=0xf7a25210, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6109
#14 0x08841f5b in js::gc::GCRuntime::collect (this=this@entry=0xf7a25210, incremental=incremental@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6238
#15 0x088422e2 in js::gc::GCRuntime::gc (this=0xf7a25210, gckind=GC_NORMAL, reason=JS::gcreason::DEBUG_GC) at js/src/jsgc.cpp:6299
#16 0x08842df9 in js::gc::GCRuntime::runDebugGC (this=this@entry=0xf7a25210) at js/src/jsgc.cpp:6740
#17 0x0825f7bb in js::gc::GCRuntime::gcIfNeededPerAllocation (this=0xf7a25210, cx=cx@entry=0xf7a03240) at js/src/gc/Allocator.cpp:28
#18 0x082a5f46 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=0xf7a25210, cx=0xf7a03240, kind=js::gc::SHAPE) at js/src/gc/Allocator.cpp:55
#19 0x082abcfc in js::Allocate<js::Shape, (js::AllowGC)1> (cx=cx@entry=0xf7a03240) at js/src/gc/Allocator.cpp:211
#20 0x0882906b in new_ (nfixed=<optimized out>, other=..., cx=0xf7a03240) at js/src/vm/Shape-inl.h:86
#21 js::PropertyTree::getChild (this=0xf7a2e5c0, cx=cx@entry=0xf7a03240, parentArg=parentArg@entry=0xf4d60250, child=..., child@entry=...) at js/src/jspropertytree.cpp:184
#22 0x0838af41 in js::NativeObject::getChildProperty (cx=cx@entry=0xf7a03240, obj=obj@entry=..., parent=parent@entry=..., child=child@entry=...) at js/src/vm/Shape.cpp:409
#23 0x083a09be in js::NativeObject::addPropertyInternal (cx=cx@entry=0xf7a03240, obj=obj@entry=..., id=id@entry=..., getter=getter@entry=0x0, setter=setter@entry=0x0, slot=slot@entry=4, attrs=attrs@entry=0, flags=flags@entry=0, entry=0x0, allowDictionary=allowDictionary@entry=true) at js/src/vm/Shape.cpp:581
#24 0x083a1321 in js::NativeObject::addProperty (cx=cx@entry=0xf7a03240, obj=obj@entry=..., id=id@entry=..., getter=getter@entry=0x0, setter=setter@entry=0x0, slot=slot@entry=4, attrs=attrs@entry=0, flags=flags@entry=0, allowDictionary=allowDictionary@entry=true) at js/src/vm/Shape.cpp:499
#25 0x082d3573 in js::NativeObject::addDataProperty (this=0xf4d4f150, cx=cx@entry=0xf7a03240, name=..., slot=slot@entry=4, attrs=attrs@entry=0) at js/src/vm/NativeObject.cpp:963
#26 0x082d368b in js::ErrorObject::assignInitialShape (cx=cx@entry=0xf7a03240, obj=obj@entry=...) at js/src/vm/ErrorObject.cpp:30
#27 0x08306736 in ensureInitialCustomShape<js::ErrorObject> (obj=..., cx=0xf7a03240) at js/src/vm/Shape-inl.h:113
#28 js::ErrorObject::init (cx=cx@entry=0xf7a03240, obj=obj@entry=..., type=type@entry=JSEXN_REFERENCEERR, errorReport=errorReport@entry=0x0, fileName=fileName@entry=..., stack=stack@entry=..., lineNumber=lineNumber@entry=0, columnNumber=columnNumber@entry=0, message=message@entry=...) at js/src/vm/ErrorObject.cpp:47
#29 0x087f1d45 in js::ErrorObject::createProto (cx=0xf7a03240, key=JSProto_ReferenceError) at js/src/jsexn.cpp:492
#30 0x082ff35c in js::GlobalObject::resolveConstructor (cx=0xf7a03240, global=..., key=JSProto_ReferenceError) at js/src/vm/GlobalObject.cpp:160
#31 0x082ff9aa in js::GlobalObject::ensureConstructor (cx=cx@entry=0xf7a03240, global=..., global@entry=..., key=key@entry=JSProto_ReferenceError) at js/src/vm/GlobalObject.cpp:100
#32 0x08306b06 in getOrCreateCustomErrorPrototype (exnType=JSEXN_REFERENCEERR, global=..., cx=0xf7a03240) at js/src/vm/GlobalObject.h:417
#33 js::ErrorObject::create (cx=cx@entry=0xf7a03240, errorType=errorType@entry=JSEXN_REFERENCEERR, stack=stack@entry=..., fileName=fileName@entry=..., lineNumber=lineNumber@entry=14, columnNumber=columnNumber@entry=1, report=report@entry=0xffffc308, message=message@entry=...) at js/src/vm/ErrorObject.cpp:91
#34 0x087f763c in js::ErrorToException (cx=cx@entry=0xf7a03240, message=message@entry=0xf7a904a0 "lfcode is not defined", reportp=reportp@entry=0xffffc410, callback=<optimized out>, userRef=userRef@entry=0x0) at js/src/jsexn.cpp:582
#35 0x08770e5b in ReportError (cx=cx@entry=0xf7a03240, message=<optimized out>, reportp=reportp@entry=0xffffc410, callback=callback@entry=0x873f430 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0) at js/src/jscntxt.cpp:230
#36 0x08788004 in js::ReportErrorNumberVA (cx=0xf7a03240, flags=flags@entry=0, callback=callback@entry=0x873f430 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1, argumentsType=argumentsType@entry=js::ArgumentsAreASCII, ap=ap@entry=0xffffc4e0 "\250\222\241", <incomplete sequence \367>) at js/src/jscntxt.cpp:751
#37 0x087880b5 in JS_ReportErrorNumberVA (cx=cx@entry=0xf7a03240, errorCallback=errorCallback@entry=0x873f430 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1, ap=ap@entry=0xffffc4e0 "\250\222\241", <incomplete sequence \367>) at js/src/jsapi.cpp:5366
#38 0x087880f9 in JS_ReportErrorNumber (cx=cx@entry=0xf7a03240, errorCallback=errorCallback@entry=0x873f430 <js::GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=1) at js/src/jsapi.cpp:5355
#39 0x08788871 in js::ReportIsNotDefined (cx=cx@entry=0xf7a03240, id=id@entry=...) at js/src/jscntxt.cpp:829
#40 0x087889d8 in js::ReportIsNotDefined (cx=cx@entry=0xf7a03240, name=name@entry=...) at js/src/jscntxt.cpp:837
#41 0x08323a74 in js::GetScopeName (cx=cx@entry=0xf7a03240, scopeChain=scopeChain@entry=..., name=name@entry=..., vp=vp@entry=...) at js/src/vm/Interpreter.cpp:4236
#42 0x0855c7e8 in js::jit::DoGetNameFallback (cx=cx@entry=0xf7a03240, frame=frame@entry=0xf4ffff00, stub_=stub_@entry=0xf7a1d470, scopeChain=scopeChain@entry=..., res=res@entry=...) at js/src/jit/BaselineIC.cpp:5618
#43 0x087ad0db in js::jit::Simulator::softwareInterrupt (this=0xf7a7e000, instr=0xf7a02f04) at js/src/jit/arm/Simulator-arm.cpp:2174
#44 0x087ad806 in js::jit::Simulator::decodeType7 (this=0xf7a7e000, instr=0xf7a02f04) at js/src/jit/arm/Simulator-arm.cpp:3280
#45 0x087ab925 in js::jit::Simulator::instructionDecode (this=this@entry=0xf7a7e000, instr=instr@entry=0xf7a02f04) at js/src/jit/arm/Simulator-arm.cpp:4199
#46 0x087af37c in execute<false> (this=0xf7a7e000) at js/src/jit/arm/Simulator-arm.cpp:4254
#47 js::jit::Simulator::callInternal (this=this@entry=0xf7a7e000, entry=entry@entry=0xf7c89340 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352\r\200\240\341\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>) at js/src/jit/arm/Simulator-arm.cpp:4342
#48 0x087af801 in js::jit::Simulator::call (this=<optimized out>, entry=entry@entry=0xf7c89340 "\377\377\377\352\360O-\351\377\377\377\352\004\320M\342\377\377\377\352\020\212-\355\377\377\377\352\r\200\240\341\377\377\377\352h\220\235\345\377\377\377\352\r\260\240\341\377\377\377\352t\240\235\345\377\377\377", <incomplete sequence \352>, argument_count=<optimized out>, argument_count@entry=8) at js/src/jit/arm/Simulator-arm.cpp:4425
#49 0x084e03a9 in EnterBaseline (cx=cx@entry=0xf7a03240, data=...) at js/src/jit/BaselineJIT.cpp:125
#50 0x0855057f in js::jit::EnterBaselineAtBranch (cx=0xf7a03240, fp=0xf4cc2028, pc=0xf7af4d0d "ず") at js/src/jit/BaselineJIT.cpp:229
#51 0x0831dab7 in Interpret (cx=cx@entry=0xf7a03240, state=...) at js/src/vm/Interpreter.cpp:2115
#52 0x0831e641 in js::RunScript (cx=cx@entry=0xf7a03240, state=...) at js/src/vm/Interpreter.cpp:704
#53 0x0833111a in js::ExecuteKernel (cx=cx@entry=0xf7a03240, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:978
#54 0x08331544 in js::Execute (cx=cx@entry=0xf7a03240, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:1012
#55 0x0877bb77 in ExecuteScript (cx=cx@entry=0xf7a03240, scope=..., script=script@entry=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4356
#56 0x0877bd66 in JS_ExecuteScript (cx=cx@entry=0xf7a03240, scriptArg=scriptArg@entry=...) at js/src/jsapi.cpp:4387
#57 0x0806b6c1 in RunFile (compileOnly=false, file=0xf7aea9e0, filename=0xffffdaa1 "min.js", cx=0xf7a03240) at js/src/shell/js.cpp:461
#58 Process (cx=cx@entry=0xf7a03240, filename=0xffffdaa1 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:579
#59 0x080cf030 in ProcessArgs (op=0xffffd7a0, cx=<optimized out>) at js/src/shell/js.cpp:5832
#60 Shell (envp=<optimized out>, op=0xffffd7a0, cx=<optimized out>) at js/src/shell/js.cpp:6121
#61 main (argc=4, argv=0xffffd8f4, envp=0xffffd908) at js/src/shell/js.cpp:6470
eax	0x0	0
ebx	0x97a5434	159011892
ecx	0xf7e4388c	-136038260
edx	0x0	0
esi	0xf	15
edi	0xf6210000	-165609472
ebp	0xffffb778	4294948728
esp	0xffffb760	4294948704
eip	0x86df29a <js::jit::Instruction::extractCond()+42>
=> 0x86df29a <js::jit::Instruction::extractCond()+42>:	movl   $0x7c5,0x0
   0x86df2a4 <js::jit::Instruction::extractCond()+52>:	call   0x80f3a90 <abort()>
(Reporter)

Comment 11

4 years ago
I have not checked yet with shells after the landing of bug 1201793 because these haven't been compiled yet by the fuzzing infrastructure.
Flags: needinfo?(choller)
(Assignee)

Comment 12

4 years ago
I am able to reproduce this issue with the test case provided in comment 10.
(Assignee)

Comment 13

4 years ago
The problem seems to be during the interpretation of the instructions in Assembler::GetCF32Target.

   f7610da4  e306cf18       movw ip, #28440
   f7610dac  e34fc761       movt ip, #63329
   f7610db4  e12fff1c       bx ip
   f7610db8  f621f261       unknown

We are decoding the 4 instructions ahead of time, including the 4th one, before recognizing the pattern.  I think we should change GetCF32Target to decode each instruction as needed instead of potentially reading unknown (?) memory.

As the pattern is recognized, the 4th instruction content is ignored.  Thus, this issue does not seems to be exploitable as we are not supposed to write anything on the generated code while we are trying to decode a jump address.
(Assignee)

Comment 14

4 years ago
This patch copy the spew function and make it static, in order to be able to call it after the code generation within gdb.
Attachment #8657903 - Flags: review?(lhansen)
(Assignee)

Comment 15

4 years ago
This patch does what is explained in comment 13, which is to avoid decoding
instructions if this is not necessary for decoding the address, or to assert
that the pattern is correct.
Attachment #8657906 - Flags: review?(hv1989)
Attachment #8657903 - Flags: review?(lhansen) → review+
BTW note my annotation in bug 1201793 that the problems with ScratchRegisterScope appear to be not-fixed by the patch on that bug.
Comment on attachment 8657906 [details] [diff] [review]
part 1 - Assembler::GetCF32Target: Decode instructions as needed.

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

Looks good
Attachment #8657906 - Flags: review?(hv1989) → review+
(Assignee)

Comment 18

4 years ago
Comment on attachment 8657906 [details] [diff] [review]
part 1 - Assembler::GetCF32Target: Decode instructions as needed.

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?

I do not think this is exploitable.  In the worse case this can cause crashes by reading random memory, but at no time this memory reads can be used to change the intent of the code.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No.

> Which older supported branches are affected by this flaw?

All.

> If not all supported branches, which bug introduced the flaw?

Bug 760457

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

The current patch should apply cleanly on all branches. (if we ever decide to backport it)

> How likely is this patch to cause regressions; how much testing does it need?

This patch is unlikely to add regression.  It effectively remove code in optimized build by moving code which is only used by assertions under debug sections.
Attachment #8657906 - Flags: sec-approval?
(Assignee)

Updated

4 years ago
Keywords: sec-highsec-low
Comment on attachment 8657906 [details] [diff] [review]
part 1 - Assembler::GetCF32Target: Decode instructions as needed.

Clearing sec-approval as it is no longer needed for this issue for checkin as a sec-low.
Attachment #8657906 - Flags: sec-approval?
https://hg.mozilla.org/mozilla-central/rev/6cb745339f3c
https://hg.mozilla.org/mozilla-central/rev/b2663d8fb86a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Group: javascript-core-security → core-security-release
Group: core-security-release
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [jsbugmon:update,bisect][fuzzblocker][b2g-adv-main2.5-]

Updated

3 years ago
Status: RESOLVED → VERIFIED

Comment 22

3 years ago
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx43
Whiteboard: [jsbugmon:update,bisect][fuzzblocker][b2g-adv-main2.5-] → [jsbugmon:update,bisect][fuzzblocker][b2g-adv-main2.5-][adv-main45+]
You need to log in before you can comment on or make changes to this bug.