From 10b4fd96e736b3cc7757486df4c4561c979457a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20H=C3=A4rer?= Date: Mon, 1 Apr 2024 19:24:57 +0200 Subject: [PATCH] [PR 24946] [GUIBaseContainer] Fix heap-use-after-free See comment in code for information. ==30885==ERROR: AddressSanitizer: heap-use-after-free on address 0x51800050bbe8 at pc 0x56aa085d20db bp 0x7ffd92777f50 sp 0x7ffd92777f48 READ of size 1 at 0x51800050bbe8 thread T0 #0 0x56aa085d20da in CGUIAction::ExecuteActions(int, int, std::shared_ptr const&) const xbmc/guilib/GUIAction.cpp:86:9 #1 0x56aa084b7701 in CStaticListProvider::OnClick(std::shared_ptr const&) xbmc/guilib/listproviders/StaticProvider.cpp:136:40 #2 0x56aa0862e065 in CGUIBaseContainer::OnClick(int) xbmc/guilib/GUIBaseContainer.cpp:881:27 #3 0x56aa0862b09c in CGUIBaseContainer::OnAction(CAction const&) xbmc/guilib/GUIBaseContainer.cpp:474:28 #4 0x56aa08c4bdf5 in CGUIWrappingListContainer::OnAction(CAction const&) xbmc/guilib/GUIWrappingListContainer.cpp:75:29 #5 0x56aa08b8f441 in CGUIWindow::OnAction(CAction const&) xbmc/guilib/GUIWindow.cpp:429:27 #6 0x56aa08bee00c in CGUIWindowManager::HandleAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1199:20 #7 0x56aa08bec973 in CGUIWindowManager::OnAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1144:11 #8 0x56aa0912be04 in CApplication::OnAction(CAction const&) xbmc/application/Application.cpp:913:54 #9 0x56aa0c914de1 in CInputManager::ExecuteInputAction(CAction const&) xbmc/input/InputManager.cpp:746:29 #10 0x56aa0c921842 in CInputManager::HandleKey(CKey const&) xbmc/input/InputManager.cpp:680:10 #11 0x56aa0c91c2ec in CInputManager::OnKeyUp(CKey const&) xbmc/input/InputManager.cpp:693:5 #12 0x56aa0c917737 in CInputManager::OnEvent(XBMC_Event&) xbmc/input/InputManager.cpp:361:7 #13 0x56aa090fe458 in CAppInboundProtocol::HandleEvents() xbmc/application/AppInboundProtocol.cpp:113:43 #14 0x56aa0915b240 in CApplication::FrameMove(bool, bool) xbmc/application/Application.cpp:1756:17 #15 0x56aa0915f200 in CApplication::Run() xbmc/application/Application.cpp:1860:7 #16 0x56aa0829c3e3 in XBMC_Run xbmc/platform/xbmc.cpp:61:26 #17 0x56aa04ecbfcf in main xbmc/platform/posix/main.cpp:70:16 #18 0x7517fb043ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) #19 0x7517fb043d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) #20 0x56aa04d91c54 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa317c54) (BuildId: 7f84180dd757174de6de03b115843129667234d3) 0x51800050bbe8 is located 872 bytes inside of 880-byte region [0x51800050b880,0x51800050bbf0) freed by thread T0 here: #0 0x56aa04ec996a in operator delete(void*) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa44f96a) (BuildId: 7f84180dd757174de6de03b115843129667234d3) #1 0x56aa08ae24d1 in CGUIStaticItem::~CGUIStaticItem() xbmc/guilib/GUIStaticItem.h:55:38 #2 0x56aa05922763 in std::_Sp_counted_ptr::_M_dispose() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/shared_ptr_base.h:428:9 #3 0x56aa04ecd0bc in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/shared_ptr_base.h:346:8 #4 0x56aa04eccca9 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/shared_ptr_base.h:1071:11 #5 0x56aa050d1c6c in std::__shared_ptr::~__shared_ptr() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/shared_ptr_base.h:1524:31 #6 0x56aa050c6ee8 in std::shared_ptr::~shared_ptr() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/shared_ptr.h:175:11 #7 0x56aa08465110 in void std::_Destroy>(std::shared_ptr*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/stl_construct.h:151:19 #8 0x56aa0846505e in void std::_Destroy_aux::__destroy*>(std::shared_ptr*, std::shared_ptr*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/stl_construct.h:163:6 #9 0x56aa08465024 in void std::_Destroy*>(std::shared_ptr*, std::shared_ptr*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/stl_construct.h:195:7 #10 0x56aa084a624b in void std::_Destroy*, std::shared_ptr>(std::shared_ptr*, std::shared_ptr*, std::allocator>&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/alloc_traits.h:947:7 #11 0x56aa084a624b in std::vector, std::allocator>>::~vector() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/stl_vector.h:732:2 #12 0x56aa086169e5 in CGUIBaseContainer::~CGUIBaseContainer() xbmc/guilib/GUIBaseContainer.cpp:117:1 #13 0x56aa08c4a148 in CGUIWrappingListContainer::~CGUIWrappingListContainer() xbmc/guilib/GUIWrappingListContainer.cpp:26:59 #14 0x56aa08c4a198 in CGUIWrappingListContainer::~CGUIWrappingListContainer() xbmc/guilib/GUIWrappingListContainer.cpp:26:59 #15 0x56aa08758935 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5 #16 0x56aa08743bb9 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:56:3 #17 0x56aa08743c48 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:55:1 #18 0x56aa08758935 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5 #19 0x56aa08743bb9 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:56:3 #20 0x56aa08743c48 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:55:1 #21 0x56aa08758935 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5 #22 0x56aa08743bb9 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:56:3 #23 0x56aa08743c48 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:55:1 #24 0x56aa08758935 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5 #25 0x56aa08743bb9 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:56:3 #26 0x56aa08743c48 in CGUIControlGroup::~CGUIControlGroup() xbmc/guilib/GUIControlGroup.cpp:55:1 #27 0x56aa08758935 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5 #28 0x56aa08b9f39d in CGUIWindow::ClearAll() xbmc/guilib/GUIWindow.cpp:816:21 #29 0x56aa08b9ed97 in CGUIWindow::FreeResources(bool) xbmc/guilib/GUIWindow.cpp:799:53 #30 0x56aa08bf8e34 in CGUIWindowManager::DeInitialize() xbmc/guilib/GUIWindowManager.cpp:1452:14 #31 0x56aa09264d22 in CApplicationSkinHandling::UnloadSkin() xbmc/application/ApplicationSkinHandling.cpp:235:29 #32 0x56aa0925e0fd in CApplicationSkinHandling::LoadSkin(std::__cxx11::basic_string, std::allocator> const&) xbmc/application/ApplicationSkinHandling.cpp:111:3 #33 0x56aa0926a8e6 in CApplicationSkinHandling::ReloadSkin(bool) xbmc/application/ApplicationSkinHandling.cpp:390:7 #34 0x56aa0c635399 in ReloadSkin(std::vector, std::allocator>, std::allocator, std::allocator>>> const&) xbmc/interfaces/builtins/SkinBuiltins.cpp:46:12 #35 0x56aa0c5a39e5 in CBuiltins::Execute(std::__cxx11::basic_string, std::allocator> const&) xbmc/interfaces/builtins/Builtins.cpp:158:14 #36 0x56aa0918597f in CApplication::ExecuteXBMCAction(std::__cxx11::basic_string, std::allocator>, std::shared_ptr const&) xbmc/application/Application.cpp:3037:32 #37 0x56aa09181b96 in CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp:3013:14 #38 0x56aa09186a20 in non-virtual thunk to CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp #39 0x56aa08bd33e1 in CGUIWindowManager::SendMessage(CGUIMessage&) xbmc/guilib/GUIWindowManager.cpp:510:23 #40 0x56aa085d2502 in CGUIAction::ExecuteActions(int, int, std::shared_ptr const&) const xbmc/guilib/GUIAction.cpp:89:52 #41 0x56aa084b7701 in CStaticListProvider::OnClick(std::shared_ptr const&) xbmc/guilib/listproviders/StaticProvider.cpp:136:40 #42 0x56aa0862e065 in CGUIBaseContainer::OnClick(int) xbmc/guilib/GUIBaseContainer.cpp:881:27 #43 0x56aa0862b09c in CGUIBaseContainer::OnAction(CAction const&) xbmc/guilib/GUIBaseContainer.cpp:474:28 #44 0x56aa08c4bdf5 in CGUIWrappingListContainer::OnAction(CAction const&) xbmc/guilib/GUIWrappingListContainer.cpp:75:29 #45 0x56aa08b8f441 in CGUIWindow::OnAction(CAction const&) xbmc/guilib/GUIWindow.cpp:429:27 #46 0x56aa08bee00c in CGUIWindowManager::HandleAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1199:20 #47 0x56aa08bec973 in CGUIWindowManager::OnAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1144:11 #48 0x56aa0912be04 in CApplication::OnAction(CAction const&) xbmc/application/Application.cpp:913:54 #49 0x56aa0c914de1 in CInputManager::ExecuteInputAction(CAction const&) xbmc/input/InputManager.cpp:746:29 #50 0x56aa0c921842 in CInputManager::HandleKey(CKey const&) xbmc/input/InputManager.cpp:680:10 #51 0x56aa0c91c2ec in CInputManager::OnKeyUp(CKey const&) xbmc/input/InputManager.cpp:693:5 #52 0x56aa0c917737 in CInputManager::OnEvent(XBMC_Event&) xbmc/input/InputManager.cpp:361:7 #53 0x56aa090fe458 in CAppInboundProtocol::HandleEvents() xbmc/application/AppInboundProtocol.cpp:113:43 #54 0x56aa0915b240 in CApplication::FrameMove(bool, bool) xbmc/application/Application.cpp:1756:17 #55 0x56aa0915f200 in CApplication::Run() xbmc/application/Application.cpp:1860:7 #56 0x56aa0829c3e3 in XBMC_Run xbmc/platform/xbmc.cpp:61:26 #57 0x56aa04ecbfcf in main xbmc/platform/posix/main.cpp:70:16 #58 0x7517fb043ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) previously allocated by thread T0 here: #0 0x56aa04ec8ed2 in operator new(unsigned long) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa44eed2) (BuildId: 7f84180dd757174de6de03b115843129667234d3) #1 0x56aa084b3183 in CStaticListProvider::CStaticListProvider(TiXmlElement const*, int) xbmc/guilib/listproviders/StaticProvider.cpp:28:33 #2 0x56aa0849c590 in std::__detail::_MakeUniq::__single_object std::make_unique(TiXmlElement const*&&, int&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/13.2.1/../../../../include/c++/13.2.1/bits/unique_ptr.h:1070:34 #3 0x56aa0849bac7 in IListProvider::CreateSingle(TiXmlNode const*, int) xbmc/guilib/listproviders/IListProvider.cpp:34:12 #4 0x56aa0849b582 in IListProvider::Create(TiXmlNode const*, int) xbmc/guilib/listproviders/IListProvider.cpp:25:12 #5 0x56aa0864bbe8 in CGUIBaseContainer::LoadListProvider(TiXmlElement*, int, bool) xbmc/guilib/GUIBaseContainer.cpp:1282:20 #6 0x56aa0871b1c3 in CGUIControlFactory::Create(int, CRectGen const&, TiXmlElement*, bool) xbmc/guilib/GUIControlFactory.cpp:1543:17 #7 0x56aa08b884c4 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen const&) xbmc/guilib/GUIWindow.cpp:281:38 #8 0x56aa08b8a088 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen const&) xbmc/guilib/GUIWindow.cpp:309:9 #9 0x56aa08b8a088 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen const&) xbmc/guilib/GUIWindow.cpp:309:9 #10 0x56aa08b8a088 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen const&) xbmc/guilib/GUIWindow.cpp:309:9 #11 0x56aa08b8a088 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen const&) xbmc/guilib/GUIWindow.cpp:309:9 #12 0x56aa08b87cf6 in CGUIWindow::Load(TiXmlElement*) xbmc/guilib/GUIWindow.cpp:264:11 #13 0x56aa08b80657 in CGUIWindow::LoadXML(std::__cxx11::basic_string, std::allocator> const&, std::__cxx11::basic_string, std::allocator> const&) xbmc/guilib/GUIWindow.cpp:155:10 #14 0x56aa08b7e6c0 in CGUIWindow::Load(std::__cxx11::basic_string, std::allocator> const&, bool) xbmc/guilib/GUIWindow.cpp:109:14 #15 0x56aa08b9dc45 in CGUIWindow::AllocResources(bool) xbmc/guilib/GUIWindow.cpp:765:7 #16 0x56aa08b95df7 in CGUIWindow::OnMessage(CGUIMessage&) xbmc/guilib/GUIWindow.cpp:594:52 #17 0x56aa08be2521 in CGUIWindowManager::ActivateWindow_Internal(int, std::vector, std::allocator>, std::allocator, std::allocator>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:896:15 #18 0x56aa08bddfbc in CGUIWindowManager::ActivateWindow(int, std::vector, std::allocator>, std::allocator, std::allocator>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:802:5 #19 0x56aa0c5b75f3 in int (anonymous namespace)::ActivateWindow(std::vector, std::allocator>, std::allocator, std::allocator>>> const&) xbmc/interfaces/builtins/GUIBuiltins.cpp:109:52 #20 0x56aa0c5a39e5 in CBuiltins::Execute(std::__cxx11::basic_string, std::allocator> const&) xbmc/interfaces/builtins/Builtins.cpp:158:14 #21 0x56aa0918597f in CApplication::ExecuteXBMCAction(std::__cxx11::basic_string, std::allocator>, std::shared_ptr const&) xbmc/application/Application.cpp:3037:32 #22 0x56aa09181b96 in CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp:3013:14 #23 0x56aa09186a20 in non-virtual thunk to CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp #24 0x56aa08bd33e1 in CGUIWindowManager::SendMessage(CGUIMessage&) xbmc/guilib/GUIWindowManager.cpp:510:23 #25 0x56aa085d2502 in CGUIAction::ExecuteActions(int, int, std::shared_ptr const&) const xbmc/guilib/GUIAction.cpp:89:52 #26 0x56aa0867f896 in CGUIButtonControl::OnClick() xbmc/guilib/GUIButtonControl.cpp:393:16 #27 0x56aa08677e86 in CGUIButtonControl::OnAction(CAction const&) xbmc/guilib/GUIButtonControl.cpp:212:5 #28 0x56aa08b8f441 in CGUIWindow::OnAction(CAction const&) xbmc/guilib/GUIWindow.cpp:429:27 #29 0x56aa08bee00c in CGUIWindowManager::HandleAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1199:20 #30 0x56aa08bec973 in CGUIWindowManager::OnAction(CAction const&) const xbmc/guilib/GUIWindowManager.cpp:1144:11 #31 0x56aa0912be04 in CApplication::OnAction(CAction const&) xbmc/application/Application.cpp:913:54 #32 0x56aa0c914de1 in CInputManager::ExecuteInputAction(CAction const&) xbmc/input/InputManager.cpp:746:29 #33 0x56aa0c921842 in CInputManager::HandleKey(CKey const&) xbmc/input/InputManager.cpp:680:10 #34 0x56aa0c91c2ec in CInputManager::OnKeyUp(CKey const&) xbmc/input/InputManager.cpp:693:5 #35 0x56aa0c917737 in CInputManager::OnEvent(XBMC_Event&) xbmc/input/InputManager.cpp:361:7 #36 0x56aa090fe458 in CAppInboundProtocol::HandleEvents() xbmc/application/AppInboundProtocol.cpp:113:43 #37 0x56aa0915b240 in CApplication::FrameMove(bool, bool) xbmc/application/Application.cpp:1756:17 #38 0x56aa0915f200 in CApplication::Run() xbmc/application/Application.cpp:1860:7 #39 0x56aa0829c3e3 in XBMC_Run xbmc/platform/xbmc.cpp:61:26 #40 0x56aa04ecbfcf in main xbmc/platform/posix/main.cpp:70:16 #41 0x7517fb043ccf (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af) SUMMARY: AddressSanitizer: heap-use-after-free xbmc/guilib/GUIAction.cpp:86:9 in CGUIAction::ExecuteActions(int, int, std::shared_ptr const&) const Shadow bytes around the buggy address: 0x51800050b900: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x51800050b980: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x51800050ba00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x51800050ba80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x51800050bb00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x51800050bb80: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fa fa 0x51800050bc00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x51800050bc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x51800050bd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x51800050bd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x51800050be00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 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 ==30885==ABORTING (cherry picked from commit 9e4cfd2d174bd939191d64e1788835a00ffcb28f) --- xbmc/guilib/GUIBaseContainer.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/xbmc/guilib/GUIBaseContainer.cpp b/xbmc/guilib/GUIBaseContainer.cpp index c22eb71284f27..897459790dd76 100644 --- a/xbmc/guilib/GUIBaseContainer.cpp +++ b/xbmc/guilib/GUIBaseContainer.cpp @@ -13,6 +13,7 @@ #include "GUIListItemLayout.h" #include "GUIMessage.h" #include "ServiceBroker.h" +#include "guilib/GUIListItem.h" #include "guilib/guiinfo/GUIInfoLabels.h" #include "guilib/listproviders/IListProvider.h" #include "input/actions/Action.h" @@ -875,10 +876,16 @@ bool CGUIBaseContainer::OnClick(int actionID) int selected = GetSelectedItem(); if (selected >= 0 && selected < static_cast(m_items.size())) { + // One of the actions could trigger a reload of the GUI which destroys + // this CGUIBaseContainer and therefore the m_items[selected] we are + // going to process. The shared_ptr ensures that item survives until + // it has been processed. + std::shared_ptr item = m_items[selected]; + if (m_clickActions.HasActionsMeetingCondition()) - m_clickActions.ExecuteActions(0, GetParentID(), m_items[selected]); + m_clickActions.ExecuteActions(0, GetParentID(), item); else - m_listProvider->OnClick(m_items[selected]); + m_listProvider->OnClick(item); } return true; }