Check transport before registering.
Note that we already check this when getting a service.
VTS checks that the static VINTF manifest information directly reflects
the HALs that are at runtime. This is a correctness check that ensures
that the manifest is correct. The VINTF manifest needs to be correct so
that:
- the manifest can be read in order to understand what happens at
runtime. For instnace, to understand which HAL interfaces are still
being used (and therefore the cost of deprecating them).
- so that every HAL in the manifest can be certified by VTS
Fixes: 139274536
Test: hidl_test (passes since servers already declare
TREBLE_TESTING_OVERRIDE in order to load passthrough implementations
from dlopen(nullptr)).
Test: remove entry from manifest and check for log/that service didn't
register.
Change-Id: If851e892729cbda5fb4bf014c79242a0ef5b5152
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 @@
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 @@
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 @@
}
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 @@
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 @@
}
if (registered) {
- onRegistrationImpl(getDescriptor(service.get()), name);
+ onRegistrationImpl(descriptor, name);
}
return registered ? OK : UNKNOWN_ERROR;