982162 - Assertion failure: hasScript(), at jsfun.h:316 or Crash [@ js::XDRScript<(js::XDRMode)0>] with invalid read

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 67485526e241 (run with --fuzzing-safe):


test = (function () {
  function f() {
    var x = function inner() { 
	"use asm"; 
	function g() {} 
	return g 
    };
  };
  return f.toSource()
})();
evalWithCache(test, {});
function evalWithCache(code, ctx) {
  code = cacheEntry(code);
  ctx.compileAndGo = true;
  evaluate(code, Object.create(ctx, {saveBytecode: { value: true } }));
}

Comment 1

[Luke Wagner [:luke]]

I expect this problem only arises via the saveBytecode option to evaluate which isn't in general in the browser and thus now s-s.  Also, this may just be a case where saveBytecode just isn't fuzzing-safe.  CC'ing nbp.

Comment 2

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 3

[Luke Wagner [:luke]]

s/now s-s/not s-s/

Comment 4

[Christian Holler (:decoder)]

I agree, it could be shell only. However, the crash looks very dangerous and I wasn't sure if this is really shell-only or if the same mechanism can be triggered in the browser:


==28572== Invalid read of size 8
==28572==    at 0x76376C: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (Barrier.h:322)
==28572==    by 0x6C257E: bool js::XDRInterpretedFunction<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JSObject*>) (jsfun.cpp:448)
==28572==    by 0x7637F2: bool js::XDRScript<(js::XDRMode)0>(js::XDRState<(js::XDRMode)0>*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>) (jsscript.cpp:951)
==28572==    by 0x8254CE: js::XDRState<(js::XDRMode)0>::codeScript(JS::MutableHandle<JSScript*>) (Xdr.cpp:111)
==28572==    by 0x684D8B: JS_EncodeScript(JSContext*, JS::Handle<JSScript*>, unsigned int*) (jsapi.cpp:6219)
==28572==    by 0x40E819: Evaluate(JSContext*, unsigned int, JS::Value*) (js.cpp:1272)
==28572==  Address 0x1302494894858 is not stack'd, malloc'd or (recently) free'd



Therefore marked s-s until triaged :)

Comment 5

[Nicolas B. Pierron [:nbp]]

Yes, this a temporary known issue which would be handled when we will support XDR'd asmjs as part of Bug 959268.  To be honest I was kind of expecting this bug as a way to see how well the bytecode cache is verified :)

This is not a security issue as evaluate & saveBytecode is only exposed to the shell as a way to use JS_EncodeScript.  So far JS_EncodeScript / JS_EncodeFunction are only used by XUL, and we do not use this "use asm" directive in XUL. (otherwise it will crash all the time with this exact signature)

I do not intend to fix Bug 959268 prior making a prototype of the bytecode cache working on B2G.

What I can do in the mean time as part of this bug, is to replace this assertion by a NYI exception, would that be enough for fuzzers?

Comment 6

[Nicolas B. Pierron [:nbp]]

The bisect would lead to the introduction of the test infrastructure (Bug 900789), and to be more precise:

Bug 900789 - Instrument evaluate function to save/load the bytecode. r=luke
author	Nicolas B. Pierron
	Thu Feb 20 07:09:34 2014 -0800 (at Thu Feb 20 07:09:34 2014 -0800)
changeset 169693	f317d2cb357b

The bug reported here pre-dates this patch, and if this is exploitable, this would affect all versions where asmjs is enabled.

Comment 7

[Luke Wagner [:luke]]

(In reply to Nicolas B. Pierron [:nbp] from comment #6)
> The bug reported here pre-dates this patch, and if this is exploitable, this
> would affect all versions where asmjs is enabled.

I don't think there is any pre-existing bug; asm.js compilation is disabled for !compileAndGo scripts and therefore it is never serialized in the browser.

Comment 8

[Nicolas B. Pierron [:nbp]]

(In reply to Luke Wagner [:luke] from comment #7)
> (In reply to Nicolas B. Pierron [:nbp] from comment #6)
> > The bug reported here pre-dates this patch, and if this is exploitable, this
> > would affect all versions where asmjs is enabled.
> 
> I don't think there is any pre-existing bug; asm.js compilation is disabled
> for !compileAndGo scripts and therefore it is never serialized in the
> browser.

Oh, so no XUL can have any asm.js module, even if they are using "use asm".  So this cannot be a security issue.
But we still need to fix that for langfuzz.

Comment 9

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unknown exception (check manually)

Comment 10

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140220065927" and the hash "f1664bf0e431".
The "bad" changeset has the timestamp "20140220071028" and the hash "112accda13eb".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f1664bf0e431&tochange=112accda13eb

Comment 11

[Nicolas B. Pierron [:nbp]]

Attachment: Throw when we attempt to serialize an AsmJS module.

Comment 12

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45a012689f10

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/45a012689f10