summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Steven Moreland <smoreland@google.com> 2019-09-10 10:18:00 -0700
committer Steven Moreland <smoreland@google.com> 2019-09-17 11:21:18 -0700
commit86a17f87f6a588ed291f7946889fa64fcdad88ec (patch)
tree63f35ecd16ac060d4156508cb72d38239d92e7b5
parent13286c3b510661eb5f1ffbdad5e5a6881a1dbb5e (diff)
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
-rw-r--r--cmds/servicemanager/Android.bp7
-rw-r--r--cmds/servicemanager/ServiceManager.cpp45
-rw-r--r--libs/binder/BpBinder.cpp3
-rw-r--r--libs/binder/Stability.cpp12
-rw-r--r--libs/binder/include/binder/Stability.h3
-rw-r--r--libs/binder/tests/binderStabilityTest.cpp26
6 files changed, 89 insertions, 7 deletions
diff --git a/cmds/servicemanager/Android.bp b/cmds/servicemanager/Android.bp
index 9cf3c5c134..7277e85d99 100644
--- a/cmds/servicemanager/Android.bp
+++ b/cmds/servicemanager/Android.bp
@@ -15,11 +15,18 @@ cc_defaults {
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 463d67f945..934436847e 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 @@ Status ServiceManager::addService(const std::string& name, const sp<IBinder>& bi
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 74ffde2175..c5aa0075ab 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -221,6 +221,9 @@ status_t BpBinder::transact(
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 b6f10c8759..7ce5e36292 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -37,6 +37,10 @@ void Stability::markVndk(IBinder* binder) {
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 @@ bool Stability::check(int32_t provided, Level required) {
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 @@ std::string Stability::stabilityString(int32_t stability) {
}
} // 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 f8240e4c72..b84657ac56 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -57,6 +57,9 @@ public:
// 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 0336b9e3ab..0bee56c943 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -122,6 +122,32 @@ public:
}
};
+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);