[native] Restore ServiceManager#checkService() to return IBinder

This fixes crashes in 3p libraries.

A new API ServiceManager#checkService2() has been introduced to
work with the Service enum type.

Bug: 387175643
Test: atest servicemanager_test
Change-Id: I647f4a11469717c54111afab562a0be2d5260044

9 files changed, 51 insertions, 11 deletions

diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 38a125bb54..59c4d53bc0 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -410,7 +410,16 @@ Status ServiceManager::getService2(const std::string& name, os::Service* outServ
     return Status::ok();
 }
 
-Status ServiceManager::checkService(const std::string& name, os::Service* outService) {
+Status ServiceManager::checkService(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, false).service;
+    // returns ok regardless of result for legacy reasons
+    return Status::ok();
+}
+
+Status ServiceManager::checkService2(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 964abee6af..5c4d891218 100644
--- a/cmds/servicemanager/ServiceManager.h
+++ b/cmds/servicemanager/ServiceManager.h
@@ -46,7 +46,8 @@ public:
     // getService will try to start any services it cannot find
     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 checkService(const std::string& name, sp<IBinder>* outBinder) override;
+    binder::Status checkService2(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;
     binder::Status listServices(int32_t dumpPriority, std::vector<std::string>* outList) override;
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index e620770e18..7ad84faca0 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -204,6 +204,11 @@ TEST(GetService, HappyHappy) {
     sp<IBinder> outBinder;
     EXPECT_TRUE(sm->getService("foo", &outBinder).isOk());
     EXPECT_EQ(service, outBinder);
+
+    EXPECT_TRUE(sm->checkService2("foo", &out).isOk());
+    EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service);
+    EXPECT_TRUE(sm->checkService("foo", &outBinder).isOk());
+    EXPECT_EQ(service, outBinder);
 }
 
 TEST(GetService, NonExistant) {
diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp
index ee3d6af742..4220cdd0e8 100644
--- a/libs/binder/BackendUnifiedServiceManager.cpp
+++ b/libs/binder/BackendUnifiedServiceManager.cpp
@@ -238,7 +238,17 @@ Status BackendUnifiedServiceManager::getService2(const ::std::string& name, os::
     return status;
 }
 
-Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os::Service* _out) {
+Status BackendUnifiedServiceManager::checkService(const ::std::string& name,
+                                                  sp<IBinder>* _aidl_return) {
+    os::Service service;
+    Status status = checkService2(name, &service);
+    if (status.isOk()) {
+        *_aidl_return = service.get<os::Service::Tag::serviceWithMetadata>().service;
+    }
+    return status;
+}
+
+Status BackendUnifiedServiceManager::checkService2(const ::std::string& name, os::Service* _out) {
     os::Service service;
     if (returnIfCached(name, _out)) {
         return Status::ok();
@@ -246,7 +256,7 @@ Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os:
 
     Status status = Status::ok();
     if (mTheRealServiceManager) {
-        status = mTheRealServiceManager->checkService(name, &service);
+        status = mTheRealServiceManager->checkService2(name, &service);
     }
     if (status.isOk()) {
         status = toBinderService(name, service, _out);
diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h
index 2496f62503..c14f28063f 100644
--- a/libs/binder/BackendUnifiedServiceManager.h
+++ b/libs/binder/BackendUnifiedServiceManager.h
@@ -122,7 +122,8 @@ public:
 
     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 checkService(const ::std::string& name, sp<IBinder>* _aidl_return) override;
+    binder::Status checkService2(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;
     binder::Status listServices(int32_t dumpPriority,
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
index 5c72ed3197..719e445794 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -624,7 +624,7 @@ sp<IBinder> CppBackendShim::getService(const String16& name) const {
 
 sp<IBinder> CppBackendShim::checkService(const String16& name) const {
     Service ret;
-    if (!mUnifiedServiceManager->checkService(String8(name).c_str(), &ret).isOk()) {
+    if (!mUnifiedServiceManager->checkService2(String8(name).c_str(), &ret).isOk()) {
         return nullptr;
     }
     return ret.get<Service::Tag::serviceWithMetadata>().service;
diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl
index 69edef86aa..6539238ce7 100644
--- a/libs/binder/aidl/android/os/IServiceManager.aidl
+++ b/libs/binder/aidl/android/os/IServiceManager.aidl
@@ -83,11 +83,20 @@ interface IServiceManager {
 
     /**
      * Retrieve an existing service called @a name from the service
+     * manager. Non-blocking. Returns null if the service does not exist.
+     *
+     * @deprecated TODO(b/355394904): Use checkService2 instead. This does not
+     * return metadata that is included in ServiceWithMetadata
+     */
+    @UnsupportedAppUsage
+    @nullable IBinder checkService(@utf8InCpp String name);
+
+    /**
+     * Retrieve an existing service called @a name from the service
      * manager. Non-blocking. Returns null if the service does not
      * exist.
      */
-    @UnsupportedAppUsage
-    Service checkService(@utf8InCpp String name);
+    Service checkService2(@utf8InCpp String name);
 
     /**
      * Place a new @a service called @a name into the service
diff --git a/libs/binder/servicedispatcher.cpp b/libs/binder/servicedispatcher.cpp
index be990657d5..78fe2a8714 100644
--- a/libs/binder/servicedispatcher.cpp
+++ b/libs/binder/servicedispatcher.cpp
@@ -127,7 +127,12 @@ public:
         // We can't send BpBinder for regular binder over RPC.
         return android::binder::Status::fromStatusT(android::INVALID_OPERATION);
     }
-    android::binder::Status checkService(const std::string&, android::os::Service*) override {
+    android::binder::Status checkService(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 checkService2(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);
     }
diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp
index 19395c2dcd..121e5ae5e9 100644
--- a/libs/binder/tests/binderCacheUnitTest.cpp
+++ b/libs/binder/tests/binderCacheUnitTest.cpp
@@ -74,7 +74,7 @@ class MockAidlServiceManager : public os::IServiceManagerDefault {
 public:
     MockAidlServiceManager() : innerSm() {}
 
-    binder::Status checkService(const ::std::string& name, os::Service* _out) override {
+    binder::Status checkService2(const ::std::string& name, os::Service* _out) override {
         os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata();
         serviceWithMetadata.service = innerSm.getService(String16(name.c_str()));
         serviceWithMetadata.isLazyService = false;
@@ -98,7 +98,7 @@ class MockAidlServiceManager2 : public os::IServiceManagerDefault {
 public:
     MockAidlServiceManager2() : innerSm() {}
 
-    binder::Status checkService(const ::std::string& name, os::Service* _out) override {
+    binder::Status checkService2(const ::std::string& name, os::Service* _out) override {
         os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata();
         serviceWithMetadata.service = innerSm.getService(String16(name.c_str()));
         serviceWithMetadata.isLazyService = true;