565373 - TM: Invalid write of size 8 with testcase

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

for(x in <x>></x>)
(function () {
    (function f(a) {
        g();
        f(a)
    }());
    function g() {
        let(b) function () {
            return
        }()
    }
}())

Pass this in as a CLI argument to js shell on TM tip with -j on Ubuntu 10.04 64-bit, it will show an "*** glibc detected *** ./js-opt-64-tm-linux: corrupted double-linked list: 0x00000000020b77d0 ***" error. This occurs both in dbg and opt builds.

s-s because I don't know how scary this is.

Testing in valgrind reveals:

$ valgrind ./js-opt-64-tm-linux -j w42174-cj-in.js 
==17639== Memcheck, a memory error detector
==17639== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==17639== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==17639== Command: ./js-opt-64-tm-linux -j w42174-cj-in.js
==17639== 
==17639== Invalid write of size 8
==17639==    at 0x41ADF4C: ???
==17639==    by 0x51A3E6: js::ExecuteTree(JSContext*, js::TreeFragment*, unsigned int&, js::VMSideExit**, js::VMSideExit**) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x51FC34: js::MonitorLoopEdge(JSContext*, unsigned int&, js::RecordReason) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x54933D: js_Interpret (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x4573CC: js_Execute (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x40B0E5: JS_ExecuteScript (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x406294: Process(JSContext*, JSObject*, char*, int) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x406EA3: main (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==  Address 0x5ff28f8 is 0 bytes after a block of size 69,544 alloc'd
==17639==    at 0x4C27CC1: operator new(unsigned long) (vg_replace_malloc.c:261)
==17639==    by 0x4F9444: js::InitJIT(js::TraceMonitor*) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x42277F: js_InitThreads(JSRuntime*) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x40DB10: JSRuntime::init(unsigned int) (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x40DB91: JS_Init (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639==    by 0x4068A4: main (in /home/fuzz1/Desktop/3interesting/js-opt-64-tm-linux)
==17639== 
w42174-cj-in.js:4: InternalError: too much recursion
==17639== 
==17639== HEAP SUMMARY:
==17639==     in use at exit: 0 bytes in 0 blocks
==17639==   total heap usage: 1,101 allocs, 1,101 frees, 3,586,497 bytes allocated
==17639== 
==17639== All heap blocks were freed -- no leaks are possible
==17639== 
==17639== For counts of detected and suppressed errors, rerun with: -v
==17639== ERROR SUMMARY: 5 errors from 1 contexts (suppressed: 4 from 4)

Comment 1

[Andreas Gal :gal]

Did this start recently?

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #1)
> Did this start recently?

No idea (autoBisect.py is still under test) - earlier similar bug is bug 563243.

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I'm on changeset tm-41833-d9ef93881da0 64-bit Ubuntu btw.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #2)
> (In reply to comment #1)
> > Did this start recently?
> 
> No idea (autoBisect.py is still under test) - earlier similar bug is bug
> 563243.

autoBisect shows this is probably related to bug 525120:

The first bad revision is:
changeset:   34373:252097674133
user:        Luke Wagner
date:        Wed Oct 28 16:44:44 2009 -0700
summary:     Bug 525120 - move native stack off the C stack (fixes native global frame alignment) (r=dvander)

Comment 5

[Luke Wagner [:luke]]

Man, valgrind is awesome.  It informs me that the write is to one-past-the-end of TraceNativeStorage.  This looks like an off-by-one error that is just being exposed by moving the native storage to a place where valgrind can more easily see the error.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #5)
> Man, valgrind is awesome.  It informs me that the write is to one-past-the-end
> of TraceNativeStorage.  This looks like an off-by-one error that is just being
> exposed by moving the native storage to a place where valgrind can more easily
> see the error.

More accurately, Valgrind helped diagnose the issue.

This bug apparently was detected by jsfunfuzz as an opt crash in a 64-bit js shell build, and Valgrind played no part in this..

Comment 7

[Luke Wagner [:luke]]

(In reply to comment #6)
> (In reply to comment #5)
> More accurately, Valgrind helped diagnose the issue.

That's what I said.

> This bug apparently was detected by jsfunfuzz as an opt crash in a 64-bit js
> shell build, and Valgrind played no part in this..

Don't worry, I think that jsfunfuzz is awesome too ;)

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Silly bug, easy fix. We're not checking the tree's max calldepth usage when seeing if it's okay to recursive. Stack usage already had this check, so no worries there.

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/9d88c701b46f

This is most likely exploitable though not easily by any means.

Comment 10

[Robert Sayre]

this is failing a trace test

Comment 11

[Damon Sicore (:damons)]

Blocking 1.9.3 final as it's an sg:crit.

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/9d88c701b46f

Comment 13

[LegNeato]

Any chance of getting this backported to 1.9.2 for 3.6.9? Code freeze is scheduled for tomorrow night for that release btw. We were tracking bug 575263 as a blocker, and this bug may have fixed that issue on trunk.

Comment 14

[David Anderson [:dvander] - inactive, e-mail if emergency]

Earlier branches aren't affected by this bug.

Comment 15

[Christian Holler (:decoder)]

Tracer has been long gone on trunk, marking verified.

Comment 16

[Christian Holler (:decoder)]

Filter on qa-project-auto-change:

Bug in removed tracer code, setting in-testsuite- flag.