fixed a security issue in the use of RPC mutex.

Signed-off-by: 18392170496 <magentang4@huawei.com>

ipc/native/src/mock/include/dbinder_base_invoker.h

@@ -96,6 +96,7 @@ public:
     virtual int CheckAndSetCallerInfo(uint32_t listenFd, uint64_t stubIndex) = 0;
     virtual int OnSendRawData(std::shared_ptr<T> session, const void *data, size_t size) = 0;
     bool CheckTransactionData(const dbinder_transaction_data *tr) const;
+    std::mutex &GetObjectMutex();
 
 private:
     uint32_t TranslateBinderType(flat_binder_object *binderObject, char *sessionOffset, std::shared_ptr<T> session);
@@ -125,6 +126,9 @@ private:
     std::shared_ptr<ThreadProcessInfo> MakeThreadProcessInfo(uint32_t handle, const char *buffer, uint32_t size);
     std::shared_ptr<ThreadMessageInfo> MakeThreadMessageInfo(uint32_t handle);
     uint32_t MakeRemoteHandle(std::shared_ptr<T> session);
+
+private:
+    std::mutex objectMutex_;
 };
 
 template<class T>
@@ -901,27 +905,32 @@ template <class T> void DBinderBaseInvoker<T>::ProcessTransaction(dbinder_transa
 
     const uint32_t flags = tr->flags;
     uint64_t senderSeqNumber = tr->seqNumber;
-    auto *stub = current->QueryStubByIndex(tr->cookie);
-    if (stub == nullptr) {
-        ZLOGE(LOG_LABEL, "stubIndex is invalid");
-        return;
-    }
-    if (!IRemoteObjectTranslateWhenRcv(reinterpret_cast<char *>(tr->buffer), tr->buffer_size, data,
-        listenFd, nullptr)) {
-        ZLOGE(LOG_LABEL, "translate object failed");
-        return;
+    int error = ERR_NONE;
+    {
+        std::lock_guard<std::mutex> lockGuard(objectMutex_);
+        auto *stub = current->QueryStubByIndex(tr->cookie);
+        if (stub == nullptr) {
+            ZLOGE(LOG_LABEL, "stubIndex is invalid");
+            return;
+        }
+        if (!IRemoteObjectTranslateWhenRcv(reinterpret_cast<char *>(tr->buffer), tr->buffer_size, data,
+            listenFd, nullptr)) {
+            ZLOGE(LOG_LABEL, "translate object failed");
+            return;
+        }
+
+        auto *stubObject = reinterpret_cast<IPCObjectStub *>(stub);
+        MessageOption option;
+        option.SetFlags(flags);
+        // cannot use stub any more after SendRequest because this cmd may be
+        // dbinder dec ref and thus stub will be destroyed
+        int error = stubObject->SendRequest(tr->code, data, reply, option);
+        if (error != ERR_NONE) {
+            ZLOGE(LOG_LABEL, "stub sendrequest failed, cmd: %{public}u, error: %{public}d", tr->code, error);
+            // can not return;
+        }
     }
 
-    auto *stubObject = reinterpret_cast<IPCObjectStub *>(stub);
-    MessageOption option;
-    option.SetFlags(flags);
-    // cannot use stub any more after SendRequest because this cmd may be
-    // dbinder dec ref and thus stub will be destroyed
-    int error = stubObject->SendRequest(tr->code, data, reply, option);
-    if (error != ERR_NONE) {
-        ZLOGE(LOG_LABEL, "stub sendrequest failed, cmd: %{public}u, error: %{public}d", tr->code, error);
-        // can not return;
-    }
     if (data.GetRawData() != nullptr) {
         ZLOGE(LOG_LABEL, "delete raw data in process skeleton, fd: %{public}u", listenFd);
         current->DetachRawData(listenFd);
@@ -1058,5 +1067,11 @@ template <class T> bool DBinderBaseInvoker<T>::CheckTransactionData(const dbinde
 
     return true;
 }
+
+template <class T> std::mutex &DBinderBaseInvoker<T>::GetObjectMutex()
+{
+    return objectMutex_;
+}
+
 } // namespace OHOS
 #endif // OHOS_IPC_DBINDER_BASE_INVOKER_H

ipc/native/src/mock/source/dbinder_databus_invoker.cpp

@@ -547,6 +547,7 @@ bool DBinderDatabusInvoker::OnDatabusSessionServerSideClosed(std::shared_ptr<Ses
     // detach info whose listen fd equals the given one
     std::list<uint64_t> stubIndexs = current->DetachAppInfoToStubIndex(session->GetPeerPid(), session->GetPeerUid(),
         tokenId, session->GetPeerDeviceId(), IPCProcessSkeleton::ConvertChannelID2Int(channelId));
+    std::lock_guard<std::mutex> lockGuard(GetObjectMutex());
     for (auto it = stubIndexs.begin(); it != stubIndexs.end(); it++) {
         // note that we canont remove mapping from stub to index here because other session may still be used
         IRemoteObject *stub = current->QueryStubByIndex(*it);

services/dbinder/dbinder_service/src/socket/dbinder_remote_listener.cpp

@@ -59,7 +59,6 @@ bool DBinderRemoteListener::StartListener(std::shared_ptr<DBinderRemoteListener>
 
 bool DBinderRemoteListener::StopListener()
 {
-    ClearDeviceLock();
     std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
     if (softbusManager_ == nullptr) {
         DBINDER_LOGE(LOG_LABEL, "softbus manager is null");
@@ -154,7 +153,6 @@ bool DBinderRemoteListener::SendDataReply(const std::string &deviceId, const str
 
 bool DBinderRemoteListener::CloseDatabusSession(const std::string &deviceId)
 {
-    EraseDeviceLock(deviceId);
     std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
     if (softbusManager_ == nullptr) {
         DBINDER_LOGE(LOG_LABEL, "softbus manager is null");
@@ -175,23 +173,16 @@ bool DBinderRemoteListener::CloseDatabusSession(const std::string &deviceId)
 
 std::shared_ptr<Session> DBinderRemoteListener::OpenSoftbusSession(const std::string &peerDeviceId)
 {
-    {
-        std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
-        if (softbusManager_ == nullptr) {
-            DBINDER_LOGE(LOG_LABEL, "softbus manager is null");
-            return nullptr;
-        }
-        auto it = clientSessionMap_.find(peerDeviceId);
-        if (it != clientSessionMap_.end()) {
-            return it->second;
-        }
-    }
-    std::shared_ptr<DeviceLock> lockInfo = QueryOrNewDeviceLock(peerDeviceId);
-    if (lockInfo == nullptr) {
+    std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
+    if (softbusManager_ == nullptr) {
+        DBINDER_LOGE(LOG_LABEL, "softbus manager is null");
         return nullptr;
     }
-    // OpenSession is not thread-safe
-    std::lock_guard<std::mutex> lock_unique(lockInfo->mutex);
+    auto it = clientSessionMap_.find(peerDeviceId);
+    if (it != clientSessionMap_.end()) {
+        return it->second;
+    }
+
     std::shared_ptr<Session> session = softbusManager_->OpenSession(OWN_SESSION_NAME, PEER_SESSION_NAME,
         peerDeviceId, std::string(""), Session::TYPE_BYTES);
     if (session == nullptr) {
@@ -199,10 +190,8 @@ std::shared_ptr<Session> DBinderRemoteListener::OpenSoftbusSession(const std::st
             DBinderService::ConvertToSecureDeviceID(peerDeviceId).c_str());
         return nullptr;
     }
-    {
-        std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
-        clientSessionMap_.insert(std::pair<std::string, std::shared_ptr<Session>>(peerDeviceId, session));
-    }
+
+    clientSessionMap_.insert(std::pair<std::string, std::shared_ptr<Session>>(peerDeviceId, session));
     return session;
 }
 
@@ -249,7 +238,6 @@ void DBinderRemoteListener::OnSessionClosed(std::shared_ptr<Session> session)
             }
         }
     } else {
-        EraseDeviceLock(session->GetPeerDeviceId());
         std::lock_guard<std::mutex> lockGuard(busManagerMutex_);
         for (auto it = clientSessionMap_.begin(); it != clientSessionMap_.end(); it++) {
             if (it->second->GetChannelId() == session->GetChannelId()) {