summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/binder/RpcServer.cpp34
-rw-r--r--libs/binder/RpcSession.cpp22
-rw-r--r--libs/binder/RpcState.cpp15
-rw-r--r--libs/binder/RpcState.h2
-rw-r--r--libs/binder/RpcWireFormat.h22
-rw-r--r--libs/binder/include/binder/RpcAddress.h6
-rw-r--r--libs/binder/include/binder/RpcServer.h4
-rw-r--r--libs/binder/include/binder/RpcSession.h9
8 files changed, 58 insertions, 56 deletions
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index ad377d3737..3599427f5a 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -270,14 +270,25 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie
return;
}
- if (header.sessionId == RPC_SESSION_ID_NEW) {
+ RpcAddress sessionId = RpcAddress::fromRawEmbedded(&header.sessionId);
+
+ if (sessionId.isZero()) {
if (reverse) {
ALOGE("Cannot create a new session with a reverse connection, would leak");
return;
}
- LOG_ALWAYS_FATAL_IF(server->mSessionIdCounter >= INT32_MAX, "Out of session IDs");
- server->mSessionIdCounter++;
+ RpcAddress sessionId = RpcAddress::zero();
+ size_t tries = 0;
+ do {
+ // don't block if there is some entropy issue
+ if (tries++ > 5) {
+ ALOGE("Cannot find new address: %s", sessionId.toString().c_str());
+ return;
+ }
+
+ sessionId = RpcAddress::random(true /*forServer*/);
+ } while (server->mSessions.end() != server->mSessions.find(sessionId));
session = RpcSession::make();
session->setMaxThreads(server->mMaxThreads);
@@ -285,16 +296,17 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie
sp<RpcServer::EventListener>::fromExisting(
static_cast<RpcServer::EventListener*>(
server.get())),
- server->mSessionIdCounter)) {
+ sessionId)) {
ALOGE("Failed to attach server to session");
return;
}
- server->mSessions[server->mSessionIdCounter] = session;
+ server->mSessions[sessionId] = session;
} else {
- auto it = server->mSessions.find(header.sessionId);
+ auto it = server->mSessions.find(sessionId);
if (it == server->mSessions.end()) {
- ALOGE("Cannot add thread, no record of session with ID %d", header.sessionId);
+ ALOGE("Cannot add thread, no record of session with ID %s",
+ sessionId.toString().c_str());
return;
}
session = it->second;
@@ -353,12 +365,14 @@ bool RpcServer::setupSocketServer(const RpcSocketAddress& addr) {
void RpcServer::onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) {
auto id = session->mId;
LOG_ALWAYS_FATAL_IF(id == std::nullopt, "Server sessions must be initialized with ID");
- LOG_RPC_DETAIL("Dropping session %d", *id);
+ LOG_RPC_DETAIL("Dropping session with address %s", id->toString().c_str());
std::lock_guard<std::mutex> _l(mLock);
auto it = mSessions.find(*id);
- LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %d", *id);
- LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %d", *id);
+ LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %s",
+ id->toString().c_str());
+ LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %s",
+ id->toString().c_str());
(void)mSessions.erase(it);
}
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index f953a05c08..931a876ac4 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -218,18 +218,17 @@ status_t RpcSession::readId() {
LOG_ALWAYS_FATAL_IF(mForServer != nullptr, "Can only update ID for client.");
}
- int32_t id;
-
ExclusiveConnection connection;
status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
ConnectionUse::CLIENT, &connection);
if (status != OK) return status;
- status = state()->getSessionId(connection.get(), sp<RpcSession>::fromExisting(this), &id);
+ mId = RpcAddress::zero();
+ status = state()->getSessionId(connection.get(), sp<RpcSession>::fromExisting(this),
+ &mId.value());
if (status != OK) return status;
- LOG_RPC_DETAIL("RpcSession %p has id %d", this, id);
- mId = id;
+ LOG_RPC_DETAIL("RpcSession %p has id %s", this, mId->toString().c_str());
return OK;
}
@@ -329,7 +328,7 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) {
mOutgoingConnections.size());
}
- if (!setupOneSocketConnection(addr, RPC_SESSION_ID_NEW, false /*reverse*/)) return false;
+ if (!setupOneSocketConnection(addr, RpcAddress::zero(), false /*reverse*/)) return false;
// TODO(b/189955605): we should add additional sessions dynamically
// instead of all at once.
@@ -366,7 +365,8 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) {
return true;
}
-bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t id, bool reverse) {
+bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, const RpcAddress& id,
+ bool reverse) {
for (size_t tries = 0; tries < 5; tries++) {
if (tries > 0) usleep(10000);
@@ -390,9 +390,9 @@ bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t
return false;
}
- RpcConnectionHeader header{
- .sessionId = id,
- };
+ RpcConnectionHeader header{.options = 0};
+ memcpy(&header.sessionId, &id.viewRawEmbedded(), sizeof(RpcWireAddress));
+
if (reverse) header.options |= RPC_CONNECTION_OPTION_REVERSE;
if (sizeof(header) != TEMP_FAILURE_RETRY(write(serverFd.get(), &header, sizeof(header)))) {
@@ -469,7 +469,7 @@ bool RpcSession::addOutgoingConnection(unique_fd fd) {
}
bool RpcSession::setForServer(const wp<RpcServer>& server, const wp<EventListener>& eventListener,
- int32_t sessionId) {
+ const RpcAddress& sessionId) {
LOG_ALWAYS_FATAL_IF(mForServer != nullptr);
LOG_ALWAYS_FATAL_IF(server == nullptr);
LOG_ALWAYS_FATAL_IF(mEventListener != nullptr);
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 15eec20d1d..fd2eff6870 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -369,7 +369,7 @@ status_t RpcState::getMaxThreads(const sp<RpcSession::RpcConnection>& connection
}
status_t RpcState::getSessionId(const sp<RpcSession::RpcConnection>& connection,
- const sp<RpcSession>& session, int32_t* sessionIdOut) {
+ const sp<RpcSession>& session, RpcAddress* sessionIdOut) {
Parcel data;
data.markForRpc(session);
Parcel reply;
@@ -382,12 +382,7 @@ status_t RpcState::getSessionId(const sp<RpcSession::RpcConnection>& connection,
return status;
}
- int32_t sessionId;
- status = reply.readInt32(&sessionId);
- if (status != OK) return status;
-
- *sessionIdOut = sessionId;
- return OK;
+ return sessionIdOut->readFromParcel(reply);
}
status_t RpcState::transact(const sp<RpcSession::RpcConnection>& connection,
@@ -767,9 +762,9 @@ processTransactInternalTailCall:
}
case RPC_SPECIAL_TRANSACT_GET_SESSION_ID: {
// for client connections, this should always report the value
- // originally returned from the server
- int32_t id = session->mId.value();
- replyStatus = reply.writeInt32(id);
+ // originally returned from the server, so this is asserting
+ // that it exists
+ replyStatus = session->mId.value().writeToParcel(&reply);
break;
}
default: {
diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h
index d306595d83..529dee534c 100644
--- a/libs/binder/RpcState.h
+++ b/libs/binder/RpcState.h
@@ -62,7 +62,7 @@ public:
status_t getMaxThreads(const sp<RpcSession::RpcConnection>& connection,
const sp<RpcSession>& session, size_t* maxThreadsOut);
status_t getSessionId(const sp<RpcSession::RpcConnection>& connection,
- const sp<RpcSession>& session, int32_t* sessionIdOut);
+ const sp<RpcSession>& session, RpcAddress* sessionIdOut);
[[nodiscard]] status_t transact(const sp<RpcSession::RpcConnection>& connection,
const sp<IBinder>& address, uint32_t code, const Parcel& data,
diff --git a/libs/binder/RpcWireFormat.h b/libs/binder/RpcWireFormat.h
index 4bd8e36aad..2016483138 100644
--- a/libs/binder/RpcWireFormat.h
+++ b/libs/binder/RpcWireFormat.h
@@ -20,20 +20,26 @@ namespace android {
#pragma clang diagnostic push
#pragma clang diagnostic error "-Wpadded"
-constexpr int32_t RPC_SESSION_ID_NEW = -1;
-
enum : uint8_t {
RPC_CONNECTION_OPTION_REVERSE = 0x1,
};
+constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_CREATED = 1 << 0; // distinguish from '0' address
+constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_FOR_SERVER = 1 << 1;
+
+struct RpcWireAddress {
+ uint64_t options;
+ uint8_t address[32];
+};
+
/**
* This is sent to an RpcServer in order to request a new connection is created,
* either as part of a new session or an existing session
*/
struct RpcConnectionHeader {
- int32_t sessionId;
+ RpcWireAddress sessionId;
uint8_t options;
- uint8_t reserved[3];
+ uint8_t reserved[7];
};
#define RPC_CONNECTION_INIT_OKAY "cci"
@@ -89,14 +95,6 @@ struct RpcWireHeader {
uint32_t reserved[2];
};
-constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_CREATED = 1 << 0; // distinguish from '0' address
-constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_FOR_SERVER = 1 << 1;
-
-struct RpcWireAddress {
- uint64_t options;
- uint8_t address[32];
-};
-
struct RpcWireTransaction {
RpcWireAddress address;
uint32_t code;
diff --git a/libs/binder/include/binder/RpcAddress.h b/libs/binder/include/binder/RpcAddress.h
index e856fa94ba..e428908c88 100644
--- a/libs/binder/include/binder/RpcAddress.h
+++ b/libs/binder/include/binder/RpcAddress.h
@@ -29,11 +29,7 @@ class Parcel;
struct RpcWireAddress;
/**
- * This class represents an identifier of a binder object.
- *
- * The purpose of this class it to hide the ABI of an RpcWireAddress, and
- * potentially allow us to change the size of it in the future (RpcWireAddress
- * is PIMPL, essentially - although the type that is used here is not exposed).
+ * This class represents an identifier across an RPC boundary.
*/
class RpcAddress {
public:
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index fdcb3a88c6..c8d2857347 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -17,6 +17,7 @@
#include <android-base/unique_fd.h>
#include <binder/IBinder.h>
+#include <binder/RpcAddress.h>
#include <binder/RpcSession.h>
#include <utils/Errors.h>
#include <utils/RefBase.h>
@@ -171,8 +172,7 @@ private:
std::map<std::thread::id, std::thread> mConnectingThreads;
sp<IBinder> mRootObject;
wp<IBinder> mRootObjectWeak;
- std::map<int32_t, sp<RpcSession>> mSessions;
- int32_t mSessionIdCounter = 0;
+ std::map<RpcAddress, sp<RpcSession>> mSessions;
std::unique_ptr<RpcSession::FdTrigger> mShutdownTrigger;
std::condition_variable mShutdownCv;
};
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index 218de205f5..1548e763b4 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -221,12 +221,12 @@ private:
static void join(sp<RpcSession>&& session, PreJoinSetupResult&& result);
[[nodiscard]] bool setupSocketClient(const RpcSocketAddress& address);
- [[nodiscard]] bool setupOneSocketConnection(const RpcSocketAddress& address, int32_t sessionId,
- bool server);
+ [[nodiscard]] bool setupOneSocketConnection(const RpcSocketAddress& address,
+ const RpcAddress& sessionId, bool server);
[[nodiscard]] bool addOutgoingConnection(base::unique_fd fd);
[[nodiscard]] bool setForServer(const wp<RpcServer>& server,
const wp<RpcSession::EventListener>& eventListener,
- int32_t sessionId);
+ const RpcAddress& sessionId);
sp<RpcConnection> assignIncomingConnectionToThisThread(base::unique_fd fd);
[[nodiscard]] bool removeIncomingConnection(const sp<RpcConnection>& connection);
@@ -278,8 +278,7 @@ private:
sp<WaitForShutdownListener> mShutdownListener; // used for client sessions
wp<EventListener> mEventListener; // mForServer if server, mShutdownListener if client
- // TODO(b/183988761): this shouldn't be guessable
- std::optional<int32_t> mId;
+ std::optional<RpcAddress> mId;
std::unique_ptr<FdTrigger> mShutdownTrigger;
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-20 09:08:31 +0000