[PR 24948] [GUIDialogSettingsBase] Clean-up owned edit control in Fre…

…eControls()

When using a skin that doesn't provide a CGUIEditControl, GUIDialogSettingsBase
creates its own. When switching to a skin that does provide one, it loads it
from the skin, overwrites the pointer to its own edit control (memory leak!)
but still thinks it owns the control because m_newOriginalEdit is true. In
DeleteControls() it then deletes the edit control that it doesn't own.
Cleaning up and resetting the flag in FreeControls() solves the problem.

ASAN error:

==29999==ERROR: AddressSanitizer: heap-use-after-free on address 0x51d0015bd080 at pc 0x5dcd1a23e410 bp 0x7ffe96645b50 sp 0x7ffe96645b48
READ of size 8 at 0x51d0015bd080 thread T0
    #0 0x5dcd1a23e40f in CGUIDialogSettingsBase::DeleteControls() xbmc/settings/dialogs/GUIDialogSettingsBase.cpp:476:5
    #1 0x5dcd1a22b1c0 in CGUIDialogSettingsBase::~CGUIDialogSettingsBase() xbmc/settings/dialogs/GUIDialogSettingsBase.cpp:77:3
    #2 0x5dcd1a27e8a8 in CGUIDialogSettingsManagerBase::~CGUIDialogSettingsManagerBase() xbmc/settings/dialogs/GUIDialogSettingsManagerBase.cpp:19:63
    #3 0x5dcd19fee328 in CGUIWindowSettingsCategory::~CGUIWindowSettingsCategory() xbmc/settings/windows/GUIWindowSettingsCategory.cpp:66:57
    #4 0x5dcd19fee438 in CGUIWindowSettingsCategory::~CGUIWindowSettingsCategory() xbmc/settings/windows/GUIWindowSettingsCategory.cpp:66:57
    #5 0x5dcd1899e2ea in CGUIWindowManager::DestroyWindow(int) xbmc/guilib/GUIWindowManager.cpp:489:5
    #6 0x5dcd1899d5bd in CGUIWindowManager::DestroyWindows() xbmc/guilib/GUIWindowManager.cpp:459:5
    #7 0x5dcd18f2e94e in CApplication::Cleanup() xbmc/application/Application.cpp:1917:34
    #8 0x5dcd18f2d405 in CApplication::Run() xbmc/application/Application.cpp:1876:3
    #9 0x5dcd1806a143 in XBMC_Run xbmc/platform/xbmc.cpp:61:26
    #10 0x5dcd14c97b2f in main xbmc/platform/posix/main.cpp:70:16
    #11 0x7fb259c43ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #12 0x7fb259c43d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #13 0x5dcd14b5d7b4 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa3197b4) (BuildId: e4bf2336bbd9ba3ae66ffab4d8a0bca77c50c089)

0x51d0015bd080 is located 0 bytes inside of 2096-byte region [0x51d0015bd080,0x51d0015bd8b0)
freed by thread T0 here:
    #0 0x5dcd14c954ca in operator delete(void*) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa4514ca) (BuildId: e4bf2336bbd9ba3ae66ffab4d8a0bca77c50c089)
    #1 0x5dcd18582f01 in CGUIEditControl::~CGUIEditControl() xbmc/guilib/GUIEditControl.cpp:106:39
    #2 0x5dcd18526695 in CGUIControlGroup::ClearAll() xbmc/guilib/GUIControlGroup.cpp:525:5
    #3 0x5dcd1896d04d in CGUIWindow::ClearAll() xbmc/guilib/GUIWindow.cpp:816:21
    #4 0x5dcd1896ca47 in CGUIWindow::FreeResources(bool) xbmc/guilib/GUIWindow.cpp:799:53
    #5 0x5dcd189c6ae4 in CGUIWindowManager::DeInitialize() xbmc/guilib/GUIWindowManager.cpp:1452:14
    #6 0x5dcd190329d2 in CApplicationSkinHandling::UnloadSkin() xbmc/application/ApplicationSkinHandling.cpp:235:29
    #7 0x5dcd18f2dd81 in CApplication::Cleanup() xbmc/application/Application.cpp:1895:47
    #8 0x5dcd18f2d405 in CApplication::Run() xbmc/application/Application.cpp:1876:3
    #9 0x5dcd1806a143 in XBMC_Run xbmc/platform/xbmc.cpp:61:26
    #10 0x5dcd14c97b2f in main xbmc/platform/posix/main.cpp:70:16
    #11 0x7fb259c43ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

