Opened on 04/06/2017 at 01:46:36 PM

Closed on 04/06/2017 at 03:55:06 PM

#5116 closed defect (fixed)

DefaultTimer crash

Reported by: hfiguiere Assignee:
Priority: P1 Milestone:
Module: Libadblockplus Keywords:
Cc: sergz Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

​https://codereview.adblockplus.org/29404607/

Description

Environment

Linux or macOS. Build using the AddressSanitizer from clang.

How to reproduce

I run the test with:

make CXXFLAGS="-O1 -fno-optimize-sibling-calls -fsanitize=address -fno-omit-frame-pointer" LDFLAGS=-fsanitize=address test

And it crashed with:

==41589==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600001afc0 at pc 0x000108235bb4 bp 0x70000c7edbf0 sp 0x70000c7edbe8
READ of size 8 at 0x60600001afc0 thread T54
    #0 0x108235bb3 in std::__1::cv_status std::__1::condition_variable::wait_until<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >(std::__1::unique_lock<std::__1::mutex>&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > const&) chrono:778
    #1 0x10829e882 in AdblockPlus::DefaultTimer::ThreadFunc() DefaultTimer.cpp:66
    #2 0x10829f374 in AdblockPlus::DefaultTimer::DefaultTimer()::$_0::operator()() const DefaultTimer.cpp:27
    #3 0x10829f28c in std::__1::__thread_proxy<std::__1::tuple<AdblockPlus::DefaultTimer::DefaultTimer()::$_0> >(void*, void*) __functional_base:416
    #4 0x7fffe2fe09ae in _pthread_body (libsystem_pthread.dylib:x86_64+0x39ae)
    #5 0x7fffe2fe08fa in _pthread_start (libsystem_pthread.dylib:x86_64+0x38fa)
    #6 0x7fffe2fe0100 in thread_start (libsystem_pthread.dylib:x86_64+0x3100)

0x60600001afc0 is located 0 bytes inside of 64-byte region [0x60600001afc0,0x60600001b000)
freed by thread T0 here:
    #0 0x10a023d6b in wrap__ZdlPv (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x60d6b)
    #1 0x1082a00b3 in std::__1::__split_buffer<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&>::~__split_buffer() new:177
    #2 0x10829fd08 in std::__1::__split_buffer<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&>::~__split_buffer() __split_buffer:340
    #3 0x10829f6c6 in void std::__1::vector<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit> >::__push_back_slow_path<AdblockPlus::DefaultTimer::TimerUnit const&>(AdblockPlus::DefaultTimer::TimerUnit const&&&) vector:1580
    #4 0x10829e4bf in AdblockPlus::DefaultTimer::SetTimer(std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000l> > const&, std::__1::function<void ()> const&) vector:1596
    #5 0x1082beaff in AdblockPlus::JsEngine::ScheduleTimer(v8::Arguments const&) JsEngine.cpp:108
    #6 0x1082bd431 in (anonymous namespace)::SetTimeoutCallback(v8::Arguments const&) GlobalJsObject.cpp:40
    #7 0x1083b4cb5 in v8::internal::FunctionCallbackArguments::Call(v8::Handle<v8::Value> (*)(v8::Arguments const&)) arguments.cc:109
    #8 0x108406014 in v8::internal::MaybeObject* v8::internal::HandleApiCallHelper<false>(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) builtins.cc:1272
    #9 0x108405b98 in v8::internal::Builtin_Impl_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) builtins.cc:1289
    #10 0x1083f53eb in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) builtins.cc:1288
    #11 0xf5ea27072cd  (<unknown module>)
    #12 0xf5ea2770daf  (<unknown module>)
    #13 0xf5ea2770c70  (<unknown module>)
    #14 0xf5ea27108f3  (<unknown module>)
    #15 0xf5ea2770855  (<unknown module>)
    #16 0xf5ea272ab36  (<unknown module>)
    #17 0xf5ea2772fac  (<unknown module>)
    #18 0xf5ea2772d1d  (<unknown module>)
    #19 0xf5ea2772e60  (<unknown module>)
    #20 0xf5ea272aee3  (<unknown module>)
    #21 0xf5ea2717e96  (<unknown module>)
    #22 0x108491881 in v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*) execution.cc:119
    #23 0x108490d71 in v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool) execution.cc:182
    #24 0x10833dfc5 in v8::Script::Run() api.cc:2039
    #25 0x1082c0bfb in AdblockPlus::JsEngine::Evaluate(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) JsEngine.cpp:163
    #26 0x1082abec0 in AdblockPlus::FilterEngine::CreateAsync(std::__1::shared_ptr<AdblockPlus::JsEngine> const&, std::__1::function<void (std::__1::shared_ptr<AdblockPlus::FilterEngine> const&)> const&, AdblockPlus::FilterEngine::CreationParameters const&) FilterEngine.cpp:204
    #27 0x1082ac4a5 in AdblockPlus::FilterEngine::Create(std::__1::shared_ptr<AdblockPlus::JsEngine> const&, AdblockPlus::FilterEngine::CreationParameters const&) FilterEngine.cpp:212
    #28 0x108231831 in (anonymous namespace)::FilterEngineTestGeneric<LazyFileSystem, AdblockPlus::DefaultLogSystem>::SetUp() FilterEngine.cpp:58
    #29 0x108322861 in testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) gtest.cc:2402

