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

Issue 713293 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

CFI: invalid cast in HeapAllocator

Project Member Reported by krasin@chromium.org, Apr 19 2017

Issue description

Chrome Version: tip
OS: Linux x86-64

What steps will reproduce the problem?
(1) Build blink_heap_unittests with Control Flow Integrity enabled:

GYP_DEFINES='buildtype=Official' gclient sync
gn gen out/cfi '--args=is_debug=false is_cfi=true use_cfi_cast=true use_cfi_diag=true' --check
ninja -C out/cfi blink_heap_unittests


(2) Run the test:
$ ./out/cfi-tot/blink_heap_unittests --gtest_filter=HeapTest.VectorDestructorsWithVtable
IMPORTANT DEBUGGING NOTE: batches of tests are run inside their
own process. For debugging a test inside a debugger, use the
--gtest_filter=<your_test_name> flag along with
--single-process-tests.
Using sharding settings from environment. This is shard 0/1
Using 1 parallel jobs.
Note: Google Test filter = HeapTest.VectorDestructorsWithVtable
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from HeapTest
[ RUN      ] HeapTest.VectorDestructorsWithVtable
../../third_party/WebKit/Source/platform/heap/HeapAllocator.h:73:12: runtime error: control flow integrity check for type 'blink::InlinedVectorObjectWithVtable' failed during cast to unrelated type (vtable address 0x000000000000)
0x000000000000: note: invalid vtable
<memory cannot be printed>
../../third_party/WebKit/Source/platform/heap/HeapAllocator.h:98:12: runtime error: control flow integrity check for type 'blink::InlinedVectorObjectWithVtable' failed during cast to unrelated type (vtable address 0x000000000000)
0x000000000000: note: invalid vtable

What happens here is the case of non-allocated memory to a class pointer with virtual a virtual table. CFI check correctly complains about invalid vtable pointer.

This breaks at least one bot:
https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT/builds/6011

We have seen such issues in the past, and the solution is to call New before making a cast to the specific type. In this case, the code becomes correct and functionally equivalent to the desire expressed by the existing code.

I will try to make a fix.
 

Comment 1 by krasin@chromium.org, Apr 19 2017

Labels: -Pri-3 Pri-1

Comment 2 by krasin@chromium.org, Apr 19 2017

Actually, the issue is somewhat known, as we have an entry in cfi blacklist. I am going to update the cfi blacklist to fix the bot, and then think about a proper fix.

Comment 3 by krasin@chromium.org, Apr 19 2017

This is what we have in tools/cfi/blacklist:

# WTF allocators.
fun:*allocate*Backing*

Comment 4 by krasin@chromium.org, Apr 19 2017

And the immediate trigger was a rename from allocate*Backing to Allocate*Backing. We really need to get rid of such entries in the CFI blacklist.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/647872c5ada49e2c4a7985fcca5c4a93c5bb1760

commit 647872c5ada49e2c4a7985fcca5c4a93c5bb1760
Author: krasin <krasin@chromium.org>
Date: Wed Apr 19 20:20:13 2017

Update CFI blacklist to match the recent renames in Blink.

In particular, it's a follow up to
https://chromium-review.googlesource.com/472192.

This CL is just to fix the bots. I will follow up with proper
changes to the allocator which will eliminate the need for the
blacklist entry.

BUG=713293

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

[modify] https://crrev.com/647872c5ada49e2c4a7985fcca5c4a93c5bb1760/tools/cfi/blacklist.txt

Comment 6 by thakis@chromium.org, Apr 19 2017

Blocking: 675877

Comment 7 by krasin@chromium.org, Apr 19 2017

The immediate issue is supposedly fixed. I will work on the proper fix, once we get all Clang ToT bots green (that blocks an important update to the Clang toolchain).

Comment 8 by dcheng@chromium.org, Apr 22 2017

Blocking: -675877
I think this may been marked as blocking the wrong bug (issue 675877 is about resolving remaining issues for the Blink rename)

Sign in to add a comment