previously allocated by thread T0 here:
    #0 0x5dcd14c94a32 in operator new(unsigned long) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa450a32) (BuildId: e4bf2336bbd9ba3ae66ffab4d8a0bca77c50c089)
    #1 0x5dcd184dd051 in CGUIControlFactory::Create(int, CRectGen<float> const&, TiXmlElement*, bool) xbmc/guilib/GUIControlFactory.cpp:1298:17
    #2 0x5dcd18956174 in CGUIWindow::LoadControl(TiXmlElement*, CGUIControlGroup*, CRectGen<float> const&) xbmc/guilib/GUIWindow.cpp:281:38
    #3 0x5dcd189559a6 in CGUIWindow::Load(TiXmlElement*) xbmc/guilib/GUIWindow.cpp:264:11
    #4 0x5dcd18578d5a in CGUIDialog::Load(TiXmlElement*) xbmc/guilib/GUIDialog.cpp:39:22
    #5 0x5dcd1894e307 in CGUIWindow::LoadXML(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/guilib/GUIWindow.cpp:155:10
    #6 0x5dcd1894c370 in CGUIWindow::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool) xbmc/guilib/GUIWindow.cpp:109:14
    #7 0x5dcd1896b8f5 in CGUIWindow::AllocResources(bool) xbmc/guilib/GUIWindow.cpp:765:7
    #8 0x5dcd18963aa7 in CGUIWindow::OnMessage(CGUIMessage&) xbmc/guilib/GUIWindow.cpp:594:52
    #9 0x5dcd1857a996 in CGUIDialog::OnMessage(CGUIMessage&) xbmc/guilib/GUIDialog.cpp:93:19
    #10 0x5dcd1a2332c2 in CGUIDialogSettingsBase::OnMessage(CGUIMessage&) xbmc/settings/dialogs/GUIDialogSettingsBase.cpp:264:22
    #11 0x5dcd19feeab3 in CGUIWindowSettingsCategory::OnMessage(CGUIMessage&) xbmc/settings/windows/GUIWindowSettingsCategory.cpp:75:38
    #12 0x5dcd189b01d1 in CGUIWindowManager::ActivateWindow_Internal(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:896:15
    #13 0x5dcd189abc6c in CGUIWindowManager::ActivateWindow(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:802:5
    #14 0x5dcd189a9ac5 in CGUIWindowManager::ActivateWindow(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/guilib/GUIWindowManager.cpp:779:3
    #15 0x5dcd19030b15 in CApplicationSkinHandling::LoadSkin(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/application/ApplicationSkinHandling.cpp:186:50
    #16 0x5dcd19038596 in CApplicationSkinHandling::ReloadSkin(bool) xbmc/application/ApplicationSkinHandling.cpp:390:7
    #17 0x5dcd1c404429 in ReloadSkin(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) xbmc/interfaces/builtins/SkinBuiltins.cpp:46:12
    #18 0x5dcd1c372a75 in CBuiltins::Execute(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/interfaces/builtins/Builtins.cpp:158:14
    #19 0x5dcd18f1bf6a in CApplication::OnApplicationMessage(KODI::MESSAGING::ThreadMessage*) xbmc/application/Application.cpp:1577:30
    #20 0x5dcd18f27390 in non-virtual thunk to CApplication::OnApplicationMessage(KODI::MESSAGING::ThreadMessage*) xbmc/application/Application.cpp
    #21 0x5dcd181b400d in KODI::MESSAGING::CApplicationMessenger::ProcessMessage(KODI::MESSAGING::ThreadMessage*) xbmc/messaging/ApplicationMessenger.cpp:244:17
    #22 0x5dcd181b6325 in KODI::MESSAGING::CApplicationMessenger::ProcessMessages() xbmc/messaging/ApplicationMessenger.cpp:217:5
    #23 0x5dcd18f5501a in CApplication::Process() xbmc/application/Application.cpp:3156:38
    #24 0x5dcd18f2cac8 in CApplication::Run() xbmc/application/Application.cpp:1855:5
    #25 0x5dcd1806a143 in XBMC_Run xbmc/platform/xbmc.cpp:61:26
    #26 0x5dcd14c97b2f in main xbmc/platform/posix/main.cpp:70:16
    #27 0x7fb259c43ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

SUMMARY: AddressSanitizer: heap-use-after-free xbmc/settings/dialogs/GUIDialogSettingsBase.cpp:476:5 in CGUIDialogSettingsBase::DeleteControls()
Shadow bytes around the buggy address:
  0x51d0015bce00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51d0015bce80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51d0015bcf00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51d0015bcf80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x51d0015bd000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x51d0015bd080:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51d0015bd100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51d0015bd180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51d0015bd200: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51d0015bd280: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x51d0015bd300: fd fd fd fd fd fd fd fd fd fd fd fd 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
==29999==ABORTING

xbmc/settings/dialogs/GUIDialogSettingsBase.cpp

@@ -466,17 +466,20 @@ void CGUIDialogSettingsBase::FreeControls()
     control->ClearAll();
   }
   m_categories.clear();
-  FreeSettingsControls();
-}
 
-void CGUIDialogSettingsBase::DeleteControls()
-{
+  // If we created our own edit control instead of borrowing it then clean it up
   if (m_newOriginalEdit)
   {
     delete m_pOriginalEdit;
-    m_pOriginalEdit = NULL;
+    m_pOriginalEdit = nullptr;
+    m_newOriginalEdit = false;
   }
 
+  FreeSettingsControls();
+}
+
+void CGUIDialogSettingsBase::DeleteControls()
+{
   m_resetSetting.reset();
   m_dummyCategory.reset();
 }