Fix Use After Free from SignalReceiver

The SignalReceiver can be freed while callin the registered callback
functions when the pkgmgr_client_free is called in the callback function

Change-Id: I873508ddd5baa1ad2a27399d3b19c507312171c4
Signed-off-by: Ilho Kim <ilho159.kim@samsung.com>

diff --git a/client/src/connector.cc b/client/src/connector.cc
index 1382998..eb7be46 100644 (file)
--- a/client/src/connector.cc
+++ b/client/src/connector.cc
@@ -52,7 +52,10 @@ static bool __is_system_type(pkgmgr_client_type type)
 namespace pkgmgr {
 namespace client {
 
-Connector::~Connector() = default;
+Connector::~Connector() {
+  if (signal_receiver_)
+    signal_receiver_->StopReceiving();
+}
 
 std::string Connector::GenerateRequestId() const {
   struct timeval tv;
@@ -190,10 +193,10 @@ bool Connector::ConnectForDelayedResult() {
   return true;
 }
 
-const std::unique_ptr<SignalReceiver>& Connector::GetSignalReceiver() {
+const std::shared_ptr<SignalReceiver>& Connector::GetSignalReceiver() {
   if (!signal_receiver_)
-    signal_receiver_.reset(
-        new SignalReceiver(__is_system_type(pc_type_), status_type_));
+    signal_receiver_ = SignalReceiver::Create(
+        __is_system_type(pc_type_), status_type_);
 
   return signal_receiver_;
 }

diff --git a/client/src/connector.hh b/client/src/connector.hh
index 3c794a8..b6b10b4 100644 (file)
--- a/client/src/connector.hh
+++ b/client/src/connector.hh
@@ -45,7 +45,7 @@ class Connector {
   virtual pkg_proxy::PkgMgr* GetInfoProxy();
   virtual pkg_proxy::PkgMgrForClearCache* GetCacheProxy();
   virtual pkg_proxy::DelayedResult* GetDelayedResultProxy();
-  virtual const std::unique_ptr<SignalReceiver>& GetSignalReceiver();
+  virtual const std::shared_ptr<SignalReceiver>& GetSignalReceiver();
   std::string GenerateRequestId() const;
   void SetTep(std::string tep_path, bool tep_move);
   void SetTepArgs();
@@ -79,7 +79,7 @@ class Connector {
   ConnectionEventListener<pkg_proxy::PkgMgr> conn_info_listener_;
   ConnectionEventListener<pkg_proxy::PkgMgrForClearCache> conn_cache_listener_;
   ConnectionEventListener<pkg_proxy::DelayedResult> conn_delayed_result_listener_;
-  std::unique_ptr<SignalReceiver> signal_receiver_;
+  std::shared_ptr<SignalReceiver> signal_receiver_;
   std::vector<std::string> res_remove_path_;
   std::vector<std::string> res_create_dir_;
   std::vector<pp::ResPath> res_copy_path_;

diff --git a/client/src/signal_receiver.cc b/client/src/signal_receiver.cc
index bf020bb..39dfd2b 100644 (file)
--- a/client/src/signal_receiver.cc
+++ b/client/src/signal_receiver.cc
@@ -26,9 +26,19 @@ namespace client {
 
 #define AGENT_APPID "signal_agent"
 
+std::shared_ptr<SignalReceiver> SignalReceiver::Create(
+    bool is_system, int status_type) {
+  return std::shared_ptr<SignalReceiver>(
+      new SignalReceiver(is_system, status_type));
+}
+
+void SignalReceiver::StopReceiving() {
+  stop_flag_ = true;
+}
+
 SignalReceiver::SignalReceiver(bool is_system, int status_type)
     : PkgSignal(is_system ? "" : AGENT_APPID, is_system),
-      status_type_(status_type) {
+      status_type_(status_type), stop_flag_(false) {
 }
 
 SignalReceiver::~SignalReceiver() {
@@ -77,6 +87,8 @@ void SignalReceiver::OnAsyncResult(std::string signal, int targetUid,
     }
   }
 
+  auto handle_lock = shared_from_this();
+
   HandleHandler(targetUid, reqId, pkgs, key, val);
   HandleGlobalHandler(targetUid, reqId, pkgs, key, val);
   HandleSizeInfoHandler(reqId, pkgs, key, val);
@@ -91,6 +103,9 @@ void SignalReceiver::HandleHandler(int targetUid,
   const auto& [id, cb, app_cb, data] = it->second;
 
   for (auto& i : pkgs) {
+    if (stop_flag_)
+      return;
+
     if (cb) {
       cb(targetUid, id, i.GetPkgType().c_str(), i.GetPkgid().c_str(),
           key.c_str(), val.c_str(), nullptr, data);
@@ -106,6 +121,9 @@ void SignalReceiver::HandleGlobalHandler(int targetUid,
     const std::string& key, const std::string& val) const {
   for (auto& i : pkgs) {
     for (const auto& [id, cb, app_cb, data] : global_handlers_) {
+      if (stop_flag_)
+        return;
+
       if (cb) {
         cb(targetUid, id, i.GetPkgType().c_str(), i.GetPkgid().c_str(),
             key.c_str(), val.c_str(), nullptr, data);
@@ -149,6 +167,9 @@ void SignalReceiver::HandleSizeInfoHandler(
   };
 
   for (auto& i : pkgs) {
+    if (stop_flag_)
+      return;
+
     if (i.GetPkgid() == std::string(PKG_SIZE_INFO_TOTAL)) {
       pkgmgr_total_pkg_size_info_receive_cb callback;
       callback = (pkgmgr_total_pkg_size_info_receive_cb)cb;
@@ -174,13 +195,20 @@ void SignalReceiver::OnAsyncResultForResource(std::string signal,
     }
   }
 
+  auto handle_lock = shared_from_this();
+
   HandleResHandler(signal, targetUid, reqId, pkgid, status, extra);
   HandleGlobalResHandler(signal, targetUid, reqId, pkgid, status, extra);
 }
 
 void SignalReceiver::OnAsyncResultForPkgUpgrade(
     std::string signal, int progress) {
+  auto handle_lock = shared_from_this();
+
   for (const auto& [id, cb, data] : global_pkg_upgrade_handlers_) {
+    if (stop_flag_)
+      return;
+
     if (cb)
       cb(progress, data);
   }
@@ -203,6 +231,9 @@ void SignalReceiver::HandleGlobalResHandler(const std::string& signal,
     int targetUid, const std::string& reqId, const std::string& pkgid,
     const std::string& status, pkg_signal::ExtraData& extra) const {
   for (const auto& [id, cb, data] : global_res_handlers_) {
+    if (stop_flag_)
+      return;
+
     if (cb)
       cb(targetUid, id, pkgid.c_str(), signal.c_str(), status.c_str(),
           static_cast<void*>(&extra), data);

diff --git a/client/src/signal_receiver.hh b/client/src/signal_receiver.hh
index 18a4572..f0b386a 100644 (file)
--- a/client/src/signal_receiver.hh
+++ b/client/src/signal_receiver.hh
@@ -34,9 +34,12 @@ namespace client {
 namespace pkg_group = rpc_port::PkgSignal::group;
 namespace pkg_signal = rpc_port::PkgSignal;
 
-class SignalReceiver : public pkg_group::PkgSignal {
+class SignalReceiver : public pkg_group::PkgSignal,
+    public std::enable_shared_from_this<SignalReceiver> {
  public:
-  SignalReceiver(bool is_system, int status_type);
+  static std::shared_ptr<SignalReceiver> Create(
+      bool is_system, int status_type);
+  void StopReceiving();
   virtual ~SignalReceiver();
 
   void OnAsyncResult(std::string signal, int targetUid, std::string reqId,
@@ -60,6 +63,8 @@ class SignalReceiver : public pkg_group::PkgSignal {
   void SetStatusType(int status_type);
 
  private:
+  SignalReceiver(bool is_system, int status_type);
+
   static int GetRequestId();
   int AddEventHandler(std::string req_key, pkgmgr_handler event_cb,
       pkgmgr_app_handler app_event_cb, void* data);
@@ -82,6 +87,7 @@ class SignalReceiver : public pkg_group::PkgSignal {
  private:
   static inline int request_id_;
   int status_type_;
+  bool stop_flag_;
   std::map<std::string, std::tuple<int, pkgmgr_handler, pkgmgr_app_handler,
       void*>> handlers_;
   std::list<std::tuple<int, pkgmgr_handler, pkgmgr_app_handler, void*>> global_handlers_;