Merge "servicemanager: check VINTF manifest for VINTF ifs" am: de1ca5a324

am: 910206c397

Change-Id: I630cc325adbe31e078d197367bde6e9f7a6ef07c

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);