[native] Restore ServiceManager#getService() to return IBinder
This fixes a crash in 3p libraries.
A new API ServiceManager#getService2() has been introduced to
work with the Service enum type.
Bug: 354674329
Bug: 355187769
Test: atest servicemanager_test
Test: atest vm_accessor_test
Test: Run the demo app in b/354674329 and check it works
Change-Id: If1e0e9bee6dcd3cfceea69bea58ed5fbe431e81d
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index c44e540..ef2fa4d 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -392,7 +392,16 @@
}
}
-Status ServiceManager::getService(const std::string& name, os::Service* outService) {
+Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinder) {
+ SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
+ PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
+
+ *outBinder = tryGetBinder(name, true);
+ // returns ok regardless of result for legacy reasons
+ return Status::ok();
+}
+
+Status ServiceManager::getService2(const std::string& name, os::Service* outService) {
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS(
PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str())));
diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h
index 18bae68..0d666c6 100644
--- a/cmds/servicemanager/ServiceManager.h
+++ b/cmds/servicemanager/ServiceManager.h
@@ -44,7 +44,8 @@
~ServiceManager();
// getService will try to start any services it cannot find
- binder::Status getService(const std::string& name, os::Service* outService) override;
+ binder::Status getService(const std::string& name, sp<IBinder>* outBinder) override;
+ binder::Status getService2(const std::string& name, os::Service* outService) override;
binder::Status checkService(const std::string& name, os::Service* outService) override;
binder::Status addService(const std::string& name, const sp<IBinder>& binder,
bool allowIsolated, int32_t dumpPriority) override;
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 9d22641..95f459f 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -155,8 +155,11 @@
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
Service outA;
- EXPECT_TRUE(sm->getService("foo", &outA).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &outA).isOk());
EXPECT_EQ(serviceA, outA.get<Service::Tag::binder>());
+ sp<IBinder> outBinderA;
+ EXPECT_TRUE(sm->getService("foo", &outBinderA).isOk());
+ EXPECT_EQ(serviceA, outBinderA);
// serviceA should be overwritten by serviceB
sp<IBinder> serviceB = getBinder();
@@ -164,8 +167,11 @@
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
Service outB;
- EXPECT_TRUE(sm->getService("foo", &outB).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &outB).isOk());
EXPECT_EQ(serviceB, outB.get<Service::Tag::binder>());
+ sp<IBinder> outBinderB;
+ EXPECT_TRUE(sm->getService("foo", &outBinderB).isOk());
+ EXPECT_EQ(serviceB, outBinderB);
}
TEST(AddService, NoPermissions) {
@@ -188,16 +194,22 @@
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
Service out;
- EXPECT_TRUE(sm->getService("foo", &out).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &out).isOk());
EXPECT_EQ(service, out.get<Service::Tag::binder>());
+ sp<IBinder> outBinder;
+ EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
+ EXPECT_EQ(service, outBinder);
}
TEST(GetService, NonExistant) {
auto sm = getPermissiveServiceManager();
Service out;
- EXPECT_TRUE(sm->getService("foo", &out).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &out).isOk());
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
+ sp<IBinder> outBinder;
+ EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
+ EXPECT_EQ(nullptr, outBinder);
}
TEST(GetService, NoPermissionsForGettingService) {
@@ -205,7 +217,7 @@
EXPECT_CALL(*access, getCallingContext()).WillRepeatedly(Return(Access::CallingContext{}));
EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));
- EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(false));
+ EXPECT_CALL(*access, canFind(_, _)).WillRepeatedly(Return(false));
sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access));
@@ -214,22 +226,28 @@
Service out;
// returns nullptr but has OK status for legacy compatibility
- EXPECT_TRUE(sm->getService("foo", &out).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &out).isOk());
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
+ sp<IBinder> outBinder;
+ EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
+ EXPECT_EQ(nullptr, outBinder);
}
TEST(GetService, AllowedFromIsolated) {
std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();
EXPECT_CALL(*access, getCallingContext())
- // something adds it
- .WillOnce(Return(Access::CallingContext{}))
- // next call is from isolated app
- .WillOnce(Return(Access::CallingContext{
- .uid = AID_ISOLATED_START,
- }));
+ // something adds it
+ .WillOnce(Return(Access::CallingContext{}))
+ // next calls is from isolated app
+ .WillOnce(Return(Access::CallingContext{
+ .uid = AID_ISOLATED_START,
+ }))
+ .WillOnce(Return(Access::CallingContext{
+ .uid = AID_ISOLATED_START,
+ }));
EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));
- EXPECT_CALL(*access, canFind(_, _)).WillOnce(Return(true));
+ EXPECT_CALL(*access, canFind(_, _)).WillRepeatedly(Return(true));
sp<ServiceManager> sm = sp<NiceMock<MockServiceManager>>::make(std::move(access));
@@ -238,20 +256,26 @@
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
Service out;
- EXPECT_TRUE(sm->getService("foo", &out).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &out).isOk());
EXPECT_EQ(service, out.get<Service::Tag::binder>());
+ sp<IBinder> outBinder;
+ EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
+ EXPECT_EQ(service, outBinder);
}
TEST(GetService, NotAllowedFromIsolated) {
std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();
EXPECT_CALL(*access, getCallingContext())
- // something adds it
- .WillOnce(Return(Access::CallingContext{}))
- // next call is from isolated app
- .WillOnce(Return(Access::CallingContext{
- .uid = AID_ISOLATED_START,
- }));
+ // something adds it
+ .WillOnce(Return(Access::CallingContext{}))
+ // next calls is from isolated app
+ .WillOnce(Return(Access::CallingContext{
+ .uid = AID_ISOLATED_START,
+ }))
+ .WillOnce(Return(Access::CallingContext{
+ .uid = AID_ISOLATED_START,
+ }));
EXPECT_CALL(*access, canAdd(_, _)).WillOnce(Return(true));
// TODO(b/136023468): when security check is first, this should be called first
@@ -264,8 +288,11 @@
Service out;
// returns nullptr but has OK status for legacy compatibility
- EXPECT_TRUE(sm->getService("foo", &out).isOk());
+ EXPECT_TRUE(sm->getService2("foo", &out).isOk());
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
+ sp<IBinder> outBinder;
+ EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
+ EXPECT_EQ(nullptr, outBinder);
}
TEST(ListServices, NoPermissions) {