New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 19
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-03-21
OS: Linux
Pri: 1
Type: Bug-Security

Blocking:
issue 824443


Participants' hotlists:
Hotlist-OffHeapWasm


Sign in to add a comment

Security:crash(SEGV_MAPERR ) in wasm module

Reported by jinzhe6...@gmail.com, Mar 15

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce the problem:
1. config args.gn file as below:
		sanitizer_coverage_flags = "trace-pc-guard"
		use_sanitizer_coverage = true
		is_debug = false
		enable_nacl = false
		treat_warnings_as_errors = false

2. build chrome:
	ninja -j16 -C out/chrome_asan chrome
	also build ninja -j16 -C out/chrome_asan d8

3. 
	reproduce with chrome browser
	1) open chrome browser,"./chrome".
	2) drag crash.html.
	3) get crash.

	reproduce with d8
	1) ./d8 crash.js

I don't understand what is really going on...

What is the expected behavior?

What went wrong?
asan symbolized crash log:
Received signal 11 SEGV_MAPERR 000000000138
    #0 0x55c44f878b21 in __interceptor_backtrace /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:3957:13
    #1 0x55c45794d45e in base::debug::StackTrace::StackTrace(unsigned long) /home/cowboy/chrom/src/out/chrome_asan_shared/../../base/debug/stack_trace_posix.cc:808:41
    #2 0x55c45794c28f in base::debug::(anonymous namespace)::StackDumpSignalHandler(int, siginfo_t*, void*) /home/cowboy/chrom/src/out/chrome_asan_shared/../../base/debug/stack_trace_posix.cc:334:3
    #3 0x7f9bed61d390 in __funlockfile ??:?
    #4 0x7f9bed61d390 in ?? ??:0
    #5 0x55c4560663a0 in v8::internal::wasm::NativeModule::SetCompiledModule(v8::internal::Handle<v8::internal::WasmCompiledModule>) /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/wasm/wasm-code-manager.cc:381:20
    #6 0x55c45610540f in v8::internal::WasmCompiledModule::Clone(v8::internal::Isolate*, v8::internal::Handle<v8::internal::WasmCompiledModule>) /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/wasm/wasm-objects.cc:1471:27
    #7 0x55c455fed44c in v8::internal::wasm::(anonymous namespace)::InstanceBuilder::Build() /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/wasm/module-compiler.cc:0:11
    #8 0x55c455feb2a8 in v8::internal::wasm::InstantiateToInstanceObject(v8::internal::Isolate*, v8::internal::wasm::ErrorThrower*, v8::internal::Handle<v8::internal::WasmModuleObject>, v8::internal::MaybeHandle<v8::internal::JSReceiver>, v8::internal::MaybeHandle<v8::internal::JSArrayBuffer>) /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/wasm/module-compiler.cc:462:27
    #9 0x55c4547e23e9 in v8::internal::AsmJs::InstantiateAsmWasm(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SharedFunctionInfo>, v8::internal::Handle<v8::internal::FixedArray>, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::JSArrayBuffer>) /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/asmjs/asm-js.cc:395:31
    #10 0x55c455c2d54b in __RT_impl_Runtime_InstantiateAsmJs /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/runtime/runtime-compiler.cc:126:34
    #11 0x55c455c2d54b in v8::internal::Runtime_InstantiateAsmJs(int, v8::internal::Object**, v8::internal::Isolate*) /home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/runtime/runtime-compiler.cc:106:0
#10 0x7efd3028469d <unknown>
  r8: 00007f9bdc2f8000  r9: 0000000000004000 r10: 00007fffd86c7000 r11: 0033c8d89578d601
 r12: 00000fdffd06875e r13: 0000625000011a10 r14: 0000625001727080 r15: 0000625000011a00
  di: 000055c46aaed0a8  si: 0000625001727080  bp: 00007fffd8ec1c10  bx: 0000000000000138
  dx: 000055c4561027a0  ax: 0000000000000027  cx: 00007f9bd963a000  sp: 00007fffd8ec1c00
  ip: 000055c4560663a0 efl: 0000000000010246 cgf: 002b000000000033 erf: 0000000000000006
 trp: 000000000000000e msk: 0000000000000000 cr2: 0000000000000138
[end of stack trace]
Calling _exit(1). Core file will not be generated.
/home/cowboy/chrom/src/out/chrome_asan_shared/../../v8/src/wasm/wasm-objects.cc

debug d8n with gdb:
Thread 1 "d8" received signal SIGSEGV, Segmentation fault.
0x0000555557855a90 in v8::internal::wasm::NativeModule::SetCompiledModule(v8::internal::Handle<v8::internal::WasmCompiledModule>) () at ../../v8/src/wasm/wasm-code-manager.cc:381
381	  compiled_module_ = compiled_module;
(gdb) bt
#0  0x0000555557855a90 in v8::internal::wasm::NativeModule::SetCompiledModule(v8::internal::Handle<v8::internal::WasmCompiledModule>) () at ../../v8/src/wasm/wasm-code-manager.cc:381
#1  0x00005555578f51ff in Clone () at ../../v8/src/wasm/wasm-objects.cc:1471
#2  0x00005555577dcbfc in Build () at ../../v8/src/wasm/module-compiler.cc:2118
#3  0x00005555577daa58 in InstantiateToInstanceObject () at ../../v8/src/wasm/module-compiler.cc:462
#4  0x0000555555fdba99 in InstantiateAsmWasm () at ../../v8/src/asmjs/asm-js.cc:395
#5  0x000055555741d36b in __RT_impl_Runtime_InstantiateAsmJs () at ../../v8/src/runtime/runtime-compiler.cc:126
#6  Runtime_InstantiateAsmJs () at ../../v8/src/runtime/runtime-compiler.cc:106

(gdb) x/i $pc
=> 0x555557855a90 <_ZN2v88internal4wasm12NativeModule17SetCompiledModuleENS0_6HandleINS0_18WasmCompiledModuleEEE+48>:	mov    QWORD PTR [rbx],r14
(gdb) i r rbx
rbx            0x138	312
(gdb) i r r14
r14            0x6250002603a0	108095739397024

Did this work before? N/A 

Chrome version: 67.0.3369.0   Channel: dev
OS Version: Ubuntu 16.04
Flash Version:
 
chrome-symbolized.log
3.1 KB View Download
crash.html
167 bytes View Download
crash.js
149 bytes View Download
gdb.log
1017 bytes View Download
Components: Blink>JavaScript Blink>JavaScript>WebAssembly
Cc: titzer@chromium.org mstarzinger@chromium.org
Components: -Blink>JavaScript
Labels: -Pri-2 M-67 Pri-1
Owner: clemensh@chromium.org
Status: Assigned (was: Unconfirmed)
Can reproduce locally. The code instantiates a lot of asm.js modules by using an asm function as compare function to sort an array of >4M entries.

Will take a look.
Labels: Security_Impact-Head Security_Severity-High
Thanks for investigating. Do you know yet if this affects beta or stable?
Project Member

Comment 4 by ClusterFuzz, Mar 16

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4593492605796352.
Project Member

Comment 5 by ClusterFuzz, Mar 16

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4911182746746880.
Status: Started (was: Assigned)
Little bit shorter reproducer, which also crashes faster:

function asm() {
  'use asm';
  function f() {}
  return f;
}
for (var i = 0; i < 50000; ++i) {
  asm();
}

The number of iterations can be reduced further, but eventually it will not crash any more.
Will not start debugging.
This is about OOM in the NativeModule. We have lots of methods which return false or nullptr if some allocation failed. We should call FatalProcessOutOfMemory instead.

Working on a fix.
Labels: -Security_Impact-Head -Security_Severity-High M-65 M-66 Security_Impact-Stable Security_Severity-Low
This probably fails since M65 when we enabled wasm-jit-to-native, but it's not high severity, since in the OOM case we return a nullptr, which should crash reliably.
CL: https://crrev.com/c/966501
Will probably not land before Monday. That should be fine though as this is not that critical.
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/468a9303fd47a4b04c493f17fedd1bc96258f476

commit 468a9303fd47a4b04c493f17fedd1bc96258f476
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Mar 19 13:30:30 2018

[wasm] Call FatalProcessOutOfMemory on OOM

Instead of returning nullptr, just always call FatalProcessOutOfMemory
when we cannot allocate more memory.
In a follow-up CL, this should be extended to first try to run a GC and
see if this freed enough memory.
This CL is intentionally minimal in order to make it backmergable.

The unittest for WasmCodeManager needs to be refactored into a
parameterized test, such that each individual (parameterized) test can
die with OOM without affecting other tests.

R=mstarzinger@chromium.org

Bug:  chromium:822266 
Change-Id: I1336aa05ed50124b77ffaa4435ec9bed70e15c18
Reviewed-on: https://chromium-review.googlesource.com/966501
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52025}
[modify] https://crrev.com/468a9303fd47a4b04c493f17fedd1bc96258f476/src/base/platform/platform-win32.cc
[modify] https://crrev.com/468a9303fd47a4b04c493f17fedd1bc96258f476/src/wasm/wasm-code-manager.cc
[modify] https://crrev.com/468a9303fd47a4b04c493f17fedd1bc96258f476/src/wasm/wasm-code-manager.h
[modify] https://crrev.com/468a9303fd47a4b04c493f17fedd1bc96258f476/test/unittests/wasm/wasm-code-manager-unittest.cc

NextAction: 2018-03-21
Status: Fixed (was: Started)
Fixed. Let's wait for some canary coverage, then merge back.
Project Member

Comment 12 by sheriffbot@chromium.org, Mar 19

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
The NextAction date has arrived: 2018-03-21
Labels: Merge-Request-66
Requesting merge to M66. I think it's not important enough to merge to M65, since it reliably causes a nullptr access, hence a crash.
Project Member

Comment 16 by sheriffbot@chromium.org, Mar 21

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: M66 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
 Issue 824034  has been merged into this issue.
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge for M66. Branch:3359
Blocking: 824443
Project Member

Comment 20 by bugdroid1@chromium.org, Mar 22

Labels: merge-merged-6.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5

commit e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Thu Mar 22 10:01:33 2018

Merged: [wasm] Call FatalProcessOutOfMemory on OOM

Instead of returning nullptr, just always call FatalProcessOutOfMemory
when we cannot allocate more memory.
In a follow-up CL, this should be extended to first try to run a GC and
see if this freed enough memory.
This CL is intentionally minimal in order to make it backmergable.

The unittest for WasmCodeManager needs to be refactored into a
parameterized test, such that each individual (parameterized) test can
die with OOM without affecting other tests.

R=mstarzinger@chromium.org
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Bug:  chromium:822266 
Change-Id: Id9c5f87cceafe4be02e1a274fe83c39df0c5f0c4
Reviewed-on: https://chromium-review.googlesource.com/973964
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.6@{#27}
Cr-Branched-From: d500271571b92cb18dcd7b15885b51e8f437d640-refs/heads/6.6.346@{#1}
Cr-Branched-From: 265ef0b635f8761df7c89eb4e8ec9c1a6ebee184-refs/heads/master@{#51624}
[modify] https://crrev.com/e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5/src/base/platform/platform-win32.cc
[modify] https://crrev.com/e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5/src/wasm/wasm-code-manager.cc
[modify] https://crrev.com/e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5/src/wasm/wasm-code-manager.h
[modify] https://crrev.com/e599c0b3d081ca5f1d8e64e6ba6e13b728bcb2c5/test/unittests/wasm/wasm-code-manager-unittest.cc

Labels: -Merge-Approved-66
Cc: awhalley@chromium.org
Labels: -reward-topanel reward-0
I'm afraid the Chrome VRP Panel declined to reward for this bug, but may thanks for the report. Also, how would like to be credited in the release notes?
Hi,here is reporter information:

Zhe Jin from Chengdu Security Response Center of Qihoo 360 Technology Co. Ltd.

Have a good day^-^
Thank U~~~~
Can this issue get CVE?
yep, this will get a CVE issued when Chrome 66 goes stable.  Cheers!
Labels: Release-0-M66
Labels: CVE-2018-6116
Labels: CVE_description-missing
Project Member

Comment 29 by sheriffbot@chromium.org, Jun 25

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment