Closed Bug 953104 Opened 6 years ago Closed 6 years ago

OdinMonkey: Assertion failure: !elems_.empty(), at jit/AsmJS.cpp:1213 or Crash on Heap

Categories

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

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla29
Tracking Status
firefox27 --- wontfix
firefox28 --- wontfix
firefox29 --- fixed
firefox-esr24 --- wontfix
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- wontfix
b2g-v1.3 --- wontfix
b2g-v1.4 --- wontfix

People

(Reporter: decoder, Assigned: luke)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, sec-low, testcase, Whiteboard: [jsbugmon:update][adv-main29+])

Attachments

(2 files)

The following testcase asserts on mozilla-central revision cd3e9359fd64 (run with --fuzzing-safe --ion-eager --ion-compile-try-catch --ion-eager):


function m() {
    "use asm";
    function f() {
        var x = 0;
        var y = 0;
        y = y[(x + 1) & 3]() | 0;
    }
    return f;
}
m()();
Whiteboard: [jsbugmon:update,bisect]
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/63d537758124
user:        Luke Wagner
date:        Thu Jun 13 11:48:47 2013 -0700
summary:     Bug 880538 - OdinMonkey: make a single pass over the parse tree (r=bbouvier)

This iteration took 330.591 seconds to run.
Needinfo from Luke based on comment 2.
Flags: needinfo?(luke)
Flags: needinfo?(luke)
Oops, there totally should have been a unit test for this.  Great find!
Assignee: nobody → luke
Status: NEW → ASSIGNED
Attachment #8355255 - Flags: review?(benj)
How bad of a security issue is this, Luke?  Thanks.
Flags: needinfo?(luke)
I think this could be used to call a definitely-null pointer, so it'd be a crash, but not exploitable.  This won't happen in well-formed asm.js programs, so I'd be inclined to let the fix ride the trains.
Flags: needinfo?(luke)
Keywords: sec-low
Comment on attachment 8355255 [details] [diff] [review]
fix-func-ptr-table-bug

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

Nice catch!

::: js/src/jit-test/tests/asm.js/testFunctionPtr.js
@@ +11,5 @@
>  assertAsmTypeFail(USE_ASM + "function f() {} function g(i) {i=i|0} var tbl=[f,g]; return f");
>  assertAsmTypeFail(USE_ASM + "function f() {} function g() {return 0} var tbl=[f,g]; return f");
>  assertAsmTypeFail(USE_ASM + "function f(i) {i=i|0} function g(i) {i=+i} var tbl=[f,g]; return f");
>  assertAsmTypeFail(USE_ASM + "function f() {return 0} function g() {return 0.0} var tbl=[f,g]; return f");
> +assertAsmTypeFail(USE_ASM + "var tbl=0; function g() {tbl[0&1]()|0} return g");

Ha, nice catch! I have been sometimes worried about creating assertAsmTypeFail tests and having them pass for another reason, like what was happening here.
Attachment #8355255 - Flags: review?(benj) → review+
Yeah, I've thought it might be nice to match not just that there was an error, but part of the text of the message.  OTOH, this would make the tests more fragile, which would be annoying.

https://hg.mozilla.org/integration/mozilla-inbound/rev/567977c0c7da
https://hg.mozilla.org/mozilla-central/rev/567977c0c7da
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
(In reply to Luke Wagner [:luke] from comment #6)
> I think this could be used to call a definitely-null pointer, so it'd be a
> crash, but not exploitable.  This won't happen in well-formed asm.js
> programs, so I'd be inclined to let the fix ride the trains.

Sounds like a wontfix for B2G as well then too :)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main29+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.