Closed Bug 1204864 Opened 4 years ago Closed 4 years ago

Assertion failure: IsValidAsmJSHeapLength(mask + 1), at js/src/asmjs/AsmJSModule.h:1132

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: decoder, Assigned: luke)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 9ed17db42e3e (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2):

function foo(glob, s, b) {
    "use asm";
    var I32 = glob.Int32Array;
    var i32 = new I32(b);
    var len = glob.byteLength;
    function ch(b2) {
        if (len(b2) & 4294967295 || len(b2) <= 0xffffff || len(b2) > 80000000) {
            return false;
        }
        i32 = new I32(b2);
        b = b2;
        return true
    }
} foo();



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000005cd8c1 in js::AsmJSModule::addChangeHeap (this=0x7ffff47fa000, mask=4294967295, min=min@entry=16777216, max=<optimized out>) at js/src/asmjs/AsmJSModule.h:1132
#0  0x00000000005cd8c1 in js::AsmJSModule::addChangeHeap (this=0x7ffff47fa000, mask=4294967295, min=min@entry=16777216, max=<optimized out>) at js/src/asmjs/AsmJSModule.h:1132
#1  0x000000000043cf12 in addChangeHeap (fn=<optimized out>, fn=<optimized out>, max=<optimized out>, min=16777216, mask=<optimized out>, name=0x7ffff7e062b0, this=0x7fffffffb3b0) at js/src/asmjs/AsmJSValidate.cpp:1980
#2  CheckChangeHeap (validated=<synthetic pointer>, fn=<optimized out>, m=...) at js/src/asmjs/AsmJSValidate.cpp:10095
#3  CheckFunction (m=..., lifo=..., funcOut=funcOut@entry=0x7fffffffb130) at js/src/asmjs/AsmJSValidate.cpp:10612
#4  0x000000000043f86c in CheckFunctionsParallel (compileResults=0x7fffffffb300, group=..., m=...) at js/src/asmjs/AsmJSValidate.cpp:10920
#5  CheckFunctions (m=..., results=results@entry=0x7fffffffb300) at js/src/asmjs/AsmJSValidate.cpp:11037
#6  0x00000000005c1fe3 in CheckModule (compilationTimeReport=0x7fffffffb2e0, moduleOut=0x7fffffffb2f0, stmtList=0x7fffffffb6e0, parser=..., cx=<optimized out>) at js/src/asmjs/AsmJSValidate.cpp:12386
#7  js::ValidateAsmJS (cx=<optimized out>, parser=..., stmtList=stmtList@entry=0x7ffff69901b0, validated=validated@entry=0x7fffffffb6e0) at js/src/asmjs/AsmJSValidate.cpp:12470
#8  0x00000000004c52ca in js::frontend::Parser<js::frontend::FullParseHandler>::asmJS (this=this@entry=0x7fffffffcaa0, list=0x7ffff69901b0) at js/src/frontend/Parser.cpp:2987
#9  0x00000000004d9c4f in js::frontend::Parser<js::frontend::FullParseHandler>::maybeParseDirective (this=this@entry=0x7fffffffcaa0, list=list@entry=0x7ffff69901b0, pn=pn@entry=0x7ffff6990220, cont=cont@entry=0x7fffffffb760) at js/src/frontend/Parser.cpp:3062
#10 0x00000000004f770c in js::frontend::Parser<js::frontend::FullParseHandler>::statements (this=this@entry=0x7fffffffcaa0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:3128
#11 0x00000000004f7a8b in js::frontend::Parser<js::frontend::FullParseHandler>::functionBody (this=this@entry=0x7fffffffcaa0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, kind=kind@entry=js::frontend::Statement, type=type@entry=js::frontend::Parser<js::frontend::FullParseHandler>::StatementListBody) at js/src/frontend/Parser.cpp:1141
#12 0x00000000004f8017 in js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBodyGeneric (this=this@entry=0x7fffffffcaa0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, pn=pn@entry=0x7ffff6990020, fun=fun@entry=..., kind=kind@entry=js::frontend::Statement) at js/src/frontend/Parser.cpp:2809
#13 0x00000000004ce5c9 in js::frontend::Parser<js::frontend::FullParseHandler>::functionArgsAndBody (this=this@entry=0x7fffffffcaa0, inHandling=inHandling@entry=js::frontend::InAllowed, pn=0x7ffff6990020, fun=..., fun@entry=..., kind=kind@entry=js::frontend::Statement, generatorKind=generatorKind@entry=js::NotGenerator, inheritedDirectives=..., newDirectives=newDirectives@entry=0x7fffffffbd90) at js/src/frontend/Parser.cpp:2613
#14 0x00000000004f851a in js::frontend::Parser<js::frontend::FullParseHandler>::functionDef (this=this@entry=0x7fffffffcaa0, inHandling=inHandling@entry=js::frontend::InAllowed, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, funName=..., funName@entry=..., kind=kind@entry=js::frontend::Statement, generatorKind=generatorKind@entry=js::NotGenerator, invoked=invoked@entry=js::frontend::Parser<js::frontend::FullParseHandler>::PredictUninvoked) at js/src/frontend/Parser.cpp:2443
#15 0x00000000004f87fd in js::frontend::Parser<js::frontend::FullParseHandler>::functionStmt (this=this@entry=0x7fffffffcaa0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, defaultHandling=defaultHandling@entry=js::frontend::NameRequired) at js/src/frontend/Parser.cpp:2893
#16 0x00000000004f721d in js::frontend::Parser<js::frontend::FullParseHandler>::statement (this=this@entry=0x7fffffffcaa0, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, canHaveDirectives=<optimized out>) at js/src/frontend/Parser.cpp:6718
#17 0x000000000063556f in BytecodeCompiler::compileScript (this=this@entry=0x7fffffffc420, scopeChain=..., scopeChain@entry=..., evalCaller=evalCaller@entry=...) at js/src/frontend/BytecodeCompiler.cpp:588
#18 0x0000000000635ad3 in js::frontend::CompileScript (cx=cx@entry=0x7ffff6907000, alloc=<optimized out>, scopeChain=..., enclosingStaticScope=..., enclosingStaticScope@entry=..., evalCaller=evalCaller@entry=..., options=..., srcBuf=..., source_=source_@entry=0x0, extraSct=extraSct@entry=0x0, sourceObjectOut=sourceObjectOut@entry=0x0) at js/src/frontend/BytecodeCompiler.cpp:807
#19 0x0000000000af4f1e in Compile (cx=cx@entry=0x7ffff6907000, options=..., scopeOption=scopeOption@entry=HasSyntacticScope, srcBuf=..., script=..., script@entry=...) at js/src/jsapi.cpp:3960
#20 0x0000000000af5161 in Compile (script=..., length=<optimized out>, chars=0x7ffff699a800 u"function foo(glob, s, b) {\n    \"use asm\";\n    var I32 = glob.Int32Array;\n    var i32 = new I32(b);\n    var len = glob.byteLength;\n    function ch(b2) {\n        if (len(b2) & 4294967295 || len(b2) <= 0"..., scopeOption=HasSyntacticScope, options=..., cx=0x7ffff6907000) at js/src/jsapi.cpp:3969
#21 Compile (cx=cx@entry=0x7ffff6907000, options=..., scopeOption=scopeOption@entry=HasSyntacticScope, bytes=<optimized out>, length=347, script=script@entry=...) at js/src/jsapi.cpp:3984
#22 0x0000000000b255c7 in Compile (cx=cx@entry=0x7ffff6907000, options=..., scopeOption=scopeOption@entry=HasSyntacticScope, fp=fp@entry=0x7ffff699a400, script=...) at js/src/jsapi.cpp:3995
#23 0x0000000000b25602 in JS::Compile (cx=cx@entry=0x7ffff6907000, options=..., file=file@entry=0x7ffff699a400, script=..., script@entry=...) at js/src/jsapi.cpp:4035
#24 0x00000000004283c0 in RunFile (compileOnly=false, file=0x7ffff699a400, filename=0x7fffffffe047 "min.js", cx=0x7ffff6907000) at js/src/shell/js.cpp:453
#25 Process (cx=cx@entry=0x7ffff6907000, filename=0x7fffffffe047 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:580
#26 0x000000000047a966 in ProcessArgs (op=0x7fffffffdae0, cx=0x7ffff6907000) at js/src/shell/js.cpp:5834
#27 Shell (envp=<optimized out>, op=0x7fffffffdae0, cx=0x7ffff6907000) at js/src/shell/js.cpp:6123
#28 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6472
rax	0x0	0
rbx	0x7ffff47fa000	140737295392768
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffab20	140737488333600
rsp	0x7fffffffab00	140737488333568
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffa8c0	140737488332992
r11	0x7ffff6c27960	140737333328224
r12	0x0	0
r13	0x7ffff7e062b0	140737352065712
r14	0x7ffff6990d88	140737330613640
r15	0x7ffff6990000	140737330610176
rip	0x5cd8c1 <js::AsmJSModule::addChangeHeap(unsigned int, unsigned int, unsigned int)+161>
=> 0x5cd8c1 <js::AsmJSModule::addChangeHeap(unsigned int, unsigned int, unsigned int)+161>:	movl   $0x46c,0x0
   0x5cd8cc <js::AsmJSModule::addChangeHeap(unsigned int, unsigned int, unsigned int)+172>:	callq  0x49b340 <abort()>


Marking s-s because I don't know if this assert is safe and it sounds like it might not be.
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:   https://hg.mozilla.org/mozilla-central/rev/8d4702c0db51
user:        Luke Wagner
date:        Wed Oct 15 09:09:44 2014 -0500
summary:     Bug 1082107 - OdinMonkey: add maximum-length requirement to change-heap definition (r=bbouvier)

This iteration took 155.208 seconds to run.
Needinfo from Luke based on comment 1.
Flags: needinfo?(luke)
Flags: needinfo?(luke)
Attached patch reject-big-maskSplinter Review
This assert is well-intentioned but technically spurious: this mask value is only ever used as a mask (I just re-audited) and never incremented with the expectation that the value is a valid heap size.  However, out of an abundance of caution and future-proofing, this patch explicitly rejects UINT32_MAX as a mask (it's a silly mask value anyway).
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8661285 - Flags: review?(benj)
Group: javascript-core-security
Comment on attachment 8661285 [details] [diff] [review]
reject-big-mask

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

Right, it's a silly value but also the most permissive. Would it be worth checking that emscripten doesn't use it for convenience?
Attachment #8661285 - Flags: review?(benj) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #4)
It's actually the *least* permissive as it will reject any non-zero heap size.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/370cf163a1f3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.