[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) {
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp
index 0bf3cad..54f687b 100644
--- a/libs/binder/BackendUnifiedServiceManager.cpp
+++ b/libs/binder/BackendUnifiedServiceManager.cpp
@@ -33,10 +33,19 @@
sp<AidlServiceManager> BackendUnifiedServiceManager::getImpl() {
return mTheRealServiceManager;
}
+
binder::Status BackendUnifiedServiceManager::getService(const ::std::string& name,
- os::Service* _out) {
+ sp<IBinder>* _aidl_return) {
os::Service service;
- binder::Status status = mTheRealServiceManager->getService(name, &service);
+ binder::Status status = getService2(name, &service);
+ *_aidl_return = service.get<os::Service::Tag::binder>();
+ return status;
+}
+
+binder::Status BackendUnifiedServiceManager::getService2(const ::std::string& name,
+ os::Service* _out) {
+ os::Service service;
+ binder::Status status = mTheRealServiceManager->getService2(name, &service);
toBinderService(service, _out);
return status;
}
diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h
index 4715be4..f5d7e66 100644
--- a/libs/binder/BackendUnifiedServiceManager.h
+++ b/libs/binder/BackendUnifiedServiceManager.h
@@ -26,7 +26,8 @@
explicit BackendUnifiedServiceManager(const sp<os::IServiceManager>& impl);
sp<os::IServiceManager> getImpl();
- binder::Status getService(const ::std::string& name, os::Service* out) override;
+ binder::Status getService(const ::std::string& name, sp<IBinder>* _aidl_return) override;
+ binder::Status getService2(const ::std::string& name, os::Service* out) override;
binder::Status checkService(const ::std::string& name, os::Service* out) override;
binder::Status addService(const ::std::string& name, const sp<IBinder>& service,
bool allowIsolated, int32_t dumpPriority) override;
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
index 12a18f2..8b80aed 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -143,7 +143,7 @@
// mUnifiedServiceManager->getService so that it can be overridden in ServiceManagerHostShim.
virtual Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) {
Service service;
- Status status = mUnifiedServiceManager->getService(name, &service);
+ Status status = mUnifiedServiceManager->getService2(name, &service);
*_aidl_return = service.get<Service::Tag::binder>();
return status;
}
diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl
index ac95188..1d1f84f 100644
--- a/libs/binder/aidl/android/os/IServiceManager.aidl
+++ b/libs/binder/aidl/android/os/IServiceManager.aidl
@@ -60,9 +60,23 @@
* exists for legacy purposes.
*
* Returns null if the service does not exist.
+ *
+ * @deprecated TODO(b/355394904): Use getService2 instead.
*/
@UnsupportedAppUsage
- Service getService(@utf8InCpp String name);
+ @nullable IBinder getService(@utf8InCpp String name);
+
+ /**
+ * Retrieve an existing service called @a name from the
+ * service manager.
+ *
+ * This is the same as checkService (returns immediately) but
+ * exists for legacy purposes.
+ *
+ * Returns an enum Service that can be of different types. The
+ * enum value is null if the service does not exist.
+ */
+ Service getService2(@utf8InCpp String name);
/**
* Retrieve an existing service called @a name from the service
diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp
index 201dfbc..be99065 100644
--- a/libs/binder/servicedispatcher.cpp
+++ b/libs/binder/servicedispatcher.cpp
@@ -118,7 +118,12 @@
class ServiceManagerProxyToNative : public android::os::BnServiceManager {
public:
ServiceManagerProxyToNative(const sp<android::os::IServiceManager>& impl) : mImpl(impl) {}
- android::binder::Status getService(const std::string&, android::os::Service*) override {
+ android::binder::Status getService(const std::string&,
+ android::sp<android::IBinder>*) override {
+ // We can't send BpBinder for regular binder over RPC.
+ return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
+ }
+ android::binder::Status getService2(const std::string&, android::os::Service*) override {
// We can't send BpBinder for regular binder over RPC.
return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
}