Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f11ec8f5 authored by Zach Johnson's avatar Zach Johnson
Browse files

Do not delete executing_reactable_finished_ from multiple contexts 

It was created protected by the running reactable's mutex,
but accessed during WaitForUnregisteredReactable using
the reactor's mutex.

This meant we could get a double free where it was overwritten
(and deleted) from Unregister and also set to nullptr
(and deleted) from WaitForUnregisteredReactable.

Change it to a shared_ptr, so it's not deleted until it goes out
of scope for both.

And save a copy locally in WaitForUnregisteredReactable so access
is not racy either.

Found during fuzzing.

Test: new fuzzer I'm working on
Change-Id: Id3d3159e41052ecd80bd644b2214b6692d1aebcc
parent b26e0840

system/gd/os/linux_generic/reactor.cc

+4 −3
Original line number Diff line number Diff line
@@ -105,6 +105,7 @@ void Reactor::Run() { 
      }

      auto* reactable = static_cast<Reactor::Reactable*>(event.data.ptr);

      std::unique_lock<std::mutex> lock(mutex_);

      executing_reactable_finished_ = nullptr;

      // See if this reactable has been removed in the meantime.

      if (std::find(invalidation_list_.begin(), invalidation_list_.end(), reactable) != invalidation_list_.end()) {

        continue;
@@ -194,11 +195,11 @@ void Reactor::Unregister(Reactor::Reactable* reactable) { 


bool Reactor::WaitForUnregisteredReactable(std::chrono::milliseconds timeout) {

  std::lock_guard<std::mutex> lock(mutex_);

  if (executing_reactable_finished_ == nullptr) {

  std::shared_ptr<std::future<void>> reactable_finished_future = executing_reactable_finished_;

  if (reactable_finished_future == nullptr) {

    return true;

  }

  auto stop_status = executing_reactable_finished_->wait_for(timeout);

  executing_reactable_finished_ = nullptr;

  auto stop_status = reactable_finished_future->wait_for(timeout);

  if (stop_status != std::future_status::ready) {

    LOG_ERROR("Unregister reactable timed out");

  }

system/gd/os/reactor.h

+1 −1
Original line number Diff line number Diff line
@@ -78,7 +78,7 @@ class Reactor { 
  int control_fd_;

  std::atomic<bool> is_running_;

  std::list<Reactable*> invalidation_list_;

  std::unique_ptr<std::future<void>> executing_reactable_finished_;

  std::shared_ptr<std::future<void>> executing_reactable_finished_;

};



}  // namespace os