diff options
| author | 2019-09-10 10:18:00 -0700 | |
|---|---|---|
| committer | 2019-09-17 11:21:18 -0700 | |
| commit | 86a17f87f6a588ed291f7946889fa64fcdad88ec (patch) | |
| tree | 63f35ecd16ac060d4156508cb72d38239d92e7b5 | |
| parent | 13286c3b510661eb5f1ffbdad5e5a6881a1dbb5e (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.bp | 7 | ||||
| -rw-r--r-- | cmds/servicemanager/ServiceManager.cpp | 45 | ||||
| -rw-r--r-- | libs/binder/BpBinder.cpp | 3 | ||||
| -rw-r--r-- | libs/binder/Stability.cpp | 12 | ||||
| -rw-r--r-- | libs/binder/include/binder/Stability.h | 3 | ||||
| -rw-r--r-- | libs/binder/tests/binderStabilityTest.cpp | 26 |
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); |