Detailed report: https://clusterfuzz.com/testcase?key=5514330966851584

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::Invoke
  v8::internal::CallInternal
  v8::Script::Run
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=48148:48149

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5514330966851584

See https://github.com/google/clusterfuzz-tools for more information.

Detailed report: https://clusterfuzz.com/testcase?key=5350247143571456

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: UNKNOWN READ
Crash Address: 0xfffffffffffffffe
Crash State:
  v8::internal::Invoke
  v8::internal::CallInternal
  v8::internal::Execution::TryCall
  
Sanitizer: address (ASAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=48148:48149

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5350247143571456

See https://github.com/google/clusterfuzz-tools for more information.

Detailed report: https://clusterfuzz.com/testcase?key=4912585312043008

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Null-dereference WRITE
Crash Address: 0x000000000000
Crash State:
  v8::internal::Invoke
  v8::internal::CallInternal
  v8::Script::Run
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=48148:48149

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4912585312043008

See https://github.com/google/clusterfuzz-tools for more information.

Detailed report: https://clusterfuzz.com/testcase?key=4642756608917504

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: UNKNOWN WRITE
Crash Address: 0x000000003d93
Crash State:
  v8::internal::Invoke
  v8::internal::CallInternal
  v8::Script::Run
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=48148:48149

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4642756608917504

See https://github.com/google/clusterfuzz-tools for more information.

Testcase 4642756608917504 is a top crash on ClusterFuzz for linux platform. Please prioritize fixing this crash.

Marking this crash as a Beta release blocker.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This affects 62 as well, so we'll need merge the fix there too.

This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

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

commit 1f99c66b56579a80efd28c3c10327738b368dc1f
Author: Eric Holk <eholk@chromium.org>
Date: Fri Sep 29 20:24:04 2017

[wasm] always allocate memory when guard regions are needed

When using trap handlers, memory references do not get any checks inserted. This
means there is no check for a null memory as happens when the memory size is
0. Normally this would be correctly caught as an out of bounds access, since the
low memory addresses are not normally mapped. However, if they were mapped for
some reason, we would not catch the out of bounds access.

The fix is to ensure WebAssembly instances always have a guard region even if
the memory is size 0.

Bug:  chromium:769637 
Change-Id: I2d0f8c107563236c3780eb7746c2f820e319c65f
Reviewed-on: https://chromium-review.googlesource.com/693137
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48240}
[modify] https://crrev.com/1f99c66b56579a80efd28c3c10327738b368dc1f/src/wasm/module-compiler.cc
[modify] https://crrev.com/1f99c66b56579a80efd28c3c10327738b368dc1f/src/wasm/wasm-module.cc
[add] https://crrev.com/1f99c66b56579a80efd28c3c10327738b368dc1f/test/mjsunit/regress/wasm/regression-769637.js

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

commit 7cf29d8df3b86532de20897f071b50c07d80f73f
Author: Eric Holk <eholk@chromium.org>
Date: Fri Sep 29 21:35:37 2017

Revert "[wasm] always allocate memory when guard regions are needed"

This reverts commit 1f99c66b56579a80efd28c3c10327738b368dc1f.

Reason for revert: Test timeouts on Win64 Debug: https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20debug/builds/19226

Original change's description:
> [wasm] always allocate memory when guard regions are needed
> 
> When using trap handlers, memory references do not get any checks inserted. This
> means there is no check for a null memory as happens when the memory size is
> 0. Normally this would be correctly caught as an out of bounds access, since the
> low memory addresses are not normally mapped. However, if they were mapped for
> some reason, we would not catch the out of bounds access.
> 
> The fix is to ensure WebAssembly instances always have a guard region even if
> the memory is size 0.
> 
> Bug:  chromium:769637 
> Change-Id: I2d0f8c107563236c3780eb7746c2f820e319c65f
> Reviewed-on: https://chromium-review.googlesource.com/693137
> Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
> Commit-Queue: Eric Holk <eholk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48240}

TBR=gdeepti@chromium.org,mtrofin@chromium.org,eholk@chromium.org

Change-Id: I4065b367c6cfffe8dd601b67cd53ad54759ae96a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:769637 
Reviewed-on: https://chromium-review.googlesource.com/692918
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48242}
[modify] https://crrev.com/7cf29d8df3b86532de20897f071b50c07d80f73f/src/wasm/module-compiler.cc
[modify] https://crrev.com/7cf29d8df3b86532de20897f071b50c07d80f73f/src/wasm/wasm-module.cc
[delete] https://crrev.com/658daa65344584c8b03b780ae2b852fb8e7872c7/test/mjsunit/regress/wasm/regression-769637.js

Rejecting merge since it appears it's reverted. 

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

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

commit 5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc
Author: Eric Holk (eholk) <eholk@google.com>
Date: Wed Oct 04 16:59:25 2017

Reland "[wasm] always allocate memory when guard regions are needed"

This reverts commit 7cf29d8df3b86532de20897f071b50c07d80f73f.

Original change's description:
> [wasm] always allocate memory when guard regions are needed
>
> When using trap handlers, memory references do not get any checks inserted. This
> means there is no check for a null memory as happens when the memory size is
> 0. Normally this would be correctly caught as an out of bounds access, since the
> low memory addresses are not normally mapped. However, if they were mapped for
> some reason, we would not catch the out of bounds access.
>
> The fix is to ensure WebAssembly instances always have a guard region even if
> the memory is size 0.
>
> Bug:  chromium:769637 

Change-Id: I09fdaea92b7ccb3a6cc9e28392171ec098538a00
Reviewed-on: https://chromium-review.googlesource.com/695812
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48293}
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/heap/array-buffer-tracker-inl.h
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/heap/array-buffer-tracker.h
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/heap/heap.cc
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/heap/heap.h
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/wasm/module-compiler.cc
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/wasm/wasm-memory.cc
[modify] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/src/wasm/wasm-objects.cc
[add] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/test/mjsunit/regress/wasm/regression-769637.js

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

commit 841ca52c816aefe9944ac0f910346bf85bc1a63e
Author: Eric Holk <eholk@chromium.org>
Date: Wed Oct 04 17:21:07 2017

Revert "Reland "[wasm] always allocate memory when guard regions are needed""

This reverts commit 5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc.

Reason for revert: tsan failures - https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/17574

Original change's description:
> Reland "[wasm] always allocate memory when guard regions are needed"
> 
> This reverts commit 7cf29d8df3b86532de20897f071b50c07d80f73f.
> 
> Original change's description:
> > [wasm] always allocate memory when guard regions are needed
> >
> > When using trap handlers, memory references do not get any checks inserted. This
> > means there is no check for a null memory as happens when the memory size is
> > 0. Normally this would be correctly caught as an out of bounds access, since the
> > low memory addresses are not normally mapped. However, if they were mapped for
> > some reason, we would not catch the out of bounds access.
> >
> > The fix is to ensure WebAssembly instances always have a guard region even if
> > the memory is size 0.
> >
> > Bug:  chromium:769637 
> 
> Change-Id: I09fdaea92b7ccb3a6cc9e28392171ec098538a00
> Reviewed-on: https://chromium-review.googlesource.com/695812
> Commit-Queue: Eric Holk <eholk@chromium.org>
> Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48293}

TBR=gdeepti@chromium.org,mtrofin@chromium.org,mlippautz@chromium.org,eholk@chromium.org,eholk@google.com,clemensh@chromium.org

Change-Id: I52d5354126158a92602b08c48703d562ac95075b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/699599
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48294}
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/heap/array-buffer-tracker-inl.h
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/heap/array-buffer-tracker.h
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/heap/heap.cc
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/heap/heap.h
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/wasm/module-compiler.cc
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/wasm/wasm-memory.cc
[modify] https://crrev.com/841ca52c816aefe9944ac0f910346bf85bc1a63e/src/wasm/wasm-objects.cc
[delete] https://crrev.com/5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc/test/mjsunit/regress/wasm/regression-769637.js

ClusterFuzz testcase 6697393935613952 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.

Sorry, the fix has been hard to make stick, but it will land soon.

https://crrev.com/c/702657

M63 is branching on this Thursday (10/12) and M63 beta promotion is coming very soon. Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP. Thank you.

If the fix does not stick, please consider switching trap handlers off again for 6.3.

eholk@ - how's the fix looking?

The fix is still in progress. It turns out there are deeper issues going on that will take a more in-depth design to resolve well.

As far as the status of trap handlers goes, we disabled them in Chrome via Finch after discovering this bug. We will not re-enable them until this and all related issues are fixed. In D8, trap handlers are still on in most cases, as this is useful for finding bugs and ensuring test coverage. We do not use trap handlers on the sanitizer bots because of address space exhaustion issues, which I'll say more on later.

There are several issues at play here, with separate but related fixes:

1. Guard regions always need to be allocated (https://crrev.com/c/702657, in progress). In the old bounds check system, we'd just use nullptr for 0-size memories. They would never be dereferenced because the bounds check would catch errors first. Now we use the dereference and a SIGSEGV handler to catch the out of bounds error, meaning we can't just use nullptr.

2. Guard regions need to protect against underflow as well (https://crrev.com/c/707720, in progress). The Wasm spec says address calculation needs to always be unsigned, but TurboFan often stores offsets in the 32-bit immediate field of the memory instructions. On x64_64, these get sign extended, meaning we read memory below addresses we should be reading.

3. Address space exhaustion. Guard regions are large. We have some tests that create a large number of Wasm instances, and these can cause us to run out of address space even though the amount of committed memory is rather small. This is particularly a problem on stress and sanitizer bots. I am working on a design doc to describe a strategy for Wasm to limit the amount of address space it uses and fall back on bounds checks when address space is short.

4. Related, Clusterfuzz needs to allow user segv handlers. We landed https://crrev.com/c/713956, which lets the D8 signal handler fall back on the previous handler when the fault was not an expected Wasm fault. This will let Clusterfuzz safely allow user segv handlers. Note that this does not affect Chrome, which uses a different mechanism to integrate Breakpad with trap handlers.

Given that trap handlers are now disabled in all customer-facing cases, I would recommend removing the ReleaseBlock labels.

M63 beta promotion is coming VERY soon. Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you.

Thanks for the update, eholk@.  One thing to note is that Finch doesn't get everywhere, e.g. non-Chrome Chromium browsers, and some enterprises who block some of Chrome's network chatter. But pushing this to RB-S seems reasonable. Updating labels.

ClusterFuzz testcase 4912585312043008 is flaky and no longer crashes, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

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

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

commit d31dff841e5be4d89e43434ef08f2cc5659f8cfa
Author: Eric Holk <eholk@chromium.org>
Date: Thu Mar 22 19:44:17 2018

[wasm] always allocate memory when guard regions are needed

When using trap handlers, memory references do not get any checks inserted. This
means there is no check for a null memory as happens when the memory size is
0. Normally this would be correctly caught as an out of bounds access, since the
low memory addresses are not normally mapped. However, if they were mapped for
some reason, we would not catch the out of bounds access.

The fix is to ensure WebAssembly instances always have a guard region even if
the memory is size 0.

This is a rewrite of 5e76ff5a4a5b0363b9fbc708661bbafc8dbd97fc

Note that this can lead to a large amount of unnecessary address space usage,
so we share a single reservation for empty array buffers.

Bug:  chromium:769637 

Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ia8e84be6d595e347d3d342959f2c374db1a3f683
Reviewed-on: https://chromium-review.googlesource.com/702657
Reviewed-by: Deepti Gandluri <gdeepti@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52163}
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/heap/array-buffer-tracker.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/objects.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/objects/js-array-inl.h
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/objects/js-array.h
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/runtime/runtime-test.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/wasm/module-compiler.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/wasm/wasm-memory.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/wasm/wasm-memory.h
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/src/wasm/wasm-objects.cc
[modify] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/test/cctest/wasm/test-run-wasm-module.cc
[add] https://crrev.com/d31dff841e5be4d89e43434ef08f2cc5659f8cfa/test/mjsunit/regress/wasm/regression-769637.js

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

commit cd782a034569f7c84c74207b9ad66eae34256b0b
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Apr 09 20:45:22 2018

[wasm] Rename regression test to follow convention

R=eholk@chromium.org

Bug:  chromium:769637 
Change-Id: I347ed1cf6fe567f5a12a8191b224a27336a757d4
Reviewed-on: https://chromium-review.googlesource.com/1000700
Reviewed-by: Eric Holk <eholk@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52493}
[rename] https://crrev.com/cd782a034569f7c84c74207b9ad66eae34256b0b/test/mjsunit/regress/wasm/regress-769637.js