Closed Bug 1186973 Opened 9 years ago Closed 9 years ago

Crash [@ strlen] with evaluate and bytecache

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla44
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- fixed
firefox45 --- verified

People

(Reporter: decoder, Unassigned)

Details

(4 keywords, Whiteboard: [jsbugmon:update,ignore][post-critsmash-triage][adv-main44+])

Crash Data

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 2ddec2dedced (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2):

test = (function () {
  function f(x) {
    function ifTrue() {};
    function ifFalse() {};
    if (x) return ifTrue();
    else return ifFalse();
  }
  return f.toSource() + "; f((generation % 2) == 0)";
})();
evalWithCache(test, { assertEqBytecode: true, assertEqResult : true });
function evalWithCache(code, ctx) {
  code = code instanceof Object ? code : cacheEntry(code);
  ctx.global = newGlobal();
  ctx.global.generation = 0;
  var res1 = evaluate(code, Object.create(ctx, {saveBytecode: { value: true } }));
  ctx.global.generation = 1;
  var res2 = evaluate(code, Object.create(ctx, {loadBytecode: { value: true }, saveBytecode: { value: true } }));
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
strlen () at ../sysdeps/x86_64/strlen.S:106
#0  strlen () at ../sysdeps/x86_64/strlen.S:106
#1  0x0000000000acffcc in js::ExpandErrorArgumentsVA (cx=0x7ffff69811c0, callback=<optimized out>, userRef=<optimized out>, errorNumber=16, messagep=0x7fffffffc760, reportp=0x7fffffffc770, argumentsType=js::ArgumentsAreASCII, ap=0x7fffffffc838) at js/src/jscntxt.cpp:609
#2  0x0000000000ad05c8 in js::ReportErrorNumberVA (cx=0x7ffff69811c0, flags=0, callback=0x47add0 <js::shell::my_GetErrorMessage(void*, unsigned int)>, userRef=0x0, errorNumber=16, argumentsType=js::ArgumentsAreASCII, ap=0x7fffffffc838) at js/src/jscntxt.cpp:746
#3  0x0000000000ad068b in JS_ReportErrorNumberVA (cx=<optimized out>, errorCallback=<optimized out>, userRef=<optimized out>, errorNumber=<optimized out>, ap=ap@entry=0x7fffffffc838) at js/src/jsapi.cpp:5350
#4  0x0000000000ad0716 in JS_ReportErrorNumber (cx=cx@entry=0x7ffff69811c0, errorCallback=errorCallback@entry=0x47add0 <js::shell::my_GetErrorMessage(void*, unsigned int)>, userRef=userRef@entry=0x0, errorNumber=errorNumber@entry=16) at js/src/jsapi.cpp:5339
#5  0x000000000048a0de in Evaluate (cx=0x7ffff69811c0, argc=<optimized out>, vp=0x7ffff47f4148) at js/src/shell/js.cpp:1260
#6  0x00000000006cb9f2 in js::CallJSNative (cx=0x7ffff69811c0, native=0x489050 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#7  0x00000000006baef2 in js::Invoke (cx=cx@entry=0x7ffff69811c0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:720
#8  0x00000000006ad032 in Interpret (cx=cx@entry=0x7ffff69811c0, state=...) at js/src/vm/Interpreter.cpp:2972
#9  0x00000000006ba8f3 in js::RunScript (cx=cx@entry=0x7ffff69811c0, state=...) at js/src/vm/Interpreter.cpp:661
#10 0x00000000006c5606 in js::ExecuteKernel (cx=cx@entry=0x7ffff69811c0, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:902
#11 0x00000000006c78f3 in js::Execute (cx=cx@entry=0x7ffff69811c0, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:936
#12 0x0000000000ac4496 in ExecuteScript (cx=cx@entry=0x7ffff69811c0, scope=..., script=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4334
#13 0x0000000000ac460b in JS_ExecuteScript (cx=cx@entry=0x7ffff69811c0, scriptArg=..., scriptArg@entry=...) at js/src/jsapi.cpp:4365
#14 0x00000000004284d1 in RunFile (compileOnly=false, file=0x7ffff699e800, filename=0x7fffffffe0da "min.js", cx=0x7ffff69811c0) at js/src/shell/js.cpp:458
#15 Process (cx=cx@entry=0x7ffff69811c0, filename=0x7fffffffe0da "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:576
#16 0x0000000000477453 in ProcessArgs (op=0x7fffffffdb90, cx=0x7ffff69811c0) at js/src/shell/js.cpp:5771
#17 Shell (envp=<optimized out>, op=0x7fffffffdb90, cx=0x7ffff69811c0) at js/src/shell/js.cpp:6040
#18 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6384
rax	0x384	900
rbx	0x7fffffffc770	140737488340848
rcx	0x384	900
rdx	0x7fffffffc870	140737488341104
rsi	0x384	900
rdi	0x384	900
rbp	0x7fffffffc730	140737488340784
rsp	0x7fffffffc5c8	140737488340424
r8	0x0	0
r9	0xe00	3584
r10	0x4	4
r11	0x3	3
r12	0x7fffffffc838	140737488341048
r13	0x0	0
r14	0x0	0
r15	0x7ffff47c3e40	140737295171136
rip	0x7ffff6c4256a <strlen+42>
=> 0x7ffff6c4256a <strlen+42>:	movdqu (%rax),%xmm12
   0x7ffff6c4256f <strlen+47>:	pcmpeqb %xmm8,%xmm12


Marking s-s because the crash is an unsafe out-of-bounds. Not sure if this can be triggered in the browser since the test uses evaluate with the bytecode cache.
Flags: needinfo?(nicolas.b.pierron)
Group: core-security → javascript-core-security
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:   https://hg.mozilla.org/mozilla-central/rev/f25e7176b9d6
user:        Boris Zbarsky
date:        Wed Apr 01 12:05:29 2015 -0400
summary:     Bug 679939 part 8.  Drop the now-unused compileAndGo from CompileOptions.  r=luke

This iteration took 171.800 seconds to run.
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 473aefe5bd85).
Flags: needinfo?(nicolas.b.pierron)
Comment on attachment 8676944 [details] [diff] [review]
Do not use template for error messages involving numbers.

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

