Fix UAF due to context swithing while access loop

loop_ member variable can be accessed by both worker and main.
It can cause UAF or glib reference error.
This patch make functions related to loop_ atomic.

- Apply mutex locking when access loop_ member variable
- Task::Dispose now verify loop_ is removed well (wait if need)

Change-Id: I94147fc00bb1ccb64f11fe5543ff92a50a1f67af
Signed-off-by: newb1e <jihoi.kim@samsung.com>

diff --git a/src/tizen-core/stub.cc b/src/tizen-core/stub.cc
index 9fe6c83fdef293978ea630bb17b02c06cf11e079..282776b4a9b7fcdac1985f4fcd549642c0aadeb1 100644 (file)
--- a/src/tizen-core/stub.cc
+++ b/src/tizen-core/stub.cc
@@ -188,8 +188,8 @@ API int tizen_core_task_destroy(tizen_core_task_h task) {
   }
 
   auto* handle = static_cast<tizen_core::Task*>(task);
-  if (handle->GetRefCount() == 1) handle->Dispose();
   if (handle->IsRunning()) handle->Quit();
+  if (handle->GetRefCount() == 1) handle->Dispose();
   handle->UnrefSelf();
   return TIZEN_CORE_ERROR_NONE;
 }

diff --git a/src/tizen-core/task.cc b/src/tizen-core/task.cc
index bdba380856cd11eeadc69a53c6b1cccb149067f3..060afe6809e980aecd77c26d1ae7a9eec283f5fe 100644 (file)
--- a/src/tizen-core/task.cc
+++ b/src/tizen-core/task.cc
@@ -226,6 +226,12 @@ std::shared_ptr<Task> Task::Create(std::string name, bool use_thread) {
 }
 
 void Task::Dispose() {
+  if(use_thread_) {
+    std::unique_lock<std::mutex> quit_lock(loop_mutex_);
+    cond_var_.wait(quit_lock, [this]() {
+        return (loop_ == nullptr);
+    });
+  }
   context_->SetLoop(nullptr);
   ContextManager::GetInst().Dispose(name_);
 }
@@ -235,14 +241,24 @@ std::shared_ptr<Context> Task::GetContext() const {
 }
 
 void Task::Quit() {
-  if (loop_ && g_main_loop_is_running(loop_))
-    g_main_loop_quit(loop_);
+  if(use_thread_) {
+    std::unique_lock<std::mutex> quit_lock(loop_mutex_);
+    if (loop_ && g_main_loop_is_running(loop_))
+      g_main_loop_quit(loop_);
+  } else {
+    if (loop_ && g_main_loop_is_running(loop_))
+      g_main_loop_quit(loop_);
+  }
 }
 
 void Task::Run() {
-  loop_ = g_main_loop_new(context_->GetHandle(), FALSE);
   if (use_thread_) {
     std::unique_lock<std::mutex> lock(loop_mutex_);
+    if(loop_ != nullptr) {
+      _W("Task GMainLoop already in progress");
+      return;
+    }
+    loop_ = g_main_loop_new(context_->GetHandle(), FALSE);
     idle_entered_ = false;
     thread_ = std::thread([&]() -> void {
       auto self = shared_from_this();
@@ -253,21 +269,30 @@ void Task::Run() {
     });
     cond_var_.wait(lock, [this]() { return idle_entered_; });
   } else {
+    if(loop_ != nullptr) {
+      _W("Task GMainLoop already in progress");
+      return;
+    }
+    loop_ = g_main_loop_new(context_->GetHandle(), FALSE);
     tid_ = getpid();
     g_main_loop_run(loop_);
-    g_main_loop_unref(loop_);
-    loop_ = nullptr;
 
     while (g_main_context_pending(context_->GetHandle()))
       g_main_context_dispatch(context_->GetHandle());
 
     tid_ = -1;
+    g_main_loop_unref(loop_);
+    loop_ = nullptr;
   }
 }
 
 bool Task::IsRunning() const {
-  if (loop_ == nullptr) return false;
-  return g_main_loop_is_running(loop_);
+  if(use_thread_) {
+    std::unique_lock<std::mutex> loop_lock(loop_mutex_);
+    return (loop_ && g_main_loop_is_running(loop_));
+  } else {
+    return (loop_ && g_main_loop_is_running(loop_));
+  }
 }
 
 std::shared_ptr<ISource> Task::AddIdleJob(std::function<bool()> job) {
@@ -411,13 +436,18 @@ void Task::ThreadLoop() {
 
   pthread_setname_np(pthread_self(), name_.c_str());
   g_main_loop_run(loop_);
-  g_main_loop_unref(loop_);
-  loop_ = nullptr;
 
   while (g_main_context_pending(context_->GetHandle()))
     g_main_context_dispatch(context_->GetHandle());
-
   g_main_context_pop_thread_default(context_->GetHandle());
+
+  {
+    std::unique_lock<std::mutex> loop_lock(loop_mutex_);
+    g_main_loop_unref(loop_);
+    loop_ = nullptr;
+  }
+  cond_var_.notify_one();
+
   _D("[%s] END", name_.c_str());
 }