New issue
Advanced search Search tips

Issue 672041 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

chunk->Contains(slot_addr) in remembered-set.h

Project Member Reported by ClusterFuzz, Dec 7 2016

Issue description

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

Fuzzer: v8_builtins_generator
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  chunk->Contains(slot_addr) in remembered-set.h
  
Regressed: V8: r41525:41526

Minimized Testcase (22.12 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97TWXZejzlddNawvdkdnxB-N_MAKgIb6oVf6ejCNq2V1UZYOCe991eQdG4ryc84qO4mO6mU6thMkvP7MHmA5bzOQimmhFyl0LR6wpcVK36xD6zYSOML1SKjG6I5mIxy-OEh6_au5KdhUX-aP7RM8F0RN1TBdb2T7FqTO7IYpB8hl3wkg6Q?testcase_id=5457976132632576

Issue manually filed by: titzer

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
Assigning to jgruber@ due to his recent changes to growing FixedArrays in the regex engine.
Looking at this. Minimal repro: 

// Force endless loop in @@match.
RegExp.prototype.__defineGetter__("global", () => true);
"a".match();
Cc: mlippautz@chromium.org u...@chromium.org
So what's happening:

* @@match grows its internal results array, with the new array being allocated in lospace. The allocated block is zapped in MemoryAllocator::AllocateChunk.
* We then immediately copy over the old elements to the new array.
* During copying, the store buffer overflows.
* We crash while handling the overflow because some logic is only correct if the memory location has already been initialized; but in this case we overflow the buffer while initializing.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 12 2016

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

commit 9b6808bfb5366beebe3af30a06f9851edb2039d4
Author: jgruber <jgruber@chromium.org>
Date: Mon Dec 12 13:18:18 2016

[heap] Initialize the owner on each page after lospace allocation

The least two bits of the owner field of a Page are used to determine
whether the Page is part of a large object. If these bits are not equal
to 0x11, the page is part of a large object and needs special handling
e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
belongs to.

This CL fixes an issue in which the store buffer overflows after
a large object space allocation but before the object has been fully
initialized. Store buffer overflow handling attempts to look up the
chunk of a page, but fails to do so correctly since the page's owner
field has not yet been initialized.

This CL ensures that the owner field of all pages belonging to a large
object allocation are initialized to a value that is interpreted
correctly.

BUG= chromium:672041 

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

[modify] https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4/src/heap/spaces-inl.h
[modify] https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4/src/heap/spaces.h
[add] https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4/test/mjsunit/regress/regress-672041.js

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 12 2016

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

commit 1e70454f732a959a124b4de4522d9e2e5de56066
Author: hablich <hablich@chromium.org>
Date: Mon Dec 12 14:36:34 2016

Revert of [heap] Initialize the owner on each page after lospace allocation (patchset #2 id:20001 of https://codereview.chromium.org/2565713002/ )

Reason for revert:
Tree closer: https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/12409

Original issue's description:
> [heap] Initialize the owner on each page after lospace allocation
>
> The least two bits of the owner field of a Page are used to determine
> whether the Page is part of a large object. If these bits are not equal
> to 0x11, the page is part of a large object and needs special handling
> e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
> belongs to.
>
> This CL fixes an issue in which the store buffer overflows after
> a large object space allocation but before the object has been fully
> initialized. Store buffer overflow handling attempts to look up the
> chunk of a page, but fails to do so correctly since the page's owner
> field has not yet been initialized.
>
> This CL ensures that the owner field of all pages belonging to a large
> object allocation are initialized to a value that is interpreted
> correctly.
>
> BUG= chromium:672041 
>
> Committed: https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4
> Cr-Commit-Position: refs/heads/master@{#41641}

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

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

[modify] https://crrev.com/1e70454f732a959a124b4de4522d9e2e5de56066/src/heap/spaces-inl.h
[modify] https://crrev.com/1e70454f732a959a124b4de4522d9e2e5de56066/src/heap/spaces.h
[delete] https://crrev.com/626d620d4d130485b5a4ea14bfb9097038ab3575/test/mjsunit/regress/regress-672041.js

Status: Assigned (was: Fixed)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 14 2016

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

commit bbf3c697aee6bcde1bf4e110c8aec5cdae26ebcf
Author: jgruber <jgruber@chromium.org>
Date: Wed Dec 14 06:45:22 2016

[heap] Initialize the owner on each page after lospace allocation

The least two bits of the owner field of a Page are used to determine
whether the Page is part of a large object. If these bits are not equal
to 0x11, the page is part of a large object and needs special handling
e.g. in MemoryChunk::FromAnyPointerAddress to determine which chunk it
belongs to.

This CL fixes an issue in which the store buffer overflows after
a large object space allocation but before the object has been fully
initialized. Store buffer overflow handling attempts to look up the
chunk of a page, but fails to do so correctly since the page's owner
field has not yet been initialized.

This CL ensures that the owner field of all pages belonging to a large
object allocation are initialized to a value that is interpreted
correctly.

BUG= chromium:672041 

Committed: https://crrev.com/9b6808bfb5366beebe3af30a06f9851edb2039d4
Review-Url: https://codereview.chromium.org/2565713002
Cr-Original-Commit-Position: refs/heads/master@{#41641}
Cr-Commit-Position: refs/heads/master@{#41687}

[modify] https://crrev.com/bbf3c697aee6bcde1bf4e110c8aec5cdae26ebcf/src/heap/spaces-inl.h
[modify] https://crrev.com/bbf3c697aee6bcde1bf4e110c8aec5cdae26ebcf/src/heap/spaces.cc
[modify] https://crrev.com/bbf3c697aee6bcde1bf4e110c8aec5cdae26ebcf/src/heap/spaces.h
[add] https://crrev.com/bbf3c697aee6bcde1bf4e110c8aec5cdae26ebcf/test/mjsunit/regress/regress-672041.js

Status: Fixed (was: Assigned)
Project Member

Comment 10 by ClusterFuzz, Dec 14 2016

ClusterFuzz has detected this issue as fixed in range 41686:41687.

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

Fuzzer: v8_builtins_generator
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  chunk->Contains(slot_addr) in remembered-set.h
  
Regressed: V8: r41525:41526
Fixed: V8: r41686:41687

Minimized Testcase (22.12 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97TWXZejzlddNawvdkdnxB-N_MAKgIb6oVf6ejCNq2V1UZYOCe991eQdG4ryc84qO4mO6mU6thMkvP7MHmA5bzOQimmhFyl0LR6wpcVK36xD6zYSOML1SKjG6I5mIxy-OEh6_au5KdhUX-aP7RM8F0RN1TBdb2T7FqTO7IYpB8hl3wkg6Q?testcase_id=5457976132632576

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

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

Sign in to add a comment