Flaky UAF in syncer::AttachmentDownloaderImpl::OnGetTokenFailure |
|||||
Issue descriptionI saw this happening a few times today: https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%281%29/builds/47261/ https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%281%29/builds/47262/ https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%281%29/builds/47264/ Invalid read of size 8 std::basic_string<char, std::char_traits<char>, std::allocator<char> >::size() const (/build/buildd/gcc-4.6-4.6.3/build/x86_64-linux-gnu/libstdc++-v3/include/bits/basic_string.h:711) _ZSteqIcEN9__gnu_cxx11__enable_ifIXsr9__is_charIT_EE7__valueEbE6__typeERKSbIS2_St11char_traitsIS2_ESaIS2_EESA_ (build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/basic_string.h:2436) std::equal_to<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >::operator()(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/sync_unit_tests) std::__detail::_Hash_code_base<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, std::_Select1st<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::equal_to<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, false>::_M_compare(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned long, std::__detail::_Hash_node<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, false>*) const (/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/sync_unit_tests) std::_Hashtable<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::_Select1st<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::equal_to<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, false, false, true>::erase(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/hashtable.h:1101) syncer::AttachmentDownloaderImpl::OnGetTokenFailure(OAuth2TokenService::Request const*, GoogleServiceAuthError const&) (sync/internal_api/attachments/attachment_downloader_impl.cc:142) non-virtual thunk to syncer::AttachmentDownloaderImpl::OnGetTokenFailure(OAuth2TokenService::Request const*, GoogleServiceAuthError const&) (sync/internal_api/attachments/attachment_downloader_impl.cc:126) (anonymous namespace)::RequestCore::InformOwnerOnGetTokenFailure(GoogleServiceAuthError) (google_apis/gaia/oauth2_token_service_request.cc:267) Address 0xa9b6a00 is 0 bytes inside a block of size 135 free'd operator delete(void*) (m_replacemalloc/vg_replace_malloc.c:1149) std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() (/build/buildd/gcc-4.6-4.6.3/build/x86_64-linux-gnu/libstdc++-v3/include/bits/basic_string.h:235) std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >::~pair() (/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/sync_unit_tests) std::__detail::_Hash_node<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, false>::~_Hash_node() (/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/sync_unit_tests) __gnu_cxx::new_allocator<std::__detail::_Hash_node<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, false> >::destroy(std::__detail::_Hash_node<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, false>*) (/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/sync_unit_tests) std::_Hashtable<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::_Select1st<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::equal_to<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, false, false, true>::_M_deallocate_node(std::__detail::_Hash_node<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, false>*) (build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/hashtable.h:505) std::_Hashtable<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > >, std::allocator<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::_Select1st<std::pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<syncer::AttachmentDownloaderImpl::DownloadState, std::default_delete<syncer::AttachmentDownloaderImpl::DownloadState> > > >, std::equal_to<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, false, false, true>::erase(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/hashtable.h:1111) syncer::AttachmentDownloaderImpl::OnGetTokenFailure(OAuth2TokenService::Request const*, GoogleServiceAuthError const&) (sync/internal_api/attachments/attachment_downloader_impl.cc:142) non-virtual thunk to syncer::AttachmentDownloaderImpl::OnGetTokenFailure(OAuth2TokenService::Request const*, GoogleServiceAuthError const&) (sync/internal_api/attachments/attachment_downloader_impl.cc:126) (anonymous namespace)::RequestCore::InformOwnerOnGetTokenFailure(GoogleServiceAuthError) (google_apis/gaia/oauth2_token_service_request.cc:267) Suppression (error hash=#EB6008B8B7537A7B#): For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports { <insert_a_suppression_name_here> Memcheck:Unaddressable fun:_ZNKSs4sizeEv fun:_ZSteqIcEN9__gnu_cxx11__enable_ifIXsr9__is_charIT_EE7__valueEbE6__typeERKSbIS2_St11char_traitsIS2_ESaIS2_EESA_ fun:_ZNKSt8equal_toISsEclERKSsS2_ fun:_ZNKSt8__detail15_Hash_code_baseISsSt4pairIKSsSt10unique_ptrIN6syncer24AttachmentDownloaderImpl13DownloadStateESt14default_deleteIS6_EEESt10_Select1stISA_ESt8equal_toISsESt4hashISsENS_18_Mod_range_hashingENS_20_Default_ranged_hashELb0EE10_M_compareERS2_mPNS_10_Hash_nodeISA_Lb0EEE fun:_ZNSt10_HashtableISsSt4pairIKSsSt10unique_ptrIN6syncer24AttachmentDownloaderImpl13DownloadStateESt14default_deleteIS5_EEESaIS9_ESt10_Select1stIS9_ESt8equal_toISsESt4hashISsENSt8__detail18_Mod_range_hashingENSH_20_Default_ranged_hashENSH_20_Prime_rehash_policyELb0ELb0ELb1EE5eraseERS1_ fun:_ZN6syncer24AttachmentDownloaderImpl17OnGetTokenFailureEPKN18OAuth2TokenService7RequestERK22GoogleServiceAuthError fun:_ZThn8_N6syncer24AttachmentDownloaderImpl17OnGetTokenFailureEPKN18OAuth2TokenService7RequestERK22GoogleServiceAuthError fun:_ZN12_GLOBAL__N_111RequestCore28InformOwnerOnGetTokenFailureE22GoogleServiceAuthError }
,
Apr 13 2016
This was caused by switching from base::ScopedPtrHashMap to std::unordered_map in http://crrev.com/1879913002. This line erases using a key string that is owned by the value being erased: https://code.google.com/p/chromium/codesearch#chromium/src/sync/internal_api/attachments/attachment_downloader_impl.cc&l=142 Previously ScopedPtrHashMap did the erase by issuing a find and then an erase using the iterator. unordered_map, likely due to some shared code with std::multimap, does the erase inside a while loop that attempts to use the key again in its condition after the first delete is performed. Should be a simple fix of doing the find and erase separately in the attachments code.
,
Apr 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b692133db89e42a7f43aa6a916e214223986b814 commit b692133db89e42a7f43aa6a916e214223986b814 Author: maxbogue <maxbogue@chromium.org> Date: Thu Apr 14 18:32:53 2016 [Sync] Fix a subtle use-after-free in Attachments. See the bug for further explanation. BUG= 603275 Review URL: https://codereview.chromium.org/1885993003 Cr-Commit-Position: refs/heads/master@{#387373} [modify] https://crrev.com/b692133db89e42a7f43aa6a916e214223986b814/sync/internal_api/attachments/attachment_downloader_impl.cc [modify] https://crrev.com/b692133db89e42a7f43aa6a916e214223986b814/tools/valgrind/memcheck/suppressions.txt
,
Apr 14 2016
,
Oct 6 2016
,
Oct 6 2016
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Apr 13 2016