From 277a1268c0b6f5979dcfbcc199e28160fbb2b6a8 Mon Sep 17 00:00:00 2001 From: elsid Date: Thu, 19 Aug 2021 23:23:57 +0200 Subject: [PATCH] Avoid access to removed map item To fix ASAN warning: =11799==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000436058 at pc 0x55c90acccaa8 bp 0x7f787eeac830 sp 0x7f787eeac820 READ of size 8 at 0x60c000436058 thread T18 #0 0x55c90acccaa7 in std::operator==(std::_Deque_iterator, std::_List_iterator&, std::_List_iterator*> const&, std::_Deque_iterator, std::_List_iterator&, std::_List_iterator*> const&) /usr/include/c++/11.1.0/bits/stl_deque.h:269 #1 0x55c90acccaa7 in std::deque, std::allocator > >::empty() const /usr/include/c++/11.1.0/bits/stl_deque.h:1311 #2 0x55c90acccaa7 in operator() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:350 #3 0x55c90acccaa7 in wait_until >, DetourNavigator::AsyncNavMeshUpdater::getNextJob():: > /usr/include/c++/11.1.0/condition_variable:151 #4 0x55c90acccaa7 in wait_for, DetourNavigator::AsyncNavMeshUpdater::getNextJob():: > /usr/include/c++/11.1.0/condition_variable:175 #5 0x55c90acccaa7 in DetourNavigator::AsyncNavMeshUpdater::getNextJob() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:353 #6 0x55c90accdb2d in DetourNavigator::AsyncNavMeshUpdater::process() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:257 #7 0x55c90acce464 in operator() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:98 #8 0x55c90acce464 in __invoke_impl > /usr/include/c++/11.1.0/bits/invoke.h:61 #9 0x55c90acce464 in __invoke > /usr/include/c++/11.1.0/bits/invoke.h:96 #10 0x55c90acce464 in _M_invoke<0> /usr/include/c++/11.1.0/bits/std_thread.h:253 #11 0x55c90acce464 in operator() /usr/include/c++/11.1.0/bits/std_thread.h:260 #12 0x55c90acce464 in _M_run /usr/include/c++/11.1.0/bits/std_thread.h:211 #13 0x7f78c38fd3c3 in execute_native_thread_routine /build/gcc/src/gcc/libstdc++-v3/src/c++11/thread.cc:82 #14 0x7f78c36b1258 in start_thread (/usr/lib/libpthread.so.0+0x9258) #15 0x7f78c35da5e2 in __GI___clone (/usr/lib/libc.so.6+0xfe5e2) 0x60c000436058 is located 88 bytes inside of 120-byte region [0x60c000436000,0x60c000436078) freed by thread T0 here: #0 0x7f78c688cd69 in operator delete(void*, unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172 #1 0x55c90acdbe20 in __gnu_cxx::new_allocator, std::allocator > > > > >::deallocate(std::_Rb_tree_node, std::allocator > > > >*, unsigned long) /usr/include/c++/11.1.0/ext/new_allocator.h:139 #2 0x55c90acdbe20 in std::allocator_traits, std::allocator > > > > > >::deallocate(std::allocator, std::allocator > > > > >&, std::_Rb_tree_node, std::allocator > > > >*, unsigned long) /usr/include/c++/11.1.0/bits/alloc_traits.h:492 #3 0x55c90acdbe20 in std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_put_node(std::_Rb_tree_node, std::allocator > > > >*) /usr/include/c++/11.1.0/bits/stl_tree.h:565 #4 0x55c90acdbe20 in std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_drop_node(std::_Rb_tree_node, std::allocator > > > >*) /usr/include/c++/11.1.0/bits/stl_tree.h:632 #5 0x55c90acdbe20 in std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_erase(std::_Rb_tree_node, std::allocator > > > >*) /usr/include/c++/11.1.0/bits/stl_tree.h:1889 #6 0x55c90acc0569 in std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::clear() /usr/include/c++/11.1.0/bits/stl_tree.h:1254 #7 0x55c90acc0569 in std::map, std::allocator > >, std::less, std::allocator, std::allocator > > > > >::clear() /usr/include/c++/11.1.0/bits/stl_map.h:1134 #8 0x55c90acc0569 in DetourNavigator::AsyncNavMeshUpdater::~AsyncNavMeshUpdater() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:105 #9 0x55c90acab2dc in DetourNavigator::NavigatorImpl::~NavigatorImpl() (/home/elsid/dev/openmw/build/gcc/asan/openmw+0x2d152dc) #10 0x55c9097db4b5 in std::default_delete::operator()(DetourNavigator::Navigator*) const /usr/include/c++/11.1.0/bits/unique_ptr.h:85 #11 0x55c9097db4b5 in std::unique_ptr >::~unique_ptr() /usr/include/c++/11.1.0/bits/unique_ptr.h:361 #12 0x55c9097db4b5 in MWWorld::World::~World() /home/elsid/dev/openmw/apps/openmw/mwworld/worldimp.cpp:534 #13 0x55c9097dc3a4 in MWWorld::World::~World() /home/elsid/dev/openmw/apps/openmw/mwworld/worldimp.cpp:534 #14 0x55c90a1925e4 in MWBase::Environment::cleanup() /home/elsid/dev/openmw/apps/openmw/mwbase/environment.cpp:192 #15 0x55c90a1f544d in OMW::Engine::~Engine() /home/elsid/dev/openmw/apps/openmw/engine.cpp:434 #16 0x55c90a1f6700 in OMW::Engine::~Engine() /home/elsid/dev/openmw/apps/openmw/engine.cpp:455 #17 0x55c90a19d523 in std::default_delete::operator()(OMW::Engine*) const /usr/include/c++/11.1.0/bits/unique_ptr.h:85 #18 0x55c90a19d523 in std::unique_ptr >::~unique_ptr() /usr/include/c++/11.1.0/bits/unique_ptr.h:361 #19 0x55c90a19d523 in runApplication(int, char**) /home/elsid/dev/openmw/apps/openmw/main.cpp:320 #20 0x55c90a955634 in wrapApplication(int (*)(int, char**), int, char**, std::__cxx11::basic_string, std::allocator > const&) /home/elsid/dev/openmw/components/debug/debugging.cpp:205 #21 0x55c90a193be0 in main /home/elsid/dev/openmw/apps/openmw/main.cpp:328 #22 0x7f78c3503b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) previously allocated by thread T18 here: #0 0x7f78c688bca1 in operator new(unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x55c90ace8c73 in __gnu_cxx::new_allocator, std::allocator > > > > >::allocate(unsigned long, void const*) /usr/include/c++/11.1.0/ext/new_allocator.h:121 #2 0x55c90ace8c73 in std::allocator_traits, std::allocator > > > > > >::allocate(std::allocator, std::allocator > > > > >&, unsigned long) /usr/include/c++/11.1.0/bits/alloc_traits.h:460 #3 0x55c90ace8c73 in std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_get_node() /usr/include/c++/11.1.0/bits/stl_tree.h:561 #4 0x55c90ace8c73 in std::_Rb_tree_node, std::allocator > > > >* std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_create_node, std::tuple<> >(std::piecewise_construct_t const&, std::tuple&&, std::tuple<>&&) /usr/include/c++/11.1.0/bits/stl_tree.h:611 #5 0x55c90ace8c73 in std::_Rb_tree_iterator, std::allocator > > > > std::_Rb_tree, std::allocator > > >, std::_Select1st, std::allocator > > > >, std::less, std::allocator, std::allocator > > > > >::_M_emplace_hint_unique, std::tuple<> >(std::_Rb_tree_const_iterator, std::allocator > > > >, std::piecewise_construct_t const&, std::tuple&&, std::tuple<>&&) /usr/include/c++/11.1.0/bits/stl_tree.h:2429 #6 0x55c90accc5fb in std::map, std::allocator > >, std::less, std::allocator, std::allocator > > > > >::operator[](std::thread::id const&) /usr/include/c++/11.1.0/bits/stl_map.h:501 #7 0x55c90accc5fb in DetourNavigator::AsyncNavMeshUpdater::getNextJob() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:344 #8 0x55c90accdb2d in DetourNavigator::AsyncNavMeshUpdater::process() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:257 #9 0x55c90acce464 in operator() /home/elsid/dev/openmw/components/detournavigator/asyncnavmeshupdater.cpp:98 #10 0x55c90acce464 in __invoke_impl > /usr/include/c++/11.1.0/bits/invoke.h:61 #11 0x55c90acce464 in __invoke > /usr/include/c++/11.1.0/bits/invoke.h:96 #12 0x55c90acce464 in _M_invoke<0> /usr/include/c++/11.1.0/bits/std_thread.h:253 #13 0x55c90acce464 in operator() /usr/include/c++/11.1.0/bits/std_thread.h:260 #14 0x55c90acce464 in _M_run /usr/include/c++/11.1.0/bits/std_thread.h:211 #15 0x7f78c38fd3c3 in execute_native_thread_routine /build/gcc/src/gcc/libstdc++-v3/src/c++11/thread.cc:82 Thread T18 created by T0 here: #0 0x7f78c682bfa7 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:216 #1 0x7f78c38fd6aa in std::thread::_M_start_thread(std::unique_ptr >, void (*)()) /build/gcc/src/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 #2 0x55c90ae008d1 in DetourNavigator::NavMeshManager::NavMeshManager(DetourNavigator::Settings const&) /home/elsid/dev/openmw/components/detournavigator/navmeshmanager.cpp:47 #3 0x55c90aca3bfe in DetourNavigator::NavigatorImpl::NavigatorImpl(DetourNavigator::Settings const&) /home/elsid/dev/openmw/components/detournavigator/navigatorimpl.cpp:13 #4 0x55c9097d9e6f in MWWorld::World::World(osgViewer::Viewer*, osg::ref_ptr, Resource::ResourceSystem*, SceneUtil::WorkQueue*, Files::Collections const&, std::vector, std::allocator >, std::allocator, std::allocator > > > const&, std::vector, std::allocator >, std::allocator, std::allocator > > > const&, ToUTF8::Utf8Encoder*, int, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&) /home/elsid/dev/openmw/apps/openmw/mwworld/worldimp.cpp:196 #5 0x55c90a1e9992 in OMW::Engine::prepareEngine(Settings::Manager&) /home/elsid/dev/openmw/apps/openmw/engine.cpp:789 #6 0x55c90a1ed138 in OMW::Engine::go() /home/elsid/dev/openmw/apps/openmw/engine.cpp:949 #7 0x55c90a19d4cb in runApplication(int, char**) /home/elsid/dev/openmw/apps/openmw/main.cpp:316 #8 0x55c90a955634 in wrapApplication(int (*)(int, char**), int, char**, std::__cxx11::basic_string, std::allocator > const&) /home/elsid/dev/openmw/components/debug/debugging.cpp:205 #9 0x55c90a193be0 in main /home/elsid/dev/openmw/apps/openmw/main.cpp:328 #10 0x7f78c3503b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24) SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/c++/11.1.0/bits/stl_deque.h:269 in std::operator==(std::_Deque_iterator, std::_List_iterator&, std::_List_iterator*> const&, std::_Deque_iterator, std::_List_iterator&, std::_List_iterator*> const&) Shadow bytes around the buggy address: 0x0c188007ebb0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x0c188007ebc0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c188007ebd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c188007ebe0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c188007ebf0: 00 00 00 00 00 00 07 fa fa fa fa fa fa fa fa fa =>0x0c188007ec00: fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fa 0x0c188007ec10: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c188007ec20: 00 00 00 00 00 00 02 fa fa fa fa fa fa fa fa fa 0x0c188007ec30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c188007ec40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x0c188007ec50: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa 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 Shadow gap: cc ==11799==ABORTING --- components/detournavigator/asyncnavmeshupdater.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/components/detournavigator/asyncnavmeshupdater.cpp b/components/detournavigator/asyncnavmeshupdater.cpp index 7fcfdf98c5..562420955d 100644 --- a/components/detournavigator/asyncnavmeshupdater.cpp +++ b/components/detournavigator/asyncnavmeshupdater.cpp @@ -345,8 +345,12 @@ namespace DetourNavigator while (true) { + bool shouldStop = false; + const auto hasJob = [&] { - return (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now()) + shouldStop = mShouldStop; + return shouldStop + || (!mWaiting.empty() && mWaiting.front()->mProcessTime <= std::chrono::steady_clock::now()) || !threadQueue.empty(); }; @@ -357,6 +361,9 @@ namespace DetourNavigator return mJobs.end(); } + if (shouldStop) + return mJobs.end(); + Log(Debug::Debug) << "Got " << mJobs.size() << " navigator jobs and " << threadQueue.size() << " thread jobs by thread=" << std::this_thread::get_id();