TM: Crash [@ js_ValueToString] - Segfault when accessing out-of-range arguments on trace




JavaScript Engine
8 years ago
5 years ago


(Reporter: dvander, Assigned: dvander)


({crash, regression, testcase})

crash, regression, testcase
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking1.9.2 needed, status1.9.2 .7-fixed, status1.9.1 unaffected)


(Whiteboard: [sg:critical][ccbr] fixed-in-tracemonkey, crash signature)


(2 attachments)

/* vim: set ts=4 sw=4 tw=99 et: */

function f() { = function(a, b, c) {
        arguments[3] = { }
        arguments.length = 4;
        for (var i = 0; i < 100; i++) {

var o = new f();{x: -1, y: -1, w: 100600, h: 100600});
Created attachment 438309 [details] [diff] [review]

basically another variant of bug 554670
Assignee: general → dvander
Attachment #438309 - Flags: review?(gal)

Comment 2

8 years ago
Can you explain to me why we crash? I remember that we made this trace for a reason, so we are going to break some insane benchmark code.
We crash because the recorder state does not match the interpreter state. The interpreter is placing an object on the stack and the tracer is placing a void.

Comment 4

8 years ago
What object and why?

Comment 5

8 years ago
Is this the { } object there? Can we access that from trace?
When the JIT goes to box up the stack for Print(), it applies the interpreter's type (object) to the value in the JIT frame, which is an unboxed JSVAL_VOID - the result is Print() gets a vp with a "2" in it, NULL tagged as double.

We don't access overridden arguments anywhere on trace, as comment #1 hinted this is a variant of a similar case that was fixed recently:

> var a = arguments;
> a[3] = { };
> a.length = 4;

It's probably possible to trace it if we treat it as a generic GETELEM or something, but that would be a big patch. At least for 1.9.2 I'd prefer to just not trace it.

Comment 7

8 years ago
Comment on attachment 438309 [details] [diff] [review]

Double check with sayrer that this doesn't regress that weird argc overshooting benchmark.
Attachment #438309 - Flags: review?(gal) → review+


7 years ago
Whiteboard: [sg:crit]
Whiteboard: [sg:crit] → [sg:critical]


7 years ago
Keywords: testcase
Whiteboard: [sg:critical] → [sg:critical][ccbr]

Comment 8

7 years ago
try this in a browser with the peacekeeper tests.
Giving this the same status (wanted, not blocking) as bug 554670, renominate for blocking if you feel that's inappropriate!
blocking1.9.2: ? → -
status1.9.2: --- → wanted
autoBisect confirms that this is probably related to bug 453730:

The first bad revision is:
changeset:   30020:c76558a87dd9
user:        David Mandelin
date:        Wed Jul 08 11:16:41 2009 -0700
summary:     Bug 453730: trace JSOP_ARGUMENTS, r=gal
Blocks: 453730
Severity: normal → critical
Keywords: crash, regression
OS: Linux → All
Hardware: x86 → All
Version: unspecified → Trunk

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000000
Crashed Thread:  0  Dispatch queue:

Thread 0 Crashed:  Dispatch queue:
0   js-opt-32-tm-darwin           	0x000dbd13 js_ValueToString + 211
1   js-opt-32-tm-darwin           	0x00004406 Print(JSContext*, unsigned int, long*) + 166
2   ???                           	0x003d2f84 0 + 4009860
3   js-opt-32-tm-darwin           	0x00127480 js::ExecuteTree(JSContext*, js::TreeFragment*, unsigned int&, js::VMSideExit**) + 736
4   js-opt-32-tm-darwin           	0x001427c8 js::MonitorLoopEdge(JSContext*, unsigned int&, js::RecordReason) + 1064
5   js-opt-32-tm-darwin           	0x00062533 js_Interpret + 52291
6   js-opt-32-tm-darwin           	0x00065653 js_Execute + 531
7   js-opt-32-tm-darwin           	0x0000f33c JS_ExecuteScript + 60
8   js-opt-32-tm-darwin           	0x00004daf Process(JSContext*, JSObject*, char*, int) + 1647
9   js-opt-32-tm-darwin           	0x00008dea main + 1626
10  js-opt-32-tm-darwin           	0x000028dd _start + 208
11  js-opt-32-tm-darwin           	0x0000280c start + 40
Summary: TM: segfault when accessing out-of-range arguments on trace → TM: Crash [@ js_ValueToString] - Segfault when accessing out-of-range arguments on trace
Created attachment 440436 [details] [diff] [review]

branch fix. does not regress peacekeeper.
Attachment #440436 - Flags: review?(dmandelin)

I don't think this is exploitable on x86 because the low 32-bits will always be 0x2, and it'll act like a NULL deref/correctness bug if it escapes. On x64 the high 32-bits will be garbage. Well, at the least it's a pretty trivial crash to hit and experiment with so I won't mind if the bug stays closed/peach-coloured.
Whiteboard: [sg:critical][ccbr] → [sg:critical][ccbr] fixed-in-tracemonkey
Attachment #440436 - Flags: review?(dmandelin) → review+
Attachment #440436 - Flags: approval1.9.2.5?

Comment 14

7 years ago
Last Resolved: 7 years ago
Resolution: --- → FIXED


7 years ago
blocking1.9.2: - → needed
status1.9.1: --- → ?

Comment 15

7 years ago
Comment on attachment 440436 [details] [diff] [review]

a=LegNeato for Please land this on mozilla-1.9.2 default.
Attachment #440436 - Flags: approval1.9.2.5? → approval1.9.2.6+
status1.9.2: wanted → .6-fixed
Bustage fix (caused by the fact that I transplanted instead of pushing the other patch in the bug):

Comment 18

7 years ago
I want to confirm in the bug (because it is unclear) 1.9.1 affected? If so we'll mark it as such.

Comment 19

7 years ago
err, I mean we'll mark it as unaffected if this is not an issue on 1.9.1.
1.9.1 is not affected.
status1.9.1: ? → unaffected

Comment 21

7 years ago
Thanks so much for the confirmation!
What is the STR for this for verification in 1.9.2? I tried the code in comment 0 in a JS Shell but it didn't crash (just gave me a bunch of objects).
Group: core-security
Crash Signature: [@ js_ValueToString]
Automatically extracted testcase for this bug was committed:
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.