diff options
author | Steven Moreland <smoreland@google.com> | 2019-08-20 17:47:26 +0000 |
---|---|---|
committer | Steven Moreland <smoreland@google.com> | 2019-08-21 18:20:15 +0000 |
commit | 84fe49ec372f5aba6d907cbcedb9b64ef4f1f02b (patch) | |
tree | 1e38819c6e4284a0b7669df353867bf0d6601ff2 | |
parent | eff97a8123278dcb42936f97a91e9ffff783ef63 (diff) | |
download | libhidl-84fe49ec372f5aba6d907cbcedb9b64ef4f1f02b.tar.gz |
Reland "Check transport before registering."
This reverts commit eff97a8123278dcb42936f97a91e9ffff783ef63.
Reason for revert: relanding once manifests have been fixed.
Bug: 139274536
Test: boot emulator (that was failing before)
no failed registrations
Test: passes boot health check on P1, P2, P3, P3a
Change-Id: Ie39c616298a7e9d14903019299cccdba194e872c
-rw-r--r-- | transport/ServiceManagement.cpp | 76 |
1 files changed, 49 insertions, 27 deletions
diff --git a/transport/ServiceManagement.cpp b/transport/ServiceManagement.cpp index 357bb30..ab216ae 100644 --- a/transport/ServiceManagement.cpp +++ b/transport/ServiceManagement.cpp @@ -723,11 +723,32 @@ bool handleCastError(const Return<bool>& castReturn, const std::string& descript return false; } +#ifdef ENFORCE_VINTF_MANIFEST +static constexpr bool kEnforceVintfManifest = true; +#else +static constexpr bool kEnforceVintfManifest = false; +#endif + +#ifdef LIBHIDL_TARGET_DEBUGGABLE +static constexpr bool kDebuggable = true; +#else +static constexpr bool kDebuggable = false; +#endif + +static inline bool isTrebleTestingOverride() { + if (kEnforceVintfManifest && !kDebuggable) { + // don't allow testing override in production + return false; + } + + const char* env = std::getenv("TREBLE_TESTING_OVERRIDE"); + return env && !strcmp(env, "true"); +} + sp<::android::hidl::base::V1_0::IBase> getRawServiceInternal(const std::string& descriptor, const std::string& instance, bool retry, bool getStub) { - using Transport = ::android::hidl::manager::V1_0::IServiceManager::Transport; - using ::android::hidl::manager::V1_0::IServiceManager; + using Transport = IServiceManager1_0::Transport; sp<Waiter> waiter; sp<IServiceManager1_1> sm; @@ -753,30 +774,18 @@ sp<::android::hidl::base::V1_0::IBase> getRawServiceInternal(const std::string& const bool vintfHwbinder = (transport == Transport::HWBINDER); const bool vintfPassthru = (transport == Transport::PASSTHROUGH); + const bool trebleTestingOverride = isTrebleTestingOverride(); + const bool allowLegacy = !kEnforceVintfManifest || (trebleTestingOverride && kDebuggable); + const bool vintfLegacy = (transport == Transport::EMPTY) && allowLegacy; -#ifdef ENFORCE_VINTF_MANIFEST - -#ifdef LIBHIDL_TARGET_DEBUGGABLE - const char* env = std::getenv("TREBLE_TESTING_OVERRIDE"); - const bool trebleTestingOverride = env && !strcmp(env, "true"); - const bool vintfLegacy = (transport == Transport::EMPTY) && trebleTestingOverride; -#else // ENFORCE_VINTF_MANIFEST but not LIBHIDL_TARGET_DEBUGGABLE - const bool trebleTestingOverride = false; - const bool vintfLegacy = false; -#endif // LIBHIDL_TARGET_DEBUGGABLE - -#else // not ENFORCE_VINTF_MANIFEST - const char* env = std::getenv("TREBLE_TESTING_OVERRIDE"); - const bool trebleTestingOverride = env && !strcmp(env, "true"); - const bool vintfLegacy = (transport == Transport::EMPTY); - - ALOGE("getService: Potential race detected. The VINTF manifest is not being enforced. If a HAL " - "server has a delay in starting and it is not in the manifest, it will not be retrieved. " - "Please make sure all HALs on this device are in the VINTF manifest and enable " - "PRODUCT_ENFORCE_VINTF_MANIFEST on this device (this is also enabled by " - "PRODUCT_FULL_TREBLE). PRODUCT_ENFORCE_VINTF_MANIFEST will ensure that no race condition " - "is possible here."); -#endif // ENFORCE_VINTF_MANIFEST + if (!kEnforceVintfManifest) { + ALOGE("getService: Potential race detected. The VINTF manifest is not being enforced. If " + "a HAL server has a delay in starting and it is not in the manifest, it will not be " + "retrieved. Please make sure all HALs on this device are in the VINTF manifest and " + "enable PRODUCT_ENFORCE_VINTF_MANIFEST on this device (this is also enabled by " + "PRODUCT_FULL_TREBLE). PRODUCT_ENFORCE_VINTF_MANIFEST will ensure that no race " + "condition is possible here."); + } for (int tries = 0; !getStub && (vintfHwbinder || vintfLegacy); tries++) { if (waiter == nullptr && tries > 0) { @@ -820,7 +829,7 @@ sp<::android::hidl::base::V1_0::IBase> getRawServiceInternal(const std::string& } if (getStub || vintfPassthru || vintfLegacy) { - const sp<IServiceManager> pm = getPassthroughServiceManager(); + const sp<IServiceManager1_0> pm = getPassthroughServiceManager(); if (pm != nullptr) { sp<IBase> base = pm->get(descriptor, instance).withDefault(nullptr); if (!getStub || trebleTestingOverride) { @@ -843,6 +852,19 @@ status_t registerAsServiceInternal(const sp<IBase>& service, const std::string& return INVALID_OPERATION; } + const std::string descriptor = getDescriptor(service.get()); + + if (kEnforceVintfManifest && !isTrebleTestingOverride()) { + using Transport = IServiceManager1_0::Transport; + Transport transport = sm->getTransport(descriptor, name); + + if (transport != Transport::HWBINDER) { + LOG(ERROR) << "Service " << descriptor << "/" << name + << " must be in VINTF manifest in order to register/get."; + return UNKNOWN_ERROR; + } + } + bool registered = false; Return<void> ret = service->interfaceChain([&](const auto& chain) { registered = sm->addWithChain(name.c_str(), service, chain).withDefault(false); @@ -853,7 +875,7 @@ status_t registerAsServiceInternal(const sp<IBase>& service, const std::string& } if (registered) { - onRegistrationImpl(getDescriptor(service.get()), name); + onRegistrationImpl(descriptor, name); } return registered ? OK : UNKNOWN_ERROR; |