Can you explain the issue and why not using JS_ReportErrorNumber fixes it?
Attachment #8676944 - Flags: review?(hv1989)
(In reply to Hannes Verschore [:h4writer] from comment #4)
> Comment on attachment 8676944 [details] [diff] [review]
> Do not use template for error messages involving numbers.
> 
> Review of attachment 8676944 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you explain the issue and why not using JS_ReportErrorNumber fixes it?

The issue is that the numbers were surprisingly inferred as being pointers, instead of numbers.
Thus this patch replace them by %u .
Attachment #8676944 - Flags: review?(hv1989)
Comment on attachment 8676944 [details] [diff] [review]
Do not use template for error messages involving numbers.

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

Ah now I see. Instead of moving the values out of .msg,
can you do something like: https://dxr.mozilla.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#1102
Attachment #8676944 - Flags: review?(hv1989)
(In reply to Christian Holler (:decoder) from comment #0)
> Marking s-s because the crash is an unsafe out-of-bounds. Not sure if this
> can be triggered in the browser since the test uses evaluate with the
> bytecode cache.

This cannot be triggered in the browser, this is an issue with the way the error message is printed.
Comment on attachment 8679490 [details] [diff] [review]
Evaluate assertEqBytecode: Print length as numbers.

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

Nice
Attachment #8679490 - Flags: review?(hv1989) → review+
https://hg.mozilla.org/mozilla-central/rev/5326cb59d6a6
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Group: javascript-core-security → core-security-release
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update,ignore][post-critsmash-triage]
Whiteboard: [jsbugmon:update,ignore][post-critsmash-triage] → [jsbugmon:update,ignore][post-critsmash-triage][adv-main44+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.