use-after-free in ProductionSiteWindow::update_worker_table

[Klaus Halfmann]

I was playing fellowships (No AIs involved) just to get a lot of ships,
when changing the priority of log fors a charcoal kiln the game crashed
witn the attached ASAN report:

==6450==ERROR: AddressSanitizer: heap-use-after-free on address 0x6130029690e8 at pc 0x000106764f14 bp 0x7ffeea7d6450 sp 0x7ffeea7d6448
READ of size 8 at 0x6130029690e8 thread T0
    #0 0x106764f13 in UI::Table<void*>::size() const vector:632
    #1 0x106c2a80f in ProductionSiteWindow::update_worker_table(Widelands::ProductionSite*) productionsitewindow.cc:156
    #2 0x106c2eff6 in ProductionSiteWindow::ProductionSiteWindow(InteractiveGameBase&, UI::UniqueWindow::Registry&, Widelands::ProductionSite&, bool)::$_0::operator()(Widelands::NoteBuilding const&) const productionsitewindow.cc:58
    #3 0x106c2ee6c in _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN20ProductionSiteWindowC1ER19InteractiveGameBaseRN2UI12UniqueWindow8RegistryERN9Widelands14ProductionSiteEbE3$_0RKNSA_12NoteBuildingEEEEvDpOT_ type_traits:4291
    #4 0x106c2ecf8 in std::__1::__function::__func<ProductionSiteWindow::ProductionSiteWindow(InteractiveGameBase&, UI::UniqueWindow::Registry&, Widelands::ProductionSite&, bool)::$_0, std::__1::allocator<ProductionSiteWindow::ProductionSiteWindow(InteractiveGameBase&, UI::UniqueWindow::Registry&, Widelands::ProductionSite&, bool)::$_0>, void (Widelands::NoteBuilding const&)>::operator()(Widelands::NoteBuilding const&) functional:1552
    #5 0x105e02112 in std::__1::function<void (Widelands::NoteBuilding const&)>::operator()(Widelands::NoteBuilding const&) const functional:1911
    #6 0x105e01e3e in void Notifications::NotificationsManager::publish<Widelands::NoteBuilding>(Widelands::NoteBuilding const&) notifications_impl.h:76
    #7 0x105df1f2c in void Notifications::publish<Widelands::NoteBuilding>(Widelands::NoteBuilding const&) notifications.h:51
    #8 0x105ee1389 in Widelands::ProductionSite::train_workers(Widelands::Game&) productionsite.cc:931
    #9 0x105edd9e1 in Widelands::ProductionSite::program_end(Widelands::Game&, Widelands::ProgramResult) productionsite.cc:912
    #10 0x105ed3eed in Widelands::ProductionSite::act(Widelands::Game&, unsigned int) productionsite.cc:671
    #11 0x105d53141 in Widelands::CmdAct::execute(Widelands::Game&) map_object.cc:100
    #12 0x105ac1e5b in Widelands::CmdQueue::run_queue(int, unsigned int&) cmd_queue.cc:123
    #13 0x105b16bdf in Widelands::Game::think() game.cc:552

0x6130029690e8 is located 360 bytes inside of 376-byte region [0x613002968f80,0x6130029690f8)
freed by thread T0 here:
    #0 0x109c2e74b in wrap__ZdlPv (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x6474b)
    #1 0x1065fac91 in UI::Table<InternetClient const* const>::~Table() table.h:450
    #2 0x1064f73b7 in UI::Panel::free_children() panel.cc:127

previously allocated by thread T0 here:
    #0 0x109c2e14b in wrap__Znwm (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x6414b)
    #1 0x106c27bb0 in ProductionSiteWindow::init(bool) productionsitewindow.cc:93
    #2 0x106c26839 in ProductionSiteWindow::ProductionSiteWindow(InteractiveGameBase&, UI::UniqueWindow::Registry&, Widelands::ProductionSite&, bool) productionsitewindow.cc:65

I assume this is another case of me closing a Window before the Game could think().

I will not atach a save game unless I can realiably reproduces this.

[SirVer (sirver)]

I feel I have fixed this bug recently. Here it looks like the Table has been destroyed already, I checked that the building is still in existance. It seems weird that the table can be invalid while the building is still alive.

