servicemanager: check VINTF manifest for VINTF ifs
For interfaces that are exported API to vendor, check that the
interfaces are declared in the manifest before allowing them to
register.
Implements this relationship:
Device using VINTF instance => instance in manifest
The manifest is used for two things:
- understand which HAL interfaces are used so that we don't deprecate
them too early.
- understand which interfaces are on the device: this allows clients to
depend on a HAL interface IFF it is installed.
Bug: 136027762
Test: try registering interface that is and isn't in the VINTF manifest
Change-Id: I8aa09a56c638a6cc3aa93f102caf09da401a143b
diff --git a/cmds/servicemanager/Android.bp b/cmds/servicemanager/Android.bp
index 9cf3c5c..7277e85 100644
--- a/cmds/servicemanager/Android.bp
+++ b/cmds/servicemanager/Android.bp
@@ -15,11 +15,18 @@
shared_libs: [
"libbase",
"libbinder", // also contains servicemanager_interface
+ "libvintf",
"libcutils",
"liblog",
"libutils",
"libselinux",
],
+
+ target: {
+ vendor: {
+ exclude_shared_libs: ["libvintf"],
+ },
+ },
}
cc_binary {
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 463d67f..9344368 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -18,14 +18,52 @@
#include <android-base/logging.h>
#include <android-base/properties.h>
+#include <binder/Stability.h>
#include <cutils/android_filesystem_config.h>
#include <cutils/multiuser.h>
#include <thread>
+#ifndef VENDORSERVICEMANAGER
+#include <vintf/VintfObject.h>
+#include <vintf/constants.h>
+#endif // !VENDORSERVICEMANAGER
+
using ::android::binder::Status;
+using ::android::internal::Stability;
namespace android {
+#ifndef VENDORSERVICEMANAGER
+static bool meetsDeclarationRequirements(const sp<IBinder>& binder, const std::string& name) {
+ if (!Stability::requiresVintfDeclaration(binder)) {
+ return true;
+ }
+
+ size_t firstSlash = name.find('/');
+ size_t lastDot = name.rfind('.', firstSlash);
+ if (firstSlash == std::string::npos || lastDot == std::string::npos) {
+ LOG(ERROR) << "VINTF HALs require names in the format type/instance (e.g. "
+ << "some.package.foo.IFoo/default) but got: " << name;
+ return false;
+ }
+ const std::string package = name.substr(0, lastDot);
+ const std::string iface = name.substr(lastDot+1, firstSlash-lastDot-1);
+ const std::string instance = name.substr(firstSlash+1);
+
+ for (const auto& manifest : {
+ vintf::VintfObject::GetDeviceHalManifest(),
+ vintf::VintfObject::GetFrameworkHalManifest()
+ }) {
+ if (manifest->hasAidlInstance(package, iface, instance)) {
+ return true;
+ }
+ }
+ LOG(ERROR) << "Could not find " << package << "." << iface << "/" << instance
+ << " in the VINTF manifest.";
+ return false;
+}
+#endif // !VENDORSERVICEMANAGER
+
ServiceManager::ServiceManager(std::unique_ptr<Access>&& access) : mAccess(std::move(access)) {}
ServiceManager::~ServiceManager() {
// this should only happen in tests
@@ -119,6 +157,13 @@
return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
}
+#ifndef VENDORSERVICEMANAGER
+ if (!meetsDeclarationRequirements(binder, name)) {
+ // already logged
+ return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
+ }
+#endif // !VENDORSERVICEMANAGER
+
// implicitly unlinked when the binder is removed
if (OK != binder->linkToDeath(this)) {
LOG(ERROR) << "Could not linkToDeath when adding " << name;
diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 74ffde2..c5aa007 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -221,6 +221,9 @@
auto stability = Stability::get(this);
if (CC_UNLIKELY(!Stability::check(stability, Stability::kLocalStability))) {
+ ALOGE("Cannot do a user transaction on a %s binder in a %s context.",
+ Stability::stabilityString(stability).c_str(),
+ Stability::stabilityString(Stability::kLocalStability).c_str());
return BAD_TYPE;
}
}
diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp
index b6f10c8..7ce5e36 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -37,6 +37,10 @@
LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
}
+bool Stability::requiresVintfDeclaration(const sp<IBinder>& binder) {
+ return check(get(binder.get()), Level::VINTF);
+}
+
void Stability::tryMarkCompilationUnit(IBinder* binder) {
(void) set(binder, kLocalStability, false /*log*/);
}
@@ -99,12 +103,6 @@
stable = false;
}
- if (!stable) {
- ALOGE("Cannot do a user transaction on a %s binder in a %s context.",
- stabilityString(provided).c_str(),
- stabilityString(required).c_str());
- }
-
return stable;
}
@@ -123,4 +121,4 @@
}
} // namespace internal
-} // namespace stability
\ No newline at end of file
+} // namespace stability
diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h
index f8240e4..b84657a 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -57,6 +57,9 @@
// break the device during GSI or other tests.
static void markVndk(IBinder* binder);
+ // Returns true if the binder needs to be declared in the VINTF manifest or
+ // else false if the binder is local to the current partition.
+ static bool requiresVintfDeclaration(const sp<IBinder>& binder);
private:
// Parcel needs to read/write stability level in an unstable format.
friend ::android::Parcel;
diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp
index 0336b9e..0bee56c 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -122,6 +122,32 @@
}
};
+TEST(BinderStability, OnlyVintfStabilityBinderNeedsVintfDeclaration) {
+ EXPECT_FALSE(Stability::requiresVintfDeclaration(nullptr));
+ EXPECT_FALSE(Stability::requiresVintfDeclaration(BadStableBinder::undef()));
+ EXPECT_FALSE(Stability::requiresVintfDeclaration(BadStableBinder::system()));
+ EXPECT_FALSE(Stability::requiresVintfDeclaration(BadStableBinder::vendor()));
+
+ EXPECT_TRUE(Stability::requiresVintfDeclaration(BadStableBinder::vintf()));
+}
+
+TEST(BinderStability, VintfStabilityServerMustBeDeclaredInManifest) {
+ sp<IBinder> vintfServer = BadStableBinder::vintf();
+
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("."), vintfServer));
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("/"), vintfServer));
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("/."), vintfServer));
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("a.d.IFoo"), vintfServer));
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("foo"), vintfServer));
+ EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+ android::defaultServiceManager()->addService(String16("a.d.IFoo/foo"), vintfServer));
+}
+
TEST(BinderStability, CantCallVendorBinderInSystemContext) {
sp<IBinder> serverBinder = android::defaultServiceManager()->getService(kSystemStabilityServer);
auto server = interface_cast<IBinderStabilityTest>(serverBinder);