Fix invalid read

If a stub handle is defined to a global variable, a process accesses to
the released DBusConnectionManager instance while calling the __run_exit_handlers().
The GDBusConnection instance is moved to the FdBroker object.

Change-Id: I6040a9737f5b23c644f91eccb330b6aba4104ac0
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>

diff --git a/src/fdbroker-internal.cc b/src/fdbroker-internal.cc
index 6cc085d..285b335 100644 (file)
--- a/src/fdbroker-internal.cc
+++ b/src/fdbroker-internal.cc
@@ -41,57 +41,19 @@
 namespace rpc_port {
 namespace internal {
 
-FdBroker::DBusConnectionManager& FdBroker::DBusConnectionManager::GetInst() {
-  static DBusConnectionManager dm;
-
-  if (dm.disposed_) {
-    dm.Init();
-  }
-
-  return dm;
-}
-
-// LCOV_EXCL_START
-void FdBroker::DBusConnectionManager::Dispose() {
-  if (!disposed_)
-    Fini();
-}
-// LCOV_EXCL_STOP
-
-GDBusConnection* FdBroker::DBusConnectionManager::GetConnection() {
-  return gdbus_conn_;
-}
-
-FdBroker::DBusConnectionManager::~DBusConnectionManager() {
-  if (!disposed_)
-    Fini();
-}
-
-void FdBroker::DBusConnectionManager::Init() {
-  GError* error = nullptr;
-
-  gdbus_conn_ = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error);
+GDBusConnection* FdBroker::GetConnection() {
   if (gdbus_conn_ == nullptr) {
-    // LCOV_EXCL_START
-    if (error != nullptr) {
-      _E("Failed to get dbus [%s]", error->message);
-      g_error_free(error);
+    GError* error = nullptr;
+    gdbus_conn_ = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error);
+    if (gdbus_conn_ == nullptr) {
+      _E("g_bus_get_sync() is failed. error(%s)",
+          error ? error->message : "Unknown");
+      g_clear_error(&error);
+      return nullptr;
     }
-
-    return;
-    // LCOV_EXCL_STOP
   }
 
-  disposed_ = false;
-}
-
-void FdBroker::DBusConnectionManager::Fini() {
-  if (gdbus_conn_ != nullptr) {
-    g_object_unref(gdbus_conn_);
-    gdbus_conn_ = nullptr;
-  }
-
-  disposed_ = true;
+  return gdbus_conn_;
 }
 
 // LCOV_EXCL_START
@@ -212,6 +174,9 @@ FdBroker::FdBroker(bool mock) : mock_(mock) {
 
 FdBroker::~FdBroker() {
   Cancel();
+
+  if (gdbus_conn_)
+    g_object_unref(gdbus_conn_);
 }
 
 void FdBroker::Cancel() {
@@ -238,9 +203,7 @@ void FdBroker::Cancel() {
   }
 
   if (registration_id_ > 0) {
-    g_dbus_connection_unregister_object(
-        DBusConnectionManager::GetInst().GetConnection(),
-        registration_id_);
+    g_dbus_connection_unregister_object(GetConnection(), registration_id_);
     registration_id_ = 0;
   }
 
@@ -337,8 +300,7 @@ int FdBroker::Send(const std::string& target_appid,
   }
 
   g_dbus_message_set_unix_fd_list(msg, fd_list.GetRaw());
-  g_dbus_connection_send_message_with_reply(
-      DBusConnectionManager::GetInst().GetConnection(),
+  g_dbus_connection_send_message_with_reply(GetConnection(),
       msg, G_DBUS_SEND_MESSAGE_FLAGS_NONE,
       5000, NULL, cancellable_, OnResultReceived, res_data_);
   g_object_unref(msg);
@@ -420,8 +382,7 @@ int FdBroker::SendSync(const std::string& target_appid,
 
     g_dbus_message_set_unix_fd_list(msg, fd_list.GetRaw());
 
-    reply = g_dbus_connection_send_message_with_reply_sync(
-        DBusConnectionManager::GetInst().GetConnection(),
+    reply = g_dbus_connection_send_message_with_reply_sync(GetConnection(),
         msg, G_DBUS_SEND_MESSAGE_FLAGS_NONE, 500, nullptr, nullptr, &err);
     clock_gettime(CLOCK_MONOTONIC, &end_time);
     g_object_unref(msg);
@@ -553,7 +514,7 @@ int FdBroker::GetOwnerId(const std::string& interface_name) {
   GError* error = NULL;
 
   GVariant* result = g_dbus_connection_call_sync(
-      DBusConnectionManager::GetInst().GetConnection(),
+      GetConnection(),
       DBUS_SERVICE_DBUS,
       DBUS_PATH_DBUS,
       DBUS_INTERFACE_DBUS,
@@ -672,7 +633,7 @@ int FdBroker::RegisterDbusInterface(const std::string& port_name) {
 
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   registration_id_ = g_dbus_connection_register_object(
-      DBusConnectionManager::GetInst().GetConnection(),
+      GetConnection(),
       RPC_PORT_OBJECT_PATH, introspection_data->interfaces[0],
       &interface_vtable, broker, DestroyWeakPtr, nullptr);
 
@@ -831,7 +792,7 @@ int FdBroker::Watch(IEventWatcher* ev, const std::string& target_appid,
   }
 
   watcher_id_ = g_bus_watch_name_on_connection(
-      DBusConnectionManager::GetInst().GetConnection(),
+      GetConnection(),
       interface_name.c_str(),
       G_BUS_NAME_WATCHER_FLAGS_NONE,
       OnNameAppeared,

diff --git a/src/fdbroker-internal.hh b/src/fdbroker-internal.hh
index bc02d81..01dc6d0 100644 (file)
--- a/src/fdbroker-internal.hh
+++ b/src/fdbroker-internal.hh
@@ -54,10 +54,6 @@ class FdBroker : public std::enable_shared_from_this<FdBroker> {
   explicit FdBroker(bool mock = false);
   ~FdBroker();
 
-  static void Dispose() {
-    DBusConnectionManager::GetInst().Dispose();
-  }
-
   void Cancel();
   int Send(const std::string& target_appid, const std::string& port_name,
            int (*fds)[2]);
@@ -70,27 +66,6 @@ class FdBroker : public std::enable_shared_from_this<FdBroker> {
                int (*fds)[2]);
 
  private:
-  class DBusConnectionManager {
-   public:
-    DBusConnectionManager(const DBusConnectionManager&) = delete;
-    DBusConnectionManager& operator = (const DBusConnectionManager&) = delete;
-
-    static DBusConnectionManager& GetInst();
-    void Dispose();
-    GDBusConnection* GetConnection();
-
-   private:
-    DBusConnectionManager() = default;
-    ~DBusConnectionManager();
-
-    void Init();
-    void Fini();
-
-   private:
-    bool disposed_ = true;
-    GDBusConnection* gdbus_conn_ = nullptr;
-  };
-
   class DBusMock {
    public:
     DBusMock(const DBusMock&) = delete;
@@ -177,6 +152,7 @@ class FdBroker : public std::enable_shared_from_this<FdBroker> {
   void SetConnTimer();
   void UnsetConnTimer();
   std::recursive_mutex& GetMutex() const;
+  GDBusConnection* GetConnection();
 
  private:
   IEventListener* listener_ = nullptr;
@@ -191,6 +167,7 @@ class FdBroker : public std::enable_shared_from_this<FdBroker> {
   gpointer res_data_ = nullptr;
   gpointer conn_data_ = nullptr;
   mutable std::recursive_mutex mutex_;
+  GDBusConnection* gdbus_conn_ = nullptr;
 };
 
 }  // namespace internal