libbinder: RpcServer protocol version error
Return an error if you set an invalid protocol
version on an RpcServer.
Previously, this would cause errors later down
the line.
Bug: 278946301
Test: binderRpcTest
Change-Id: Id496f1d39b2a32ce775e585f7690ae59503dc3aa
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index 9282856..1e91b7e 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -123,8 +123,13 @@
return mMaxThreads;
}
-void RpcServer::setProtocolVersion(uint32_t version) {
+bool RpcServer::setProtocolVersion(uint32_t version) {
+ if (!RpcState::validateProtocolVersion(version)) {
+ return false;
+ }
+
mProtocolVersion = version;
+ return true;
}
void RpcServer::setSupportedFileDescriptorTransportModes(
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index fbad0f7..c3dee16 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -104,11 +104,7 @@
}
bool RpcSession::setProtocolVersionInternal(uint32_t version, bool checkStarted) {
- if (version >= RPC_WIRE_PROTOCOL_VERSION_NEXT &&
- version != RPC_WIRE_PROTOCOL_VERSION_EXPERIMENTAL) {
- ALOGE("Cannot start RPC session with version %u which is unknown (current protocol version "
- "is %u).",
- version, RPC_WIRE_PROTOCOL_VERSION);
+ if (!RpcState::validateProtocolVersion(version)) {
return false;
}
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 03fa699..ff35f5f 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -398,6 +398,18 @@
return OK;
}
+bool RpcState::validateProtocolVersion(uint32_t version) {
+ if (version >= RPC_WIRE_PROTOCOL_VERSION_NEXT &&
+ version != RPC_WIRE_PROTOCOL_VERSION_EXPERIMENTAL) {
+ ALOGE("Cannot use RPC binder protocol version %u which is unknown (current protocol "
+ "version "
+ "is %u).",
+ version, RPC_WIRE_PROTOCOL_VERSION);
+ return false;
+ }
+ return true;
+}
+
status_t RpcState::readNewSessionResponse(const sp<RpcSession::RpcConnection>& connection,
const sp<RpcSession>& session, uint32_t* version) {
RpcNewSessionResponse response;
diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h
index 0e23ea7..1fe71a5 100644
--- a/libs/binder/RpcState.h
+++ b/libs/binder/RpcState.h
@@ -63,6 +63,8 @@
RpcState();
~RpcState();
+ [[nodiscard]] static bool validateProtocolVersion(uint32_t version);
+
[[nodiscard]] status_t readNewSessionResponse(const sp<RpcSession::RpcConnection>& connection,
const sp<RpcSession>& session, uint32_t* version);
[[nodiscard]] status_t sendConnectionInit(const sp<RpcSession::RpcConnection>& connection,
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index 1001b64..1e3560d 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -137,7 +137,7 @@
* used. However, this can be used in order to prevent newer protocol
* versions from ever being used. This is expected to be useful for testing.
*/
- void setProtocolVersion(uint32_t version);
+ [[nodiscard]] bool setProtocolVersion(uint32_t version);
/**
* Set the supported transports for sending and receiving file descriptors.
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 287e077..4d028b4 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -1353,7 +1353,7 @@
base::unique_fd sink(TEMP_FAILURE_RETRY(open("/dev/null", O_RDWR)));
int sinkFd = sink.get();
auto server = RpcServer::make(newTlsFactory(std::get<0>(GetParam())));
- server->setProtocolVersion(std::get<1>(GetParam()));
+ ASSERT_TRUE(server->setProtocolVersion(std::get<1>(GetParam())));
ASSERT_FALSE(server->hasServer());
ASSERT_EQ(OK, server->setupExternalServer(std::move(sink)));
ASSERT_TRUE(server->hasServer());
@@ -1369,7 +1369,7 @@
auto addr = allocateSocketAddress();
auto server = RpcServer::make(newTlsFactory(std::get<0>(GetParam())));
- server->setProtocolVersion(std::get<1>(GetParam()));
+ ASSERT_TRUE(server->setProtocolVersion(std::get<1>(GetParam())));
ASSERT_EQ(OK, server->setupUnixDomainServer(addr.c_str()));
auto joinEnds = std::make_shared<OneOffSignal>();
@@ -1418,7 +1418,9 @@
std::unique_ptr<RpcAuth> auth = std::make_unique<RpcAuthSelfSigned>()) {
auto [socketType, rpcSecurity, certificateFormat, serverVersion] = param;
auto rpcServer = RpcServer::make(newTlsFactory(rpcSecurity));
- rpcServer->setProtocolVersion(serverVersion);
+ if (!rpcServer->setProtocolVersion(serverVersion)) {
+ return AssertionFailure() << "Invalid protocol version: " << serverVersion;
+ }
switch (socketType) {
case SocketType::PRECONNECTED: {
return AssertionFailure() << "Not supported by this test";
diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp
index a9736d5..5e83fbf 100644
--- a/libs/binder/tests/binderRpcTestService.cpp
+++ b/libs/binder/tests/binderRpcTestService.cpp
@@ -118,7 +118,7 @@
auto certVerifier = std::make_shared<RpcCertificateVerifierSimple>();
sp<RpcServer> server = RpcServer::make(newTlsFactory(rpcSecurity, certVerifier));
- server->setProtocolVersion(serverConfig.serverVersion);
+ CHECK(server->setProtocolVersion(serverConfig.serverVersion));
server->setMaxThreads(serverConfig.numThreads);
server->setSupportedFileDescriptorTransportModes(serverSupportedFileDescriptorTransportModes);
diff --git a/libs/binder/tests/binderRpcTestServiceTrusty.cpp b/libs/binder/tests/binderRpcTestServiceTrusty.cpp
index 8557389..5c7a96a 100644
--- a/libs/binder/tests/binderRpcTestServiceTrusty.cpp
+++ b/libs/binder/tests/binderRpcTestServiceTrusty.cpp
@@ -90,7 +90,9 @@
auto server = std::move(*serverOrErr);
serverInfo.server = server;
- serverInfo.server->setProtocolVersion(serverVersion);
+ if (!serverInfo.server->setProtocolVersion(serverVersion)) {
+ return EXIT_FAILURE;
+ }
serverInfo.server->setPerSessionRootObject([=](const void* /*addrPtr*/, size_t /*len*/) {
auto service = sp<MyBinderRpcTestTrusty>::make();
// Assign a unique connection identifier to service->port so
diff --git a/libs/binder/trusty/RpcServerTrusty.cpp b/libs/binder/trusty/RpcServerTrusty.cpp
index 68b0008..8f64323 100644
--- a/libs/binder/trusty/RpcServerTrusty.cpp
+++ b/libs/binder/trusty/RpcServerTrusty.cpp
@@ -67,7 +67,7 @@
// TODO(b/266741352): follow-up to prevent needing this in the future
// Trusty needs to be set to the latest stable version that is in prebuilts there.
- mRpcServer->setProtocolVersion(0);
+ LOG_ALWAYS_FATAL_IF(!mRpcServer->setProtocolVersion(0));
if (mPortAcl) {
// Initialize the array of pointers to uuids.
diff --git a/libs/binder/trusty/include/binder/RpcServerTrusty.h b/libs/binder/trusty/include/binder/RpcServerTrusty.h
index 6678eb8..119f2a3 100644
--- a/libs/binder/trusty/include/binder/RpcServerTrusty.h
+++ b/libs/binder/trusty/include/binder/RpcServerTrusty.h
@@ -59,7 +59,9 @@
size_t msgMaxSize,
std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory = nullptr);
- void setProtocolVersion(uint32_t version) { mRpcServer->setProtocolVersion(version); }
+ [[nodiscard]] bool setProtocolVersion(uint32_t version) {
+ return mRpcServer->setProtocolVersion(version);
+ }
void setSupportedFileDescriptorTransportModes(
const std::vector<RpcSession::FileDescriptorTransportMode>& modes) {
mRpcServer->setSupportedFileDescriptorTransportModes(modes);