From c3cd425f31d329bc0a54d78f8abd31a7d61d5315 Mon Sep 17 00:00:00 2001 From: fanxiaoyu Date: Sat, 6 Aug 2022 07:19:11 +0000 Subject: [PATCH] Fix rpc session bug when there are more than one SA in open process. Signed-off-by: fanxiaoyu --- .../ipc_core/include/ipc_object_proxy.h | 5 ++-- .../ipc_core/include/ipc_object_stub.h | 4 +-- .../libdbinder/include/dbinder_service.h | 2 +- .../src/core/source/ipc_object_proxy.cpp | 25 ++++++++----------- .../src/core/source/ipc_object_stub.cpp | 20 ++++++++------- .../src/core/source/ipc_process_skeleton.cpp | 4 --- ipc/native/src/mock/source/binder_invoker.cpp | 8 +++--- .../mock/source/dbinder_databus_invoker.cpp | 4 +-- .../dbinder_service/src/dbinder_service.cpp | 6 ++--- 9 files changed, 37 insertions(+), 41 deletions(-) diff --git a/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h b/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h index cf2ca65..188c372 100644 --- a/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h +++ b/interfaces/innerkits/ipc_core/include/ipc_object_proxy.h @@ -66,9 +66,8 @@ public: int InvokeListenThread(MessageParcel &data, MessageParcel &reply); int32_t NoticeServiceDie(); - std::string GetPidAndUidInfo(); - - std::string GetDataBusName(); + std::string GetPidAndUidInfo(int32_t systemAbilityId); + std::string GetDataBusName(int32_t systemAbilityId); std::string TransDataBusName(uint32_t uid, uint32_t pid); int GetProto() const; void WaitForInit(); diff --git a/interfaces/innerkits/ipc_core/include/ipc_object_stub.h b/interfaces/innerkits/ipc_core/include/ipc_object_stub.h index 6c6f1b9..dfa4e18 100644 --- a/interfaces/innerkits/ipc_core/include/ipc_object_stub.h +++ b/interfaces/innerkits/ipc_core/include/ipc_object_stub.h @@ -83,8 +83,8 @@ public: private: int32_t GrantDataBusName(uint32_t code, MessageParcel &data, MessageParcel &reply, MessageOption &option); int32_t TransDataBusName(uint32_t code, MessageParcel &data, MessageParcel &reply, MessageOption &option); - std::string CreateDatabusName(int uid, int pid); - std::string GetDataBusName(); + std::string CreateDatabusName(int uid, int pid, int systemAbilityId); + std::string GetDataBusName(int32_t systemAbilityId); bool IsSamgrCall(uint32_t accessToken); bool HasDumpPermission(uint32_t accessToken) const; #endif diff --git a/interfaces/innerkits/libdbinder/include/dbinder_service.h b/interfaces/innerkits/libdbinder/include/dbinder_service.h index af85a10..50fae71 100644 --- a/interfaces/innerkits/libdbinder/include/dbinder_service.h +++ b/interfaces/innerkits/libdbinder/include/dbinder_service.h @@ -160,7 +160,7 @@ private: bool OnRemoteInvokerDataBusMessage(IPCObjectProxy *proxy, struct DHandleEntryTxRx *replyMessage, std::string &remoteDeviceId, int pid, int uid); bool IsDeviceIdIllegal(const std::string &deviceID); - std::string GetDatabusNameByProxy(IPCObjectProxy *proxy); + std::string GetDatabusNameByProxy(IPCObjectProxy *proxy, int32_t systemAbilityId); uint32_t GetSeqNumber(); bool RegisterRemoteProxyInner(std::u16string serviceName, binder_uintptr_t binder); bool CheckSystemAbilityId(int32_t systemAbilityId); diff --git a/ipc/native/src/core/source/ipc_object_proxy.cpp b/ipc/native/src/core/source/ipc_object_proxy.cpp index b3338d4..f12196d 100644 --- a/ipc/native/src/core/source/ipc_object_proxy.cpp +++ b/ipc/native/src/core/source/ipc_object_proxy.cpp @@ -145,11 +145,12 @@ std::u16string IPCObjectProxy::GetInterfaceDescriptor() return remoteDescriptor_; } -std::string IPCObjectProxy::GetPidAndUidInfo() +std::string IPCObjectProxy::GetPidAndUidInfo(int32_t systemAbilityId) { MessageParcel data, reply; MessageOption option; + data.WriteInt32(systemAbilityId); int32_t err = SendRequestInner(false, GET_UIDPID_INFO, data, reply, option); if (err != ERR_NONE) { ZLOGE(LABEL, "GetPidAndUidInfo SendRequestInner return error = %{public}d", err); @@ -158,11 +159,12 @@ std::string IPCObjectProxy::GetPidAndUidInfo() return reply.ReadString(); } -std::string IPCObjectProxy::GetDataBusName() +std::string IPCObjectProxy::GetDataBusName(int32_t systemAbilityId) { MessageParcel data, reply; MessageOption option; + data.WriteInt32(systemAbilityId); int32_t err = SendRequestInner(false, GRANT_DATABUS_NAME, data, reply, option); if (err != ERR_NONE) { ZLOGE(LABEL, "GetDataBusName transact return error = %{public}d", err); @@ -260,21 +262,16 @@ void IPCObjectProxy::OnLastStrongRef(const void *objectId) ZLOGE(LABEL, "OnLastStrongRef current is null"); return; } + if (current->DetachObject(this) == false) { // if detach successfully, this proxy will be destroyed + return; + } #ifndef CONFIG_IPC_SINGLE + ReleaseProto(); std::shared_ptr session = nullptr; + session = current->ProxyQueryDBinderSession(handle_); + (void)current->ProxyDetachDBinderSession(handle_); + (void)current->DetachHandleToIndex(handle_); #endif - { - std::lock_guard lock(current->mutex_); - if (current->DetachObjectInner(this) == false) { // if detach successfully, this proxy will be destroyed - return; - } -#ifndef CONFIG_IPC_SINGLE - ReleaseProto(); - session = current->ProxyQueryDBinderSession(handle_); - (void)current->ProxyDetachDBinderSession(handle_); - (void)current->DetachHandleToIndex(handle_); -#endif - } IRemoteInvoker *invoker = IPCThreadSkeleton::GetDefaultInvoker(); if (invoker != nullptr) { invoker->ReleaseHandle(handle_); diff --git a/ipc/native/src/core/source/ipc_object_stub.cpp b/ipc/native/src/core/source/ipc_object_stub.cpp index 889a628..34ddb5a 100644 --- a/ipc/native/src/core/source/ipc_object_stub.cpp +++ b/ipc/native/src/core/source/ipc_object_stub.cpp @@ -241,7 +241,8 @@ int IPCObjectStub::SendRequest(uint32_t code, MessageParcel &data, MessageParcel result = IPC_STUB_INVALID_DATA_ERR; break; } - std::string sessionName = GetDataBusName(); + int32_t systemAbilityId = data.ReadInt32(); + std::string sessionName = GetDataBusName(systemAbilityId); if (sessionName.empty()) { ZLOGE(LABEL, "sessionName is empty"); result = IPC_STUB_CREATE_BUS_SERVER_ERR; @@ -456,7 +457,6 @@ int32_t IPCObjectStub::IncStubRefs(MessageParcel &data, MessageParcel &reply) ZLOGE(LABEL, "%s: current is null", __func__); return IPC_STUB_CURRENT_NULL_ERR; } - std::string deviceId = IPCSkeleton::GetCallingDeviceID(); if (deviceId.empty()) { ZLOGE(LABEL, "%s: calling error", __func__); @@ -466,11 +466,9 @@ int32_t IPCObjectStub::IncStubRefs(MessageParcel &data, MessageParcel &reply) ZLOGE(LABEL, "%s: attach stub ref info err, already in", __func__); return ERR_NONE; } - if (!current->DecStubRefTimes(this)) { this->IncStrongRef(this); } - return ERR_NONE; } @@ -531,7 +529,7 @@ int32_t IPCObjectStub::AddAuthInfo(MessageParcel &data, MessageParcel &reply, ui return ERR_NONE; } -std::string IPCObjectStub::GetDataBusName() +std::string IPCObjectStub::GetDataBusName(int32_t systemAbilityId) { IPCProcessSkeleton *current = IPCProcessSkeleton::GetCurrent(); if (current == nullptr) { @@ -545,14 +543,15 @@ std::string IPCObjectStub::GetDataBusName() } IPCObjectProxy *samgr = reinterpret_cast(object.GetRefPtr()); - return samgr->GetDataBusName(); + return samgr->GetDataBusName(systemAbilityId); } int32_t IPCObjectStub::GrantDataBusName(uint32_t code, MessageParcel &data, MessageParcel &reply, MessageOption &option) { int pid = IPCSkeleton::GetCallingPid(); int uid = IPCSkeleton::GetCallingUid(); - std::string sessionName = CreateDatabusName(uid, pid); + int systemAbilityId = data.ReadInt32(); + std::string sessionName = CreateDatabusName(uid, pid, systemAbilityId); if (sessionName.empty()) { ZLOGE(LABEL, "pid/uid is invalid, pid = {public}%d, uid = {public}%d", pid, uid); return IPC_STUB_INVALID_DATA_ERR; @@ -573,7 +572,7 @@ int32_t IPCObjectStub::TransDataBusName(uint32_t code, MessageParcel &data, Mess ZLOGE(LABEL, "pid/uid is invalid, pid = {public}%d, uid = {public}%d", remotePid, remoteUid); return IPC_STUB_INVALID_DATA_ERR; } - std::string sessionName = CreateDatabusName(remoteUid, remotePid); + std::string sessionName = CreateDatabusName(remoteUid, remotePid, 0); if (sessionName.empty()) { ZLOGE(LABEL, "pid/uid is invalid, pid = {public}%d, uid = {public}%d", remotePid, remoteUid); return IPC_STUB_INVALID_DATA_ERR; @@ -586,7 +585,7 @@ int32_t IPCObjectStub::TransDataBusName(uint32_t code, MessageParcel &data, Mess return ERR_NONE; } -std::string IPCObjectStub::CreateDatabusName(int uid, int pid) +std::string IPCObjectStub::CreateDatabusName(int uid, int pid, int systemAbilityId) { std::shared_ptr softbusManager = ISessionService::GetInstance(); if (softbusManager == nullptr) { @@ -595,6 +594,9 @@ std::string IPCObjectStub::CreateDatabusName(int uid, int pid) } std::string sessionName = "DBinder" + std::to_string(uid) + std::string("_") + std::to_string(pid); + if (systemAbilityId > 0) { + sessionName += std::string("_") + std::to_string(systemAbilityId); + } if (softbusManager->GrantPermission(uid, pid, sessionName) != ERR_NONE) { ZLOGE(LABEL, "fail to Grant Permission softbus name"); return ""; diff --git a/ipc/native/src/core/source/ipc_process_skeleton.cpp b/ipc/native/src/core/source/ipc_process_skeleton.cpp index 9994b6d..e436fbc 100644 --- a/ipc/native/src/core/source/ipc_process_skeleton.cpp +++ b/ipc/native/src/core/source/ipc_process_skeleton.cpp @@ -467,7 +467,6 @@ bool IPCProcessSkeleton::QueryProxyBySessionHandle(uint32_t handle, std::vector< proxyHandle.push_back(it->first); } } - return true; } @@ -1207,7 +1206,6 @@ bool IPCProcessSkeleton::AttachCommAuthInfo(IRemoteObject *stub, int pid, int ui auto check = [&stub, &pid, &uid, &deviceId, this](const std::shared_ptr &auth) { return IsSameRemoteObject(stub, pid, uid, deviceId, auth); }; - std::unique_lock lockGuard(commAuthMutex_); auto it = std::find_if(commAuth_.begin(), commAuth_.end(), check); if (it != commAuth_.end()) { @@ -1225,7 +1223,6 @@ void IPCProcessSkeleton::DetachCommAuthInfo(IRemoteObject *stub, int pid, int ui auto check = [&stub, &pid, &uid, &deviceId, this](const std::shared_ptr &auth) { return IsSameRemoteObject(stub, pid, uid, deviceId, auth); }; - std::unique_lock lockGuard(commAuthMutex_); commAuth_.remove_if(check); } @@ -1235,7 +1232,6 @@ std::shared_ptr IPCProcessSkeleton::QueryIsAuth(int pid, int uid auto check = [&pid, &uid, &deviceId, this](const std::shared_ptr &auth) { return IsSameRemoteObject(pid, uid, deviceId, auth); }; - std::shared_lock lockGuard(commAuthMutex_); auto it = std::find_if(commAuth_.begin(), commAuth_.end(), check); if (it != commAuth_.end()) { diff --git a/ipc/native/src/mock/source/binder_invoker.cpp b/ipc/native/src/mock/source/binder_invoker.cpp index 3649355..fc4350f 100644 --- a/ipc/native/src/mock/source/binder_invoker.cpp +++ b/ipc/native/src/mock/source/binder_invoker.cpp @@ -570,7 +570,7 @@ int BinderInvoker::HandleReply(MessageParcel *reply) int BinderInvoker::HandleCommandsInner(uint32_t cmd) { int error = ERR_NONE; - ZLOGD(LABEL, "HandleCommands:cmd:%{public}u\n", cmd); + ZLOGD(LABEL, "HandleCommands:cmd:%{public}u", cmd); switch (cmd) { case BR_ERROR: error = input_.ReadInt32(); @@ -679,7 +679,7 @@ int BinderInvoker::TransactWithDriver(bool doRead) bwr.write_consumed = 0; bwr.read_consumed = 0; - ZLOGD(LABEL, "TransactWithDriver write_size:%lld, read_size:%lld\n", bwr.write_size, bwr.read_size); + ZLOGD(LABEL, "TransactWithDriver write_size:%lld, read_size:%lld", bwr.write_size, bwr.read_size); int error = binderConnector_->WriteBinder(BINDER_WRITE_READ, &bwr); if (bwr.write_consumed > 0) { if (bwr.write_consumed < output_.GetDataSize()) { @@ -693,7 +693,7 @@ int BinderInvoker::TransactWithDriver(bool doRead) input_.RewindRead(0); } if (error != ERR_NONE) { - ZLOGE(LABEL, "TransactWithDriver result = %{public}d\n", error); + ZLOGE(LABEL, "TransactWithDriver result = %{public}d", error); } return error; @@ -908,6 +908,8 @@ bool BinderInvoker::FlattenObject(Parcel &parcel, const IRemoteObject *object) c flat.hdr.type = BINDER_TYPE_BINDER; flat.binder = reinterpret_cast(object); flat.cookie = flat.binder; + ZLOGD(LABEL, "write stub object: %{public}s, ptr=%{public}p", + Str16ToStr8(object->GetObjectDescriptor()).c_str(), object); } flat.flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; diff --git a/ipc/native/src/mock/source/dbinder_databus_invoker.cpp b/ipc/native/src/mock/source/dbinder_databus_invoker.cpp index a951164..53d909a 100644 --- a/ipc/native/src/mock/source/dbinder_databus_invoker.cpp +++ b/ipc/native/src/mock/source/dbinder_databus_invoker.cpp @@ -79,7 +79,6 @@ std::shared_ptr DBinderDatabusInvoker::NewSessionOfBinderP DBINDER_LOGE("current ipc process skeleton is nullptr"); return nullptr; } - sptr ipcProxy = reinterpret_cast(current->FindOrNewObject(handle).GetRefPtr()); if (ipcProxy == nullptr) { DBINDER_LOGE("attempt to send a invalid handle = %u", handle); @@ -91,7 +90,7 @@ std::shared_ptr DBinderDatabusInvoker::NewSessionOfBinderP return nullptr; } - std::string sessionName = ipcProxy->GetPidAndUidInfo(); + std::string sessionName = ipcProxy->GetPidAndUidInfo(0); if (sessionName.empty()) { DBINDER_LOGE("get bus name error"); return nullptr; @@ -324,6 +323,7 @@ void DBinderDatabusInvoker::OnMessageAvailable(std::shared_ptr session, readSize += packageSize; } else { // If the current is abnormal, the subsequent is no longer processed. + DBINDER_LOGE("not complete message"); break; } } while (readSize + sizeof(dbinder_transaction_data) < static_cast(len)); diff --git a/services/dbinder/dbinder_service/src/dbinder_service.cpp b/services/dbinder/dbinder_service/src/dbinder_service.cpp index 266cbec..0b54f9f 100644 --- a/services/dbinder/dbinder_service/src/dbinder_service.cpp +++ b/services/dbinder/dbinder_service/src/dbinder_service.cpp @@ -490,7 +490,7 @@ bool DBinderService::OnRemoteInvokerMessage(const struct DHandleEntryTxRx *messa return true; } -std::string DBinderService::GetDatabusNameByProxy(IPCObjectProxy *proxy) +std::string DBinderService::GetDatabusNameByProxy(IPCObjectProxy *proxy, int32_t systemAbilityId) { if (proxy == nullptr) { DBINDER_LOGE("proxy can not be null"); @@ -501,7 +501,7 @@ std::string DBinderService::GetDatabusNameByProxy(IPCObjectProxy *proxy) DBINDER_LOGI("sessionName has been granded"); return sessionName; } - sessionName = proxy->GetPidAndUidInfo(); + sessionName = proxy->GetPidAndUidInfo(systemAbilityId); if (sessionName.empty()) { DBINDER_LOGE("grand session name failed"); return ""; @@ -556,7 +556,7 @@ bool DBinderService::OnRemoteInvokerDataBusMessage(IPCObjectProxy *proxy, struct DBINDER_LOGE("remote device id is error"); return false; } - std::string sessionName = GetDatabusNameByProxy(proxy); + std::string sessionName = GetDatabusNameByProxy(proxy, replyMessage->stubIndex); if (sessionName.empty()) { DBINDER_LOGE("get bus name fail"); return false; -- GitLab