Closed Bug 1450503 Opened 3 years ago Closed 3 years ago

Assertion failure: INITHOMEOBJECT expects undefined slot, at js/src/jit/MacroAssembler.cpp:2013

Categories

(Core :: JavaScript Engine, defect, P1)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 + verified

People

(Reporter: decoder, Assigned: tcampbell)

Details

(6 keywords, Whiteboard: [jsbugmon:update][adv-main61-])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 8063b0c54b88 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --baseline-eager):

(function(index) {
    var o = {
        [index]: class {
            static constructor() {
                super["prop"]();
            }
        }
    }
    if (index === 0) {
        (function self() {
            self.caller(1);
        })();
    }
})(0);


Backtrace:

received signal SIGTRAP, Trace/breakpoint trap.
0x00003a0983763db0 in ?? ()
#0  0x00003a0983763db0 in ?? ()
[...]
#27 0x00007fffffffa680 in ?? ()
#28 0x0000000000794ca0 in EnterJit (cx=0x7fffffff9d40, state=..., code=0x0) at js/src/jit/Jit.cpp:101
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
rax	0x7ffff4caf600	140737300329984
rbx	0xfffe7ffff4c90160	-422212653219488
rcx	0xfffe7ffff4caf5b0	-422212653091408
rdx	0x7ffff5f3f000	140737319792640
rsi	0x7ffff4c8d060	140737300189280
rdi	0x7fffffff9f90	140737488330640
rbp	0x7fffffffa0d8	140737488330968
rsp	0x7fffffffa070	140737488330864
r8	0x0	0
r9	0x0	0
r10	0x0	0
r11	0x7ffff6b9e7a0	140737332766624
r12	0x0	0
r13	0x7fffffffae50	140737488334416
r14	0x2043	8259
r15	0x7fffffffa200	140737488331264
rip	0x3a0983763db0	63812534681008
=> 0x3a0983763db0:	mov    %rcx,0x40(%rax)
   0x3a0983763db4:	mov    %rax,%r11


Filing s-s because this hits a trap in MacroAssembler with no stack.
Maybe you can take a look at this because bug 1169743 added this check. That said, it's probably unrelated to that.
Flags: needinfo?(tcampbell)
Priority: -- → P1
This is a VM bug, not a JIT bug. I can reproduce by adding the same assert to JSOP_INITHOMEOBJECT. We seem to be be sharing the constructor lambda across multiple class expressions (which each have different [[HomeObject]] bindings). The logic for singleton lambdas probably needs to be tweaked.
Assignee: nobody → tcampbell
Flags: needinfo?(tcampbell)
My assessment was incorrect. The issue is due to [1] which uses an existing lambda and the slot is initialized with existing value before we call setExtendedSlot. In the interpreter this is fine, but my implementation in Baseline was assuming we could |init| and avoid barriers. The simple fix is to add the barriers and remove assert from baseline.

I think it is worth investigating how various FunctionExtended interact with barriers and cloning.

[1] https://searchfox.org/mozilla-central/rev/a0665934fa05158a5a943d4c8b277465910c029c/js/src/vm/JSFunction.cpp#2175
Not sure how serious it is; guessing sec-high
Keywords: sec-high
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/24191914f43b
user:        Ted Campbell
date:        Wed Jun 07 14:04:13 2017 -0400
summary:     Bug 1169743 - Implement class decls with extends in Baseline (cont.) r=jandem

This iteration took 261.595 seconds to run.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Under the Function.caller loophole with run-once lambdas, a method may be cloned after being assigned a HomeObject. This patch changes the initSlot with debug assert to a typical setSlot pattern to match the interpreter code.
Attachment #8970633 - Flags: review?(jdemooij)
This bug is equivalent to a missing pre-barrier in release. The previous value is always reachable from JSScript::objects, so if I understand what :sfink explained this isn't violating snapshot-at-the-beginning and so this may not be exploitable at all.

Either way, the change itself is low risk.
Attachment #8970633 - Flags: review?(jdemooij) → review+
It looks like this is a footgun rather than a sec issue so dropping severity. We'll fix the barriers anyways but only for FF61+.
https://hg.mozilla.org/mozilla-central/rev/7d3b07135ca5
Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main61-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.