Add binder to libbinder cache after addService
This will prevent system_server and other applications
from calling servicemanager for local binders.
Flag: RELEASE_LIBBINDER_CLIENT_CACHE
Bug: 333854840
Test: atest binderCacheUnitTest
Change-Id: I2693f21a3f5b7a5770481e5ac79719444284524d
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 0a61178..e10e4dd 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -477,9 +477,34 @@
},
}
+soong_config_module_type {
+ name: "libbinder_addservice_cache_config",
+ module_type: "cc_defaults",
+ config_namespace: "libbinder",
+ bool_variables: ["release_libbinder_addservice_cache"],
+ properties: [
+ "cflags",
+ ],
+}
+
+libbinder_addservice_cache_config {
+ name: "libbinder_addservice_cache_flag",
+ soong_config_variables: {
+ release_libbinder_addservice_cache: {
+ cflags: ["-DLIBBINDER_ADDSERVICE_CACHE"],
+ conditions_default: {
+ cflags: ["-DNO_LIBBINDER_ADDSERVICE_CACHE"],
+ },
+ },
+ },
+}
+
cc_defaults {
name: "libbinder_kernel_defaults",
- defaults: ["libbinder_client_cache_flag"],
+ defaults: [
+ "libbinder_client_cache_flag",
+ "libbinder_addservice_cache_flag",
+ ],
srcs: [
"BufferedTextOutput.cpp",
"BackendUnifiedServiceManager.cpp",
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp
index d32eecd..123dfd2 100644
--- a/libs/binder/BackendUnifiedServiceManager.cpp
+++ b/libs/binder/BackendUnifiedServiceManager.cpp
@@ -30,6 +30,12 @@
constexpr bool kUseCache = false;
#endif
+#ifdef LIBBINDER_ADDSERVICE_CACHE
+constexpr bool kUseCacheInAddService = true;
+#else
+constexpr bool kUseCacheInAddService = false;
+#endif
+
using AidlServiceManager = android::os::IServiceManager;
using android::os::IAccessor;
using binder::Status;
@@ -125,31 +131,36 @@
if (!kUseCache) {
return Status::ok();
}
+
+ if (service.getTag() == os::Service::Tag::binder) {
+ return updateCache(serviceName, service.get<os::Service::Tag::binder>());
+ }
+ return Status::ok();
+}
+
+Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName,
+ const sp<IBinder>& binder) {
std::string traceStr;
if (atrace_is_tag_enabled(ATRACE_TAG_AIDL)) {
traceStr = "BinderCacheWithInvalidation::updateCache : " + serviceName;
}
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, traceStr.c_str());
-
- if (service.getTag() == os::Service::Tag::binder) {
- sp<IBinder> binder = service.get<os::Service::Tag::binder>();
- if (!binder) {
- binder::ScopedTrace
- aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: binder_null");
- } else if (!binder->isBinderAlive()) {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: "
- "isBinderAlive_false");
- } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache successful");
- return mCacheForGetService->setItem(serviceName, binder);
- } else {
- binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
- "BinderCacheWithInvalidation::updateCache failed: "
- "caching_not_enabled");
- }
+ if (!binder) {
+ binder::ScopedTrace
+ aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: binder_null");
+ } else if (!binder->isBinderAlive()) {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: "
+ "isBinderAlive_false");
+ } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache successful");
+ return mCacheForGetService->setItem(serviceName, binder);
+ } else {
+ binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
+ "BinderCacheWithInvalidation::updateCache failed: "
+ "caching_not_enabled");
}
return Status::ok();
}
@@ -277,7 +288,12 @@
Status BackendUnifiedServiceManager::addService(const ::std::string& name,
const sp<IBinder>& service, bool allowIsolated,
int32_t dumpPriority) {
- return mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority);
+ Status status = mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority);
+ // mEnableAddServiceCache is true by default.
+ if (kUseCacheInAddService && mEnableAddServiceCache && status.isOk()) {
+ return updateCache(name, service);
+ }
+ return status;
}
Status BackendUnifiedServiceManager::listServices(int32_t dumpPriority,
::std::vector<::std::string>* _aidl_return) {
diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h
index abc0eda..b46bf19 100644
--- a/libs/binder/BackendUnifiedServiceManager.h
+++ b/libs/binder/BackendUnifiedServiceManager.h
@@ -105,8 +105,7 @@
binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL,
"BinderCacheWithInvalidation::setItem Successfully Cached");
std::lock_guard<std::mutex> lock(mCacheMutex);
- Entry entry = {.service = item, .deathRecipient = deathRecipient};
- mCache[key] = entry;
+ mCache[key] = {.service = item, .deathRecipient = deathRecipient};
return binder::Status::ok();
}
@@ -147,17 +146,20 @@
const sp<IBinder>& service) override;
binder::Status getServiceDebugInfo(::std::vector<os::ServiceDebugInfo>* _aidl_return) override;
+ void enableAddServiceCache(bool value) { mEnableAddServiceCache = value; }
// for legacy ABI
const String16& getInterfaceDescriptor() const override {
return mTheRealServiceManager->getInterfaceDescriptor();
}
private:
+ bool mEnableAddServiceCache = true;
std::shared_ptr<BinderCacheWithInvalidation> mCacheForGetService;
sp<os::IServiceManager> mTheRealServiceManager;
binder::Status toBinderService(const ::std::string& name, const os::Service& in,
os::Service* _out);
binder::Status updateCache(const std::string& serviceName, const os::Service& service);
+ binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder);
bool returnIfCached(const std::string& serviceName, os::Service* _out);
};
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
index 39d8c24..61866d7 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -127,6 +127,8 @@
}
IBinder* onAsBinder() override { return IInterface::asBinder(mUnifiedServiceManager).get(); }
+ void enableAddServiceCache(bool value) { mUnifiedServiceManager->enableAddServiceCache(value); }
+
protected:
sp<BackendUnifiedServiceManager> mUnifiedServiceManager;
// AidlRegistrationCallback -> services that its been registered for
diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h
index 81f7cdb..ca26a57 100644
--- a/libs/binder/include/binder/IServiceManager.h
+++ b/libs/binder/include/binder/IServiceManager.h
@@ -150,6 +150,12 @@
int pid;
};
virtual std::vector<ServiceDebugInfo> getServiceDebugInfo() = 0;
+
+ /**
+ * Directly enable or disable caching binder during addService calls.
+ * Only used for testing.
+ */
+ virtual void enableAddServiceCache(bool value) = 0;
};
LIBBINDER_EXPORTED sp<IServiceManager> defaultServiceManager();
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 28a3f65..642fbf3 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -70,7 +70,10 @@
static_libs: [
"libfakeservicemanager",
],
- defaults: ["libbinder_client_cache_flag"],
+ defaults: [
+ "libbinder_client_cache_flag",
+ "libbinder_addservice_cache_flag",
+ ],
test_suites: ["general-tests"],
require_root: true,
}
diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp
index c5ad793..3195c55 100644
--- a/libs/binder/tests/binderCacheUnitTest.cpp
+++ b/libs/binder/tests/binderCacheUnitTest.cpp
@@ -34,6 +34,12 @@
constexpr bool kUseLibbinderCache = false;
#endif
+#ifdef LIBBINDER_ADDSERVICE_CACHE
+constexpr bool kUseCacheInAddService = true;
+#else
+constexpr bool kUseCacheInAddService = false;
+#endif
+
// A service name which is in the static list of cachable services
const String16 kCachedServiceName = String16("isub");
@@ -74,14 +80,58 @@
innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority));
}
+ void clearServices() { innerSm.clear(); }
+
FakeServiceManager innerSm;
};
+class LibbinderCacheAddServiceTest : public ::testing::Test {
+protected:
+ void SetUp() override {
+ fakeServiceManager = sp<MockAidlServiceManager>::make();
+ mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager);
+ mServiceManager->enableAddServiceCache(true);
+ }
+
+ void TearDown() override {}
+
+public:
+ void cacheAddServiceAndConfirmCacheHit(const sp<IBinder>& binder1) {
+ // Add a service. This also caches it.
+ EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1));
+ // remove services from fakeservicemanager
+ fakeServiceManager->clearServices();
+
+ sp<IBinder> result = mServiceManager->checkService(kCachedServiceName);
+ if (kUseCacheInAddService && kUseLibbinderCache) {
+ // If cache is enabled, we should get the binder.
+ EXPECT_EQ(binder1, result);
+ } else {
+ // If cache is disabled, then we should get the null binder
+ EXPECT_EQ(nullptr, result);
+ }
+ }
+ sp<MockAidlServiceManager> fakeServiceManager;
+ sp<android::IServiceManager> mServiceManager;
+};
+
+TEST_F(LibbinderCacheAddServiceTest, AddLocalServiceAndConfirmCacheHit) {
+ sp<IBinder> binder1 = sp<BBinder>::make();
+ cacheAddServiceAndConfirmCacheHit(binder1);
+}
+
+TEST_F(LibbinderCacheAddServiceTest, AddRemoteServiceAndConfirmCacheHit) {
+ sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName);
+ ASSERT_NE(binder1, nullptr);
+ cacheAddServiceAndConfirmCacheHit(binder1);
+}
+
class LibbinderCacheTest : public ::testing::Test {
protected:
void SetUp() override {
- sp<MockAidlServiceManager> sm = sp<MockAidlServiceManager>::make();
- mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(sm);
+ fakeServiceManager = sp<MockAidlServiceManager>::make();
+ mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager);
+ mServiceManager->enableAddServiceCache(false);
}
void TearDown() override {}
@@ -108,6 +158,7 @@
}
}
+ sp<MockAidlServiceManager> fakeServiceManager;
sp<android::IServiceManager> mServiceManager;
};
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index ec2f50c..e56b7cf 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -220,6 +220,8 @@
ASSERT_GT(m_serverpid, 0);
sp<IServiceManager> sm = defaultServiceManager();
+ // disable caching during addService.
+ sm->enableAddServiceCache(false);
//printf("%s: pid %d, get service\n", __func__, m_pid);
LIBBINDER_IGNORE("-Wdeprecated-declarations")
m_server = sm->getService(binderLibTestServiceName);
@@ -295,6 +297,9 @@
virtual void SetUp() {
m_server = static_cast<BinderLibTestEnv *>(binder_env)->getServer();
IPCThreadState::self()->restoreCallingWorkSource(0);
+ sp<IServiceManager> sm = defaultServiceManager();
+ // disable caching during addService.
+ sm->enableAddServiceCache(false);
}
virtual void TearDown() {
}
@@ -1644,6 +1649,7 @@
TEST(ServiceNotifications, Unregister) {
auto sm = defaultServiceManager();
+ sm->enableAddServiceCache(false);
using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback;
class LocalRegistrationCallbackImpl : public virtual LocalRegistrationCallback {
void onServiceRegistration(const String16 &, const sp<IBinder> &) override {}
@@ -2374,6 +2380,8 @@
status_t ret;
sp<IServiceManager> sm = defaultServiceManager();
+ sm->enableAddServiceCache(false);
+
BinderLibTestService* testServicePtr;
{
sp<BinderLibTestService> testService = new BinderLibTestService(index);
diff --git a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
index f62241d..f2b2aa7 100644
--- a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
+++ b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h
@@ -65,6 +65,7 @@
std::vector<IServiceManager::ServiceDebugInfo> getServiceDebugInfo() override;
+ void enableAddServiceCache(bool /*value*/) override {}
// Clear all of the registered services
void clear();