[GunChleoc (gunchleoc)]

Should have been fixed in r8519. Which revision were you on?

[Klaus Halfmann (klaus-halfmann)]

I played bzr8524[trunk].

I tried to reproduce this quite some time but it did not happen again.
So I assume this is some "longer" story in terms of meory corruption.

[GunChleoc (gunchleoc)]

I just got this - I had 2 windows open: a Barbarian Training Camp and a construction site to enhance an Ax Factory. It happened while I did a right-click on the trainingsite, but that might have been a coincidence.

==27467==ERROR: AddressSanitizer: heap-use-after-free on address 0x6140002041c8 at pc 0x000000e3801f bp 0x7ffe5f0c1a10 sp 0x7ffe5f0c1a00
READ of size 8 at 0x6140002041c8 thread T0
    #0 0xe3801e in std::vector<UI::Table<void*>::EntryRecord*, std::allocator<UI::Table<void*>::EntryRecord*> >::size() const /usr/include/c++/5/bits/stl_vector.h:655
    #1 0x12b992f in UI::Table<void*>::size() const /home/bratzbert/sources/widelands/fh1-tutorials/src/ui_basic/table.h:211
    #2 0x13f4af1 in ProductionSiteWindow::update_worker_table(Widelands::ProductionSite*) /home/bratzbert/sources/widelands/fh1-tutorials/src/wui/productionsitewindow.cc:156
    #3 0x13f2fe3 in operator() /home/bratzbert/sources/widelands/fh1-tutorials/src/wui/productionsitewindow.cc:58
    #4 0x13f5618 in _M_invoke /usr/include/c++/5/functional:1871
    #5 0xff8f3e in std::function<void (Widelands::NoteBuilding const&)>::operator()(Widelands::NoteBuilding const&) const /usr/include/c++/5/functional:2267
    #6 0xff3aae in void Notifications::NotificationsManager::publish<Widelands::NoteBuilding>(Widelands::NoteBuilding const&) /home/bratzbert/sources/widelands/fh1-tutorials/src/notifications/notifications_impl.h:76
    #7 0xfed3ce in void Notifications::publish<Widelands::NoteBuilding>(Widelands::NoteBuilding const&) /home/bratzbert/sources/widelands/fh1-tutorials/src/notifications/notifications.h:51
    #8 0x107732d in Widelands::Building::add_worker(Widelands::Worker&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/tribes/building.cc:698
    #9 0x10f1bc7 in Widelands::TrainingSite::add_worker(Widelands::Worker&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/tribes/trainingsite.cc:344
    #10 0x1142c41 in Widelands::Worker::set_location(Widelands::PlayerImmovable*) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/tribes/worker.cc:1046
    #11 0x11443f9 in Widelands::Worker::transfer_update(Widelands::Game&, Widelands::Bob::State&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/tribes/worker.cc:1343
    #12 0x1021193 in Widelands::Bob::do_act(Widelands::Game&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/bob.cc:194
    #13 0x1020ee9 in Widelands::Bob::act(Widelands::Game&, unsigned int) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/bob.cc:180
    #14 0x1059024 in Widelands::CmdAct::execute(Widelands::Game&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/map_objects/map_object.cc:100
    #15 0x1184c41 in Widelands::CmdQueue::run_queue(int, unsigned int&) /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/cmd_queue.cc:123
    #16 0xf96227 in Widelands::Game::think() /home/bratzbert/sources/widelands/fh1-tutorials/src/logic/game.cc:552

[GunChleoc (gunchleoc)]

Jut got it again. I first reduced the number of soldiers in a training site, then closed the window.

[GunChleoc (gunchleoc)]

I think I understand what is happening here: The productionsite window is reacting to a notification, telling it to update the worker table, However, the window is already dying, so the worker table is in an undefined state.

Since a change in the worker table and the window being closed has to happen at the exact same moment, this is hard to reproduce.

The solution is to query whether the window is dying before executing the notification's request.

[GunChleoc (gunchleoc)]

Should be fixed in r8531. Please reopen the bug if this should happen again.

[GunChleoc (gunchleoc)]

Fixed in build20-rc1