previously allocated by thread T0 here:
    #0 0x10a02376b in wrap__Znwm (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x6076b)
    #1 0x10829fd8c in std::__1::__split_buffer<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&>::__split_buffer(unsigned long, unsigned long, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&) new:169
    #2 0x10829fa88 in std::__1::__split_buffer<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&>::__split_buffer(unsigned long, unsigned long, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit>&) __split_buffer:310
    #3 0x10829f676 in void std::__1::vector<AdblockPlus::DefaultTimer::TimerUnit, std::__1::allocator<AdblockPlus::DefaultTimer::TimerUnit> >::__push_back_slow_path<AdblockPlus::DefaultTimer::TimerUnit const&>(AdblockPlus::DefaultTimer::TimerUnit const&&&) vector:1575
    #4 0x10829e4bf in AdblockPlus::DefaultTimer::SetTimer(std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000l> > const&, std::__1::function<void ()> const&) vector:1596
    #5 0x1082beaff in AdblockPlus::JsEngine::ScheduleTimer(v8::Arguments const&) JsEngine.cpp:108
    #6 0x1082bd431 in (anonymous namespace)::SetTimeoutCallback(v8::Arguments const&) GlobalJsObject.cpp:40
    #7 0x1083b4cb5 in v8::internal::FunctionCallbackArguments::Call(v8::Handle<v8::Value> (*)(v8::Arguments const&)) arguments.cc:109
    #8 0x108406014 in v8::internal::MaybeObject* v8::internal::HandleApiCallHelper<false>(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) builtins.cc:1272
    #9 0x108405b98 in v8::internal::Builtin_Impl_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) builtins.cc:1289
    #10 0x1083f53eb in v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) builtins.cc:1288
    #11 0xf5ea27072cd  (<unknown module>)
    #12 0xf5ea2770daf  (<unknown module>)
    #13 0xf5ea2770c70  (<unknown module>)
    #14 0xf5ea27108f3  (<unknown module>)
    #15 0xf5ea2770855  (<unknown module>)
    #16 0xf5ea271b50f  (<unknown module>)
    #17 0xf5ea2770006  (<unknown module>)
    #18 0xf5ea276e618  (<unknown module>)
    #19 0xf5ea276e760  (<unknown module>)
    #20 0xf5ea272aee3  (<unknown module>)
    #21 0xf5ea2717e96  (<unknown module>)
    #22 0x108491881 in v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*) execution.cc:119
    #23 0x108490d71 in v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool) execution.cc:182
    #24 0x10833dfc5 in v8::Script::Run() api.cc:2039
    #25 0x1082c0bfb in AdblockPlus::JsEngine::Evaluate(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) JsEngine.cpp:163
    #26 0x1082abec0 in AdblockPlus::FilterEngine::CreateAsync(std::__1::shared_ptr<AdblockPlus::JsEngine> const&, std::__1::function<void (std::__1::shared_ptr<AdblockPlus::FilterEngine> const&)> const&, AdblockPlus::FilterEngine::CreationParameters const&) FilterEngine.cpp:204
    #27 0x1082ac4a5 in AdblockPlus::FilterEngine::Create(std::__1::shared_ptr<AdblockPlus::JsEngine> const&, AdblockPlus::FilterEngine::CreationParameters const&) FilterEngine.cpp:212
    #28 0x108231831 in (anonymous namespace)::FilterEngineTestGeneric<LazyFileSystem, AdblockPlus::DefaultLogSystem>::SetUp() FilterEngine.cpp:58
    #29 0x108322861 in testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) gtest.cc:2402

Thread T54 created by T0 here:
    #0 0x10a00fca6 in wrap_pthread_create (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x4cca6)
    #1 0x10829f1ff in std::__1::thread::thread<AdblockPlus::DefaultTimer::DefaultTimer()::$_0, void>(AdblockPlus::DefaultTimer::DefaultTimer()::$_0&&) thread:369
    #2 0x10829e108 in std::__1::thread::thread<AdblockPlus::DefaultTimer::DefaultTimer()::$_0, void>(AdblockPlus::DefaultTimer::DefaultTimer()::$_0&&) thread:365
    #3 0x10829df0f in AdblockPlus::DefaultTimer::DefaultTimer() DefaultTimer.cpp:25
    #4 0x10829e128 in AdblockPlus::DefaultTimer::DefaultTimer() DefaultTimer.cpp:24
    #5 0x1082be33e in AdblockPlus::CreateDefaultTimer() JsEngine.cpp:72
    #6 0x1081f4898 in CreateJsEngine(AdblockPlus::AppInfo const&) BaseJsTest.cpp:23
    #7 0x1081fe320 in BaseJsTest::SetUp() BaseJsTest.h:150
    #8 0x1082316f9 in (anonymous namespace)::FilterEngineTestGeneric<LazyFileSystem, AdblockPlus::DefaultLogSystem>::SetUp() FilterEngine.cpp:54
    #9 0x108322861 in testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) gtest.cc:2402
    #10 0x108305d83 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) gtest.cc:2438
    #11 0x108305bab in testing::Test::Run() gtest.cc:2470
    #12 0x108306f90 in testing::TestInfo::Run() gtest.cc:2656
    #13 0x108308011 in testing::TestCase::Run() gtest.cc:2774
    #14 0x108313f61 in testing::internal::UnitTestImpl::RunAllTests() gtest.cc:4649
    #15 0x108322861 in testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) gtest.cc:2402
    #16 0x108313943 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) gtest.cc:2438
    #17 0x10831371b in testing::UnitTest::Run() gtest.cc:4257
    #18 0x108295510 in RUN_ALL_TESTS() gtest.h:2233
    #19 0x108295497 in main gtest_main.cc:37
    #20 0x7fffe2dc7234 in start (libdyld.dylib:x86_64+0x5234)

SUMMARY: AddressSanitizer: heap-use-after-free chrono:778 in std::__1::cv_status std::__1::condition_variable::wait_until<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >(std::__1::unique_lock<std::__1::mutex>&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > const&)
Shadow bytes around the buggy address:
  0x1c0c000035a0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x1c0c000035b0: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x1c0c000035c0: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x1c0c000035d0: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x1c0c000035e0: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
=>0x1c0c000035f0: fd fd fd fd fa fa fa fa[fd]fd fd fd fd fd fd fd
  0x1c0c00003600: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x1c0c00003610: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x1c0c00003620: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd
  0x1c0c00003630: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x1c0c00003640: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==41589==ABORTING

Attachments (0)

Change History (5)

comment:1 Changed on 04/06/2017 at 01:47:00 PM by hfiguiere

  • Cc sergz added

comment:2 Changed on 04/06/2017 at 01:55:53 PM by sergz

  • Component changed from Unknown to Libadblockplus
  • Priority changed from Unknown to P1

I suspect that we don't see the full stack trace and timers.top().fireAt (which is a reference to value stored in std::vector) is passed by reference and afterwards likely by pointer (it probably calls either ​http://en.cppreference.com/w/c/thread/cnd_timedwait or its non-public analog which accepts a pointer to timeout) and the implementation of waiting is periodically trying to read the memory at this pointer, but meanwhile we invalidate it by adding new timers.

I propose to create a local copy of timers.top().fireAt and pass it into wait_until

auto fireAt = timers.top().fireAt;
conditionVariable.wait_until(lock, fireAt);

comment:3 Changed on 04/06/2017 at 03:35:15 PM by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed on 04/06/2017 at 03:53:31 PM by abpbot

A commit referencing this issue has landed:
​Issue 5116 - Copy the timer delay before waiting for it.

comment:5 Changed on 04/06/2017 at 03:55:06 PM by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.