New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 684858 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Crash in v8::internal::wasm::ModuleWireBytes::GetNameOrNull

Project Member Reported by ClusterFuzz, Jan 24 2017

Issue description

Components: Blink>JavaScript
Labels: M-57 Test-Predator-Correct
Owner: mtrofin@chromium.org
Status: Assigned (was: Untriaged)
The result is a list of CLs that change the crashed files. 

Author: mtrofin
Project: chromium-v8
Changelist: https://chromium.googlesource.com/v8/v8.git/+/8da82d30b861cdb6cc3c4276322a1592cb8ae4a2
Time: Tue Jan 24 07:11:01 2017
Lines 3752, 3772-3774 of file wasm-compiler.cc which potentially caused crash are changed in this cl (frame #3, "v8::internal::compiler::WasmCompilationUnit::WasmCompilationUnit").
Cc: msrchandra@chromium.org mtrofin@chromium.org
 Issue 684958  has been merged into this issue.
Cc: bradnelson@chromium.org titzer@chromium.org
Owner: clemensh@chromium.org
See https://codereview.chromium.org/2656563005

This is requires more analysis, because it appears we have tests that depend on the names table being present - and the names table is not part of the standard.

Clemens - does the debugger story hinge on this?
I commented on the CL. The names section is specified in the binary format: https://github.com/WebAssembly/design/blob/master/BinaryEncoding.md#name-section
We use it to print function names on stack traces and in live stack trace inspection via the debugger.
I don't think that it's only meant to be used in the asm.js case.
New CL to fix the original issue: http://crrev.com/2656713003
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2017

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

commit 0ec3a264bcba5c899f7e72fa5b98dd191477e272
Author: clemensh <clemensh@chromium.org>
Date: Wed Jan 25 11:37:48 2017

[wasm] Fix check failure on invalid name section

After decoding an invalid function name (e.g. OOB), we stored the parsed
offset and length into the WasmFunction anyway, resulting in a runtime
CHECK failure later on.
This CL fixes this, and adds a regression test.

R=titzer@chromium.org
CC=mtrofin@chromium.org, bradnelson@chromium.org
BUG= chromium:684858 

Review-Url: https://codereview.chromium.org/2656713003
Cr-Commit-Position: refs/heads/master@{#42654}

[modify] https://crrev.com/0ec3a264bcba5c899f7e72fa5b98dd191477e272/src/wasm/module-decoder.cc
[add] https://crrev.com/0ec3a264bcba5c899f7e72fa5b98dd191477e272/test/mjsunit/regress/wasm/regression-684858.js

Status: Fixed (was: Assigned)
Project Member

Comment 8 by ClusterFuzz, Jan 26 2017

ClusterFuzz has detected this issue as fixed in range 445994:446028.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5440268850692096

Fuzzer: libfuzzer_v8_wasm_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: UNKNOWN
Crash Address: 0x000001f63698
Crash State:
  v8::internal::wasm::ModuleWireBytes::GetNameOrNull
  v8::internal::compiler::WasmCompilationUnit::WasmCompilationUnit
  InitializeParallelCompilation
  
Sanitizer: address (ASAN)

Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=445713:445758
Fixed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_asan&range=445994:446028

Minimized Testcase (0.05 Kb): https://cluster-fuzz.appspot.com/download/AMIfv95bSwoQeGJS5CK6KobaKFSsegWNYsDRf9E7JrCdnMYvUBJNZfHs1Yzp-WGio_p-V_ynWdPaUzzid42UUCsn3fs7AyRXH6aLoo_0_3-GnKqKrlOmhx2LLRbkAidgrGubdCC8oqw-kh_B2BNCvgd1csdsGqMsFpUwNKJxyauySMeKcbGZQXp34qDbWNYOx1FaD1ge1PJKWZqv7GLJpYre9NFM9oZ3AuW417lPuuXmnBNXUUro5dVFd_EAqHSSD5x-oyO12ZuS2SysrQhoAu-sok4uLbLl5Ewo8bRnAq4ePQrRuMYl0WICfcuUeFLsXQftA4qqF5d0J5nLP4-9ZOf48TkLo6xjaQSldnVnq-b5txf2N5dxI-w?testcase_id=5440268850692096

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 27 2017

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

commit 98cc297fceaa828b8d4a86a8b1f2809ae7445480
Author: Brad Nelson <bradnelson@chromium.org>
Date: Fri Jan 27 01:05:18 2017

Merged: [wasm] Fix check failure on invalid name section

Revision: 0ec3a264bcba5c899f7e72fa5b98dd191477e272

BUG= chromium:684858 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=mtrofin@chromium.org

Review-Url: https://codereview.chromium.org/2657113002 .
Cr-Commit-Position: refs/branch-heads/5.7@{#33}
Cr-Branched-From: 975e9a320b6eaf9f12280c35df98e013beb8f041-refs/heads/5.7.492@{#1}
Cr-Branched-From: 8d76f0e3465a84bbf0bceab114900fbe75844e1f-refs/heads/master@{#42426}

[modify] https://crrev.com/98cc297fceaa828b8d4a86a8b1f2809ae7445480/src/wasm/module-decoder.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 27 2017

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

commit 3795989355818b4503b53761f33aded18c368208
Author: bradnelson <bradnelson@chromium.org>
Date: Fri Jan 27 02:03:26 2017

Revert of Merged: [wasm] Fix check failure on invalid name section (patchset #1 id:1 of https://codereview.chromium.org/2657113002/ )

Reason for revert:
Has an order dependency.

Original issue's description:
> Merged: [wasm] Fix check failure on invalid name section
>
> Revision: 0ec3a264bcba5c899f7e72fa5b98dd191477e272
>
> BUG= chromium:684858 
> LOG=N
> NOTRY=true
> NOPRESUBMIT=true
> NOTREECHECKS=true
> R=mtrofin@chromium.org
>
> Review-Url: https://codereview.chromium.org/2657113002 .
> Cr-Commit-Position: refs/branch-heads/5.7@{#33}
> Cr-Branched-From: 975e9a320b6eaf9f12280c35df98e013beb8f041-refs/heads/5.7.492@{#1}
> Cr-Branched-From: 8d76f0e3465a84bbf0bceab114900fbe75844e1f-refs/heads/master@{#42426}
> Committed: https://chromium.googlesource.com/v8/v8/+/98cc297fceaa828b8d4a86a8b1f2809ae7445480

TBR=mtrofin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:684858 

Review-Url: https://codereview.chromium.org/2661553002
Cr-Commit-Position: refs/branch-heads/5.7@{#42}
Cr-Branched-From: 975e9a320b6eaf9f12280c35df98e013beb8f041-refs/heads/5.7.492@{#1}
Cr-Branched-From: 8d76f0e3465a84bbf0bceab114900fbe75844e1f-refs/heads/master@{#42426}

[modify] https://crrev.com/3795989355818b4503b53761f33aded18c368208/src/wasm/module-decoder.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 27 2017

Sign in to add a comment