diff options
-rw-r--r-- | cmds/servicemanager/ServiceManager.cpp | 11 | ||||
-rw-r--r-- | cmds/servicemanager/ServiceManager.h | 3 | ||||
-rw-r--r-- | cmds/servicemanager/test_sm.cpp | 69 | ||||
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.cpp | 13 | ||||
-rw-r--r-- | libs/binder/BackendUnifiedServiceManager.h | 3 | ||||
-rw-r--r-- | libs/binder/IServiceManager.cpp | 2 | ||||
-rw-r--r-- | libs/binder/aidl/android/os/IServiceManager.aidl | 16 | ||||
-rw-r--r-- | libs/binder/servicedispatcher.cpp | 7 |
8 files changed, 95 insertions, 29 deletions
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index c44e540c6d..ef2fa4dff7 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -392,7 +392,16 @@ ServiceManager::~ServiceManager() { } } -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 18bae68332..0d666c6bce 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -44,7 +44,8 @@ public: ~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 9d22641ac5..95f459f7f0 100644 --- a/cmds/servicemanager/test_sm.cpp +++ b/cmds/servicemanager/test_sm.cpp @@ -155,8 +155,11 @@ TEST(AddService, OverwriteExistingService) { 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 @@ TEST(AddService, OverwriteExistingService) { 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 @@ TEST(GetService, HappyHappy) { 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 @@ TEST(GetService, NoPermissionsForGettingService) { 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 @@ TEST(GetService, NoPermissionsForGettingService) { 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 @@ TEST(GetService, AllowedFromIsolated) { 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 @@ TEST(GetService, NotAllowedFromIsolated) { 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 0bf3cadd35..54f687b280 100644 --- a/libs/binder/BackendUnifiedServiceManager.cpp +++ b/libs/binder/BackendUnifiedServiceManager.cpp @@ -33,10 +33,19 @@ BackendUnifiedServiceManager::BackendUnifiedServiceManager(const sp<AidlServiceM 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 = 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->getService(name, &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 4715be4580..f5d7e66568 100644 --- a/libs/binder/BackendUnifiedServiceManager.h +++ b/libs/binder/BackendUnifiedServiceManager.h @@ -26,7 +26,8 @@ public: 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 12a18f2a69..8b80aed630 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -143,7 +143,7 @@ protected: // 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 ac95188470..1d1f84fc01 100644 --- a/libs/binder/aidl/android/os/IServiceManager.aidl +++ b/libs/binder/aidl/android/os/IServiceManager.aidl @@ -60,9 +60,23 @@ interface IServiceManager { * 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 201dfbc400..be990657d5 100644 --- a/libs/binder/servicedispatcher.cpp +++ b/libs/binder/servicedispatcher.cpp @@ -118,7 +118,12 @@ int Dispatch(const char* name, const ServiceRetriever& serviceRetriever, 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); } |