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

Issue 619351 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

CFI violation in Bluetooth tests

Project Member Reported by krasin@chromium.org, Jun 12 2016

Issue description

Version: tip
OS: Linux x86-64

What steps will reproduce the problem?
(1) Build content_unittests with CFI:

$ gn gen out/gn-cfi-diag '--args=is_cfi=true use_cfi_diag=true is_debug=false is_component_build=false symbol_level=1 dcheck_always_on=true' --check
$ build/download_gold_plugin.py
$ ninja -C out/gn-cfi-diag content_unittests

Note: this requires a Linux x86-64 machine with at least 64 GB RAM. CFI relies on LTO (Link Time Optimization) and that eats a lot of memory.

(2) Run the test under GDB:

gdb --args ./out/gn-cfi-diag/content_unittests --gtest_filter=BluetoothBlacklistTest.RemoveExcludedUUIDs_NonMatching --no-sandbox --single_process
(gdb) b __ubsan::HandleCFIBadType
(gdb) r

What is the expected output?

Test passes, not CFI violations reported.

What do you see instead?

Test failed: https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux/builds/5690/steps/content_unittests
(the output from reproducing locally is to follow)
 

Comment 1 by krasin@chromium.org, Jun 12 2016

This was introduced in https://codereview.chromium.org/2015463004/

Comment 2 by krasin@chromium.org, Jun 12 2016

Here is the stack trace:

Breakpoint 1, 0x0000000000505790 in __ubsan::HandleCFIBadType(__ubsan::CFICheckFailData*, unsigned long, bool, __ubsan::ReportOptions) ()
(gdb) bt
#0  0x0000000000505790 in __ubsan::HandleCFIBadType(__ubsan::CFICheckFailData*, unsigned long, bool, __ubsan::ReportOptions) ()
#1  0x000000000050485b in __ubsan_handle_cfi_check_fail ()
#2  0x000000000063328a in device::BluetoothUUID* base::AlignedMemory<32ul, 8ul>::data_as<device::BluetoothUUID>() () at ../../base/memory/aligned_memory.h:84
#3  0x00000000006331ec in base::Optional<device::BluetoothUUID>::Init(device::BluetoothUUID&&) () at ../../base/optional.h:249
#4  0x000000000063bd4c in base::Optional<std::decay<device::BluetoothUUID>::type> base::make_optional<device::BluetoothUUID>(device::BluetoothUUID&&) () at ../../base/optional.h:436
#5  0x000000000063bd15 in content::(anonymous namespace)::Canonicalize(std::string const&) () at ../../content/browser/bluetooth/bluetooth_blacklist_unittest.cc:17
#6  0x000000000063ce47 in content::BluetoothBlacklistTest_RemoveExcludedUUIDs_NonMatching_Test::TestBody() () at ../../content/browser/bluetooth/bluetooth_blacklist_unittest.cc:344
#7  0x0000000004c3c060 in testing::Test::Run() () at ../../testing/gtest/src/gtest.cc:2474
#8  0x0000000004c3db96 in testing::TestInfo::Run() () at ../../testing/gtest/src/gtest.cc:2656
#9  0x0000000004c3deb2 in testing::TestCase::Run() () at ../../testing/gtest/src/gtest.cc:2774
#10 0x0000000004c40be2 in testing::internal::UnitTestImpl::RunAllTests() () at ../../testing/gtest/src/gtest.cc:4647
#11 0x0000000004c40de9 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) () at ../../testing/gtest/src/gtest.cc:2455
#12 0x0000000004c4077b in testing::UnitTest::Run() () at ../../testing/gtest/src/gtest.cc:4255
#13 0x000000000102aca1 in base::TestSuite::Run() () at ../../base/test/test_suite.cc:230
#14 0x0000000000f14a0b in base::internal::Invoker<base::IndexSequence<0ul>, base::internal::BindState<base::internal::RunnableAdapter<int (content::UnitTestTestSuite::*)()>, int (content::UnitTestTestSuite*), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, false, int ()>::Run(base::internal::BindStateBase*) () at ../../base/bind_internal.h:364
#15 0x0000000001041db5 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1> const&, int, int, bool, base::Callback<void (), (base::internal::CopyMode)1> const&) () at ../../base/test/launcher/unit_test_launcher.cc:206
#16 0x0000000001041bef in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1> const&) () at ../../base/test/launcher/unit_test_launcher.cc:445
#17 0x0000000000f147f9 in main () at ../../content/test/run_all_unittests.cc:34

And diagnostics:

../../base/memory/aligned_memory.h:84:1: runtime error: control flow integrity check for type 'device::BluetoothUUID' failed during cast to unrelated type (vtable address 0x3bccbd0812d0)
0x3bccbd0812d0: note: invalid vtable
 69 61 67 00  01 00 00 00 00 00 00 00  c0 13 08 bd cc 3b 00 00  30 12 08 bd cc 3b 00 00  f0 b5 0f bd
              ^ 

So, it's likely either use-before-new, or use-after-destroy. Taking a closer look at the source line in the trace.

Comment 3 by krasin@chromium.org, Jun 12 2016

Cc: krasin@chromium.org
Owner: p...@chromium.org
I tried to follow the code, and I don't have an explanation what happens where. My C++ Fu is not strong enough.

Peter, can you please take a look at this failure?

Comment 4 by ortuno@chromium.org, Jun 14 2016

Components: Blink>Bluetooth
Owner: ortuno@chromium.org
Status: Started (was: Untriaged)
So I've been looking at this. I have a change that fixes this issue but I have no idea why it works. The only reason I did the change was because Optional's use of base::AlignedMemory[1] differs from what the documentation suggests[2].

jyasskin: Do you know what's going on?

Here is my patch: http://crrev.com/2066053002

[1] https://cs.chromium.org/chromium/src/base/optional.h?dr=Ss&sq=package:chromium&rcl=1465903141&l=249
[2] https://cs.chromium.org/chromium/src/base/memory/aligned_memory.h?sq=package:chromium&dr=CSs&rcl=1465903141&l=14

Comment 5 by krasin@chromium.org, Jun 15 2016

This is the correct fix. See the inline comments on the CL.

Thank you for working on this!
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 15 2016

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

commit 3fd03b556675834c683e1453a168a874269b83fc
Author: ortuno <ortuno@chromium.org>
Date: Wed Jun 15 18:10:34 2016

Fix AlignedMemory initialization in base/optional.h

AlignedMemory::data_as<T>() casts a void pointer to AlignedMemory::data_ to a T
pointer. Before initializing an AlignedMemory instance, AlignedMemory::data_
contains garbage so trying to cast a void pointer to it to a T pointer results in
a CFI violation.

BUG= 619351 

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

[modify] https://crrev.com/3fd03b556675834c683e1453a168a874269b83fc/base/optional.h

Comment 7 by ortuno@chromium.org, Jun 15 2016

Status: Fixed (was: Started)

Sign in to add a comment