summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/binder/Binder.cpp89
-rw-r--r--libs/binder/include/binder/Binder.h5
-rw-r--r--libs/binder/include/binder/IBinder.h9
-rw-r--r--libs/binder/servicedispatcher.cpp5
-rw-r--r--libs/binder/tests/Android.bp2
-rw-r--r--libs/binder/tests/binderLibTest.cpp66
-rw-r--r--libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h3
7 files changed, 132 insertions, 47 deletions
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 194be218e8..d811b17e7e 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -17,6 +17,7 @@
#include <binder/Binder.h>
#include <atomic>
+#include <set>
#include <android-base/unique_fd.h>
#include <binder/BpBinder.h>
@@ -150,7 +151,8 @@ status_t IBinder::getDebugPid(pid_t* out) {
return OK;
}
-status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
+status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder) {
if constexpr (!kEnableRpcDevServers) {
ALOGW("setRpcClientDebug disallowed because RPC is not enabled");
return INVALID_OPERATION;
@@ -158,7 +160,7 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
BBinder* local = this->localBinder();
if (local != nullptr) {
- return local->BBinder::setRpcClientDebug(std::move(socketFd));
+ return local->BBinder::setRpcClientDebug(std::move(socketFd), keepAliveBinder);
}
BpBinder* proxy = this->remoteBinder();
@@ -173,11 +175,44 @@ status_t IBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
status = data.writeFileDescriptor(socketFd.release(), true /* own */);
if (status != OK) return status;
}
+ if (status = data.writeStrongBinder(keepAliveBinder); status != OK) return status;
return transact(SET_RPC_CLIENT_TRANSACTION, data, &reply);
}
// ---------------------------------------------------------------------------
+class BBinder::RpcServerLink : public IBinder::DeathRecipient {
+public:
+ // On binder died, calls RpcServer::shutdown on @a rpcServer, and removes itself from @a binder.
+ RpcServerLink(const sp<RpcServer>& rpcServer, const sp<IBinder>& keepAliveBinder,
+ const wp<BBinder>& binder)
+ : mRpcServer(rpcServer), mKeepAliveBinder(keepAliveBinder), mBinder(binder) {}
+ void binderDied(const wp<IBinder>&) override {
+ LOG_RPC_DETAIL("RpcServerLink: binder died, shutting down RpcServer");
+ if (mRpcServer == nullptr) {
+ ALOGW("RpcServerLink: Unable to shut down RpcServer because it does not exist.");
+ } else {
+ ALOGW_IF(!mRpcServer->shutdown(),
+ "RpcServerLink: RpcServer did not shut down properly. Not started?");
+ }
+ mRpcServer.clear();
+
+ auto promoted = mBinder.promote();
+ if (promoted == nullptr) {
+ ALOGW("RpcServerLink: Unable to remove link from parent binder object because parent "
+ "binder object is gone.");
+ } else {
+ promoted->removeRpcServerLink(sp<RpcServerLink>::fromExisting(this));
+ }
+ mBinder.clear();
+ }
+
+private:
+ sp<RpcServer> mRpcServer;
+ sp<IBinder> mKeepAliveBinder; // hold to avoid automatically unlinking
+ wp<BBinder> mBinder;
+};
+
class BBinder::Extras
{
public:
@@ -190,7 +225,7 @@ public:
// for below objects
Mutex mLock;
- sp<RpcServer> mRpcServer;
+ std::set<sp<RpcServerLink>> mRpcServerLinks;
BpBinder::ObjectManager mObjects;
};
@@ -453,11 +488,14 @@ status_t BBinder::setRpcClientDebug(const Parcel& data) {
if (hasSocketFd) {
if (status = data.readUniqueFileDescriptor(&clientFd); status != OK) return status;
}
+ sp<IBinder> keepAliveBinder;
+ if (status = data.readNullableStrongBinder(&keepAliveBinder); status != OK) return status;
- return setRpcClientDebug(std::move(clientFd));
+ return setRpcClientDebug(std::move(clientFd), keepAliveBinder);
}
-status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
+status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder) {
if constexpr (!kEnableRpcDevServers) {
ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);
return INVALID_OPERATION;
@@ -471,6 +509,11 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
return BAD_VALUE;
}
+ if (keepAliveBinder == nullptr) {
+ ALOGE("%s: No keepAliveBinder provided.", __PRETTY_FUNCTION__);
+ return UNEXPECTED_NULL;
+ }
+
size_t binderThreadPoolMaxCount = ProcessState::self()->getThreadPoolMaxThreadCount();
if (binderThreadPoolMaxCount <= 1) {
ALOGE("%s: ProcessState thread pool max count is %zu. RPC is disabled for this service "
@@ -479,24 +522,38 @@ status_t BBinder::setRpcClientDebug(android::base::unique_fd socketFd) {
return INVALID_OPERATION;
}
+ // Weak ref to avoid circular dependency:
+ // BBinder -> RpcServerLink ----> RpcServer -X-> BBinder
+ // `-X-> BBinder
+ auto weakThis = wp<BBinder>::fromExisting(this);
+
Extras* e = getOrCreateExtras();
AutoMutex _l(e->mLock);
- if (e->mRpcServer != nullptr) {
- ALOGE("%s: Already have RPC client", __PRETTY_FUNCTION__);
- return ALREADY_EXISTS;
+ auto rpcServer = RpcServer::make();
+ LOG_ALWAYS_FATAL_IF(rpcServer == nullptr, "RpcServer::make returns null");
+ rpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
+ auto link = sp<RpcServerLink>::make(rpcServer, keepAliveBinder, weakThis);
+ if (auto status = keepAliveBinder->linkToDeath(link, nullptr, 0); status != OK) {
+ ALOGE("%s: keepAliveBinder->linkToDeath returns %s", __PRETTY_FUNCTION__,
+ statusToString(status).c_str());
+ return status;
}
- e->mRpcServer = RpcServer::make();
- LOG_ALWAYS_FATAL_IF(e->mRpcServer == nullptr, "RpcServer::make returns null");
- e->mRpcServer->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
- // Weak ref to avoid circular dependency: BBinder -> RpcServer -X-> BBinder
- e->mRpcServer->setRootObjectWeak(wp<BBinder>::fromExisting(this));
- e->mRpcServer->setupExternalServer(std::move(socketFd));
- e->mRpcServer->setMaxThreads(binderThreadPoolMaxCount);
- e->mRpcServer->start();
+ rpcServer->setRootObjectWeak(weakThis);
+ rpcServer->setupExternalServer(std::move(socketFd));
+ rpcServer->setMaxThreads(binderThreadPoolMaxCount);
+ rpcServer->start();
+ e->mRpcServerLinks.emplace(link);
LOG_RPC_DETAIL("%s(fd=%d) successful", __PRETTY_FUNCTION__, socketFdForPrint);
return OK;
}
+void BBinder::removeRpcServerLink(const sp<RpcServerLink>& link) {
+ Extras* e = mExtras.load(std::memory_order_acquire);
+ if (!e) return;
+ AutoMutex _l(e->mLock);
+ (void)e->mRpcServerLinks.erase(link);
+}
+
BBinder::~BBinder()
{
Extras* e = mExtras.load(std::memory_order_relaxed);
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 53885ffc4e..472e546944 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -101,7 +101,8 @@ public:
// to another process.
void setParceled();
- [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd);
+ [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
+ const sp<IBinder>& keepAliveBinder);
protected:
virtual ~BBinder();
@@ -116,11 +117,13 @@ private:
BBinder(const BBinder& o);
BBinder& operator=(const BBinder& o);
+ class RpcServerLink;
class Extras;
Extras* getOrCreateExtras();
[[nodiscard]] status_t setRpcClientDebug(const Parcel& data);
+ void removeRpcServerLink(const sp<RpcServerLink>& link);
std::atomic<Extras*> mExtras;
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index 06cb21dac4..f9cdac7de9 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -162,14 +162,17 @@ public:
* 2. spawns 1 new thread that calls RpcServer::join()
* - join() spawns some number of threads that accept() connections; see RpcServer
*
- * setRpcClientDebug() may only be called once.
- * TODO(b/182914638): If allow to shut down the client, addRpcClient can be called repeatedly.
+ * setRpcClientDebug() may be called multiple times. Each call will add a new RpcServer
+ * and opens up a TCP port.
*
* Note: A thread is spawned for each accept()'ed fd, which may call into functions of the
* interface freely. See RpcServer::join(). To avoid such race conditions, implement the service
* functions with multithreading support.
+ *
+ * On death of @a keepAliveBinder, the RpcServer shuts down.
*/
- [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd);
+ [[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd socketFd,
+ const sp<IBinder>& keepAliveBinder);
// NOLINTNEXTLINE(google-default-arguments)
virtual status_t transact( uint32_t code,
diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp
index a2fa842cf6..0c677a8918 100644
--- a/libs/binder/servicedispatcher.cpp
+++ b/libs/binder/servicedispatcher.cpp
@@ -26,9 +26,11 @@
#include <binder/IServiceManager.h>
#include <binder/RpcServer.h>
+using android::BBinder;
using android::defaultServiceManager;
using android::OK;
using android::RpcServer;
+using android::sp;
using android::statusToString;
using android::String16;
using android::base::Basename;
@@ -74,7 +76,8 @@ int Dispatch(const char* name) {
return EX_SOFTWARE;
}
auto socket = rpcServer->releaseServer();
- auto status = binder->setRpcClientDebug(std::move(socket));
+ auto keepAliveBinder = sp<BBinder>::make();
+ auto status = binder->setRpcClientDebug(std::move(socket), keepAliveBinder);
if (status != OK) {
LOG(ERROR) << "setRpcClientDebug failed with " << statusToString(status);
return EX_SOFTWARE;
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index c7c899fcd8..326d9fd58f 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -62,6 +62,7 @@ cc_test {
shared_libs: [
"libbase",
"libbinder",
+ "liblog",
"libutils",
],
static_libs: [
@@ -104,6 +105,7 @@ cc_test {
shared_libs: [
"libbase",
"libbinder",
+ "liblog",
"libutils",
],
static_libs: [
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 91d09163ed..4c3225f302 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -15,14 +15,13 @@
*/
#include <errno.h>
-#include <fcntl.h>
-#include <fstream>
#include <poll.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <chrono>
+#include <fstream>
#include <thread>
#include <gmock/gmock.h>
@@ -31,6 +30,7 @@
#include <android-base/properties.h>
#include <android-base/result-gmock.h>
#include <android-base/result.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <binder/Binder.h>
#include <binder/BpBinder.h>
@@ -55,6 +55,7 @@ using namespace android;
using namespace std::string_literals;
using namespace std::chrono_literals;
using android::base::testing::HasValue;
+using android::base::testing::Ok;
using testing::ExplainMatchResult;
using testing::Not;
using testing::WithParamInterface;
@@ -1199,7 +1200,35 @@ public:
}
};
-class BinderLibRpcTest : public BinderLibRpcTestBase, public WithParamInterface<bool> {
+class BinderLibRpcTest : public BinderLibRpcTestBase {};
+
+TEST_F(BinderLibRpcTest, SetRpcClientDebug) {
+ auto binder = addServer();
+ ASSERT_TRUE(binder != nullptr);
+ auto [socket, port] = CreateSocket();
+ ASSERT_TRUE(socket.ok());
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), sp<BBinder>::make()), StatusEq(OK));
+}
+
+// Tests for multiple RpcServer's on the same binder object.
+TEST_F(BinderLibRpcTest, SetRpcClientDebugTwice) {
+ auto binder = addServer();
+ ASSERT_TRUE(binder != nullptr);
+
+ auto [socket1, port1] = CreateSocket();
+ ASSERT_TRUE(socket1.ok());
+ auto keepAliveBinder1 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1), keepAliveBinder1), StatusEq(OK));
+
+ auto [socket2, port2] = CreateSocket();
+ ASSERT_TRUE(socket2.ok());
+ auto keepAliveBinder2 = sp<BBinder>::make();
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2), keepAliveBinder2), StatusEq(OK));
+}
+
+// Negative tests for RPC APIs on IBinder. Call should fail in the same way on both remote and
+// local binders.
+class BinderLibRpcTestP : public BinderLibRpcTestBase, public WithParamInterface<bool> {
public:
sp<IBinder> GetService() {
return GetParam() ? sp<IBinder>(addServer()) : sp<IBinder>(sp<BBinder>::make());
@@ -1209,35 +1238,22 @@ public:
}
};
-TEST_P(BinderLibRpcTest, SetRpcClientDebug) {
- auto binder = GetService();
- ASSERT_TRUE(binder != nullptr);
- auto [socket, port] = CreateSocket();
- ASSERT_TRUE(socket.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket)), StatusEq(OK));
-}
-
-TEST_P(BinderLibRpcTest, SetRpcClientDebugNoFd) {
+TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoFd) {
auto binder = GetService();
ASSERT_TRUE(binder != nullptr);
- EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd()), StatusEq(BAD_VALUE));
+ EXPECT_THAT(binder->setRpcClientDebug(android::base::unique_fd(), sp<BBinder>::make()),
+ StatusEq(BAD_VALUE));
}
-TEST_P(BinderLibRpcTest, SetRpcClientDebugTwice) {
+TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) {
auto binder = GetService();
ASSERT_TRUE(binder != nullptr);
-
- auto [socket1, port1] = CreateSocket();
- ASSERT_TRUE(socket1.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket1)), StatusEq(OK));
-
- auto [socket2, port2] = CreateSocket();
- ASSERT_TRUE(socket2.ok());
- EXPECT_THAT(binder->setRpcClientDebug(std::move(socket2)), StatusEq(ALREADY_EXISTS));
+ auto [socket, port] = CreateSocket();
+ ASSERT_TRUE(socket.ok());
+ EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), StatusEq(UNEXPECTED_NULL));
}
-
-INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTest, testing::Bool(),
- BinderLibRpcTest::ParamToString);
+INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(),
+ BinderLibRpcTestP::ParamToString);
class BinderLibTestService : public BBinder {
public:
diff --git a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
index 1484fec786..8d2b714b5c 100644
--- a/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/BinderFuzzFunctions.h
@@ -74,7 +74,8 @@ static const std::vector<std::function<void(FuzzedDataProvider*, const sp<BBinde
bbinder->getDebugPid();
},
[](FuzzedDataProvider*, const sp<BBinder>& bbinder) -> void {
- (void)bbinder->setRpcClientDebug(android::base::unique_fd());
+ (void)bbinder->setRpcClientDebug(android::base::unique_fd(),
+ sp<BBinder>::make());
}};
} // namespace android
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-11-03 21:28:50 +0000