Merge "libbinder: mark transact as final"
diff --git a/libs/binder/ndk/Android.bp b/libs/binder/ndk/Android.bp
index 1beacdf..734a928 100644
--- a/libs/binder/ndk/Android.bp
+++ b/libs/binder/ndk/Android.bp
@@ -14,12 +14,12 @@
* limitations under the License.
*/
-cc_library {
+cc_library_shared {
name: "libbinder_ndk",
export_include_dirs: [
"include_ndk",
- "include_apex",
+ "include_platform",
],
cflags: [
@@ -80,6 +80,6 @@
symbol_file: "libbinder_ndk.map.txt",
export_include_dirs: [
"include_ndk",
- "include_apex",
+ "include_platform",
],
}
diff --git a/libs/binder/ndk/include_apex/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h
similarity index 100%
rename from libs/binder/ndk/include_apex/android/binder_manager.h
rename to libs/binder/ndk/include_platform/android/binder_manager.h
diff --git a/libs/binder/ndk/include_apex/android/binder_process.h b/libs/binder/ndk/include_platform/android/binder_process.h
similarity index 100%
rename from libs/binder/ndk/include_apex/android/binder_process.h
rename to libs/binder/ndk/include_platform/android/binder_process.h
diff --git a/libs/binder/ndk/include_apex/android/binder_stability.h b/libs/binder/ndk/include_platform/android/binder_stability.h
similarity index 100%
rename from libs/binder/ndk/include_apex/android/binder_stability.h
rename to libs/binder/ndk/include_platform/android/binder_stability.h
diff --git a/libs/binder/ndk/stability.cpp b/libs/binder/ndk/stability.cpp
index abafe1f..a5b3ece 100644
--- a/libs/binder/ndk/stability.cpp
+++ b/libs/binder/ndk/stability.cpp
@@ -27,6 +27,10 @@
#error libbinder_ndk should only be built in a system context
#endif
+#ifdef __ANDROID_NDK__
+#error libbinder_ndk should only be built in a system context
+#endif
+
// explicit extern because symbol is only declared in header when __ANDROID_VNDK__
extern "C" void AIBinder_markVendorStability(AIBinder* binder) {
Stability::markVndk(binder->getBinder().get());
diff --git a/libs/binder/ndk/test/Android.bp b/libs/binder/ndk/test/Android.bp
index 8cd4e03..bb1fe2f 100644
--- a/libs/binder/ndk/test/Android.bp
+++ b/libs/binder/ndk/test/Android.bp
@@ -44,10 +44,10 @@
"libandroid_runtime_lazy",
"libbase",
"libbinder",
+ "libbinder_ndk",
"libutils",
],
static_libs: [
- "libbinder_ndk",
"test_libbinder_ndk_library",
],
}
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index d59a40d..bc457ce 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -142,7 +142,6 @@
name: "binderStabilityTestIface",
srcs: [
"IBinderStabilityTest.aidl",
- "IBinderStabilityTestSub.aidl",
],
}
diff --git a/libs/binder/tests/IBinderStabilityTest.aidl b/libs/binder/tests/IBinderStabilityTest.aidl
index 9559196..36e1c2c 100644
--- a/libs/binder/tests/IBinderStabilityTest.aidl
+++ b/libs/binder/tests/IBinderStabilityTest.aidl
@@ -14,34 +14,32 @@
* limitations under the License.
*/
-import IBinderStabilityTestSub;
-
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
interface IBinderStabilityTest {
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- void sendBinder(IBinderStabilityTestSub binder);
+ void sendBinder(IBinder binder);
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- void sendAndCallBinder(IBinderStabilityTestSub binder);
+ void sendAndCallBinder(IBinder binder);
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- IBinderStabilityTestSub returnNoStabilityBinder();
+ IBinder returnNoStabilityBinder();
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- IBinderStabilityTestSub returnLocalStabilityBinder();
+ IBinder returnLocalStabilityBinder();
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- IBinderStabilityTestSub returnVintfStabilityBinder();
+ IBinder returnVintfStabilityBinder();
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
- IBinderStabilityTestSub returnVendorStabilityBinder();
+ IBinder returnVendorStabilityBinder();
}
// DO NOT EVER IN A MILLION YEARS WRITE AN INTERFACE LIKE THIS!
// THIS IS ONLY FOR TESTING!
diff --git a/libs/binder/tests/IBinderStabilityTestSub.aidl b/libs/binder/tests/IBinderStabilityTestSub.aidl
deleted file mode 100644
index a81ebf9..0000000
--- a/libs/binder/tests/IBinderStabilityTestSub.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * Copyright (C) 2019 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-interface IBinderStabilityTestSub {
- void userDefinedTransaction();
-}
diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp
index 936b821..490850e 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -26,9 +26,7 @@
#include <sys/prctl.h>
-#include "aidl/BnBinderStabilityTestSub.h"
#include "aidl/BnBinderStabilityTest.h"
-#include "BnBinderStabilityTestSub.h"
#include "BnBinderStabilityTest.h"
using namespace android;
@@ -36,167 +34,217 @@
using android::binder::Status;
using android::internal::Stability; // for testing only!
-const String16 kNoStabilityServer = String16("binder_stability_test_service_low");
-const String16 kCompilationUnitServer = String16("binder_stability_test_service_compl");
-const String16 kVintfServer = String16("binder_stability_test_service_vintf");
+const String16 kSystemStabilityServer = String16("binder_stability_test_service_system");
-const String16 kCompilationUnitNdkServer = String16("binder_stability_test_service_compl");
-
-class BadStabilityTestSub : public BnBinderStabilityTestSub {
+// This is handwritten so that we can test different stability levels w/o having the AIDL
+// compiler assign them. Hand-writing binder interfaces is considered a bad practice
+// sanity reasons. YOU SHOULD DEFINE AN AIDL INTERFACE INSTEAD!
+class BadStableBinder : public BBinder {
public:
- Status userDefinedTransaction() {
- return Status::ok();
+ static constexpr uint32_t USER_TRANSACTION = IBinder::FIRST_CALL_TRANSACTION;
+ static String16 kDescriptor;
+
+ bool gotUserTransaction = false;
+
+ static status_t doUserTransaction(const sp<IBinder>& binder) {
+ Parcel data, reply;
+ data.writeInterfaceToken(kDescriptor);
+ return binder->transact(USER_TRANSACTION, data, &reply, 0/*flags*/);
}
- static sp<IBinderStabilityTestSub> system() {
- sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
- // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
- // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
- Stability::markCompilationUnit(iface.get()); // <- BAD, NO! DO NOT COPY
+ status_t onTransact(uint32_t code,
+ const Parcel& data, Parcel* reply, uint32_t flags) override {
+ if (code == USER_TRANSACTION) {
+ // not interested in this kind of stability. Make sure
+ // we have a test failure
+ LOG_ALWAYS_FATAL_IF(!data.enforceInterface(kDescriptor));
+
+ gotUserTransaction = true;
+
+ ALOGE("binder stability: Got user transaction");
+ return OK;
+ }
+ return BBinder::onTransact(code, data, reply, flags);
+ }
+
+ static sp<BadStableBinder> undef() {
+ sp<BadStableBinder> iface = new BadStableBinder();
return iface;
}
- static sp<IBinderStabilityTestSub> vintf() {
- sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
- // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
- // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
- Stability::markVintf(iface.get()); // <- BAD, NO! DO NOT COPY
+ static sp<BadStableBinder> system() {
+ sp<BadStableBinder> iface = new BadStableBinder();
+ Stability::markCompilationUnit(iface.get()); // <- for test only
return iface;
}
- static sp<IBinderStabilityTestSub> vendor() {
- sp<BnBinderStabilityTestSub> iface = new BadStabilityTestSub();
- // NO! NO! NO! NO! DO NOT EVERY DO SOMETHING LIKE THIS?
- // WHAT ARE YOU CRAZY? IT'S VERY DANGEROUS
- Stability::markVndk(iface.get()); // <- BAD, NO! DO NOT COPY
+ static sp<BadStableBinder> vintf() {
+ sp<BadStableBinder> iface = new BadStableBinder();
+ Stability::markVintf(iface.get()); // <- for test only
+ return iface;
+ }
+
+ static sp<BadStableBinder> vendor() {
+ sp<BadStableBinder> iface = new BadStableBinder();
+ Stability::markVndk(iface.get()); // <- for test only
return iface;
}
};
+String16 BadStableBinder::kDescriptor = String16("BadStableBinder.test");
// NO! NO! NO! Do not even think of doing something like this!
// This is for testing! If a class like this was actually used in production,
// it would ruin everything!
-class BadStabilityTester : public BnBinderStabilityTest {
+class MyBinderStabilityTest : public BnBinderStabilityTest {
public:
- Status sendBinder(const sp<IBinderStabilityTestSub>& /*binder*/) override {
+ Status sendBinder(const sp<IBinder>& /*binder*/) override {
return Status::ok();
}
- Status sendAndCallBinder(const sp<IBinderStabilityTestSub>& binder) override {
- Stability::debugLogStability("sendAndCallBinder got binder", IInterface::asBinder(binder));
- return binder->userDefinedTransaction();
+ Status sendAndCallBinder(const sp<IBinder>& binder) override {
+ Stability::debugLogStability("sendAndCallBinder got binder", binder);
+ return Status::fromExceptionCode(BadStableBinder::doUserTransaction(binder));
}
- Status returnNoStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
- *_aidl_return = new BadStabilityTestSub();
+ Status returnNoStabilityBinder(sp<IBinder>* _aidl_return) override {
+ *_aidl_return = BadStableBinder::undef();
return Status::ok();
}
- Status returnLocalStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
- *_aidl_return = BadStabilityTestSub::system();
+ Status returnLocalStabilityBinder(sp<IBinder>* _aidl_return) override {
+ *_aidl_return = BadStableBinder::system();
return Status::ok();
}
- Status returnVintfStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
- *_aidl_return = BadStabilityTestSub::vintf();
+ Status returnVintfStabilityBinder(sp<IBinder>* _aidl_return) override {
+ *_aidl_return = BadStableBinder::vintf();
return Status::ok();
}
- Status returnVendorStabilityBinder(sp<IBinderStabilityTestSub>* _aidl_return) override {
- *_aidl_return = BadStabilityTestSub::vendor();
+ Status returnVendorStabilityBinder(sp<IBinder>* _aidl_return) override {
+ *_aidl_return = BadStableBinder::vendor();
return Status::ok();
}
};
-void checkSystemStabilityBinder(const sp<IBinderStabilityTest>& complServer) {
- EXPECT_TRUE(complServer->sendBinder(new BadStabilityTestSub()).isOk());
- EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::system()).isOk());
- EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::vintf()).isOk());
- EXPECT_TRUE(complServer->sendBinder(BadStabilityTestSub::vendor()).isOk());
+TEST(BinderStability, CantCallVendorBinderInSystemContext) {
+ sp<IBinder> serverBinder = android::defaultServiceManager()->getService(kSystemStabilityServer);
+ auto server = interface_cast<IBinderStabilityTest>(serverBinder);
- EXPECT_TRUE(complServer->sendAndCallBinder(new BadStabilityTestSub()).isOk());
- EXPECT_TRUE(complServer->sendAndCallBinder(BadStabilityTestSub::system()).isOk());
- EXPECT_TRUE(complServer->sendAndCallBinder(BadStabilityTestSub::vintf()).isOk());
+ ASSERT_NE(nullptr, server.get());
+ ASSERT_NE(nullptr, IInterface::asBinder(server)->remoteBinder());
- // !!! user-defined transaction may not be stable for remote server !!!
- EXPECT_FALSE(complServer->sendAndCallBinder(BadStabilityTestSub::vendor()).isOk());
+ EXPECT_TRUE(server->sendBinder(BadStableBinder::undef()).isOk());
+ EXPECT_TRUE(server->sendBinder(BadStableBinder::system()).isOk());
+ EXPECT_TRUE(server->sendBinder(BadStableBinder::vintf()).isOk());
+ EXPECT_TRUE(server->sendBinder(BadStableBinder::vendor()).isOk());
- sp<IBinderStabilityTestSub> out;
- EXPECT_TRUE(complServer->returnNoStabilityBinder(&out).isOk());
+ {
+ sp<BadStableBinder> binder = BadStableBinder::undef();
+ EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+ EXPECT_TRUE(binder->gotUserTransaction);
+ }
+ {
+ sp<BadStableBinder> binder = BadStableBinder::system();
+ EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+ EXPECT_TRUE(binder->gotUserTransaction);
+ }
+ {
+ sp<BadStableBinder> binder = BadStableBinder::vintf();
+ EXPECT_TRUE(server->sendAndCallBinder(binder).isOk());
+ EXPECT_TRUE(binder->gotUserTransaction);
+ }
+ {
+ // !!! user-defined transaction may not be stable for remote server !!!
+ // !!! so, it does not work !!!
+ sp<BadStableBinder> binder = BadStableBinder::vendor();
+ EXPECT_EQ(BAD_TYPE, server->sendAndCallBinder(binder).exceptionCode());
+ EXPECT_FALSE(binder->gotUserTransaction);
+ }
+
+ sp<IBinder> out;
+ EXPECT_TRUE(server->returnNoStabilityBinder(&out).isOk());
ASSERT_NE(nullptr, out.get());
- EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
- EXPECT_TRUE(out->userDefinedTransaction().isOk());
+ EXPECT_EQ(OK, out->pingBinder());
+ EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
- EXPECT_TRUE(complServer->returnLocalStabilityBinder(&out).isOk());
+ EXPECT_TRUE(server->returnLocalStabilityBinder(&out).isOk());
ASSERT_NE(nullptr, out.get());
- EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
- EXPECT_TRUE(out->userDefinedTransaction().isOk());
+ EXPECT_EQ(OK, out->pingBinder());
+ EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
- EXPECT_TRUE(complServer->returnVintfStabilityBinder(&out).isOk());
+ EXPECT_TRUE(server->returnVintfStabilityBinder(&out).isOk());
ASSERT_NE(nullptr, out.get());
- EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
- EXPECT_TRUE(out->userDefinedTransaction().isOk());
+ EXPECT_EQ(OK, out->pingBinder());
+ EXPECT_EQ(OK, BadStableBinder::doUserTransaction(out));
- EXPECT_TRUE(complServer->returnVendorStabilityBinder(&out).isOk());
+ EXPECT_TRUE(server->returnVendorStabilityBinder(&out).isOk());
ASSERT_NE(nullptr, out.get());
// !!! libbinder-defined transaction works !!!
- EXPECT_EQ(OK, IInterface::asBinder(out)->pingBinder());
+ EXPECT_EQ(OK, out->pingBinder());
// !!! user-defined transaction may not be stable !!!
- EXPECT_FALSE(out->userDefinedTransaction().isOk());
+ // !!! so, it does not work !!!
+ EXPECT_EQ(BAD_TYPE, BadStableBinder::doUserTransaction(out));
}
-TEST(BinderStability, RemoteNoStabilityServer) {
- sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kNoStabilityServer);
- auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
+// This is handwritten so that we can test different stability levels w/o having the AIDL
+// compiler assign them. Hand-writing binder interfaces is considered a bad practice
+// sanity reasons. YOU SHOULD DEFINE AN AIDL INTERFACE INSTEAD!
- ASSERT_NE(nullptr, remoteServer.get());
- ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
-
- checkSystemStabilityBinder(remoteServer);
-}
-
-TEST(BinderStability, RemoteLowStabilityServer) {
- sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kCompilationUnitServer);
- auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
-
- ASSERT_NE(nullptr, remoteServer.get());
- ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
-
- checkSystemStabilityBinder(remoteServer);
-}
-
-TEST(BinderStability, RemoteVintfServer) {
- sp<IBinder> remoteBinder = android::defaultServiceManager()->getService(kVintfServer);
- auto remoteServer = interface_cast<IBinderStabilityTest>(remoteBinder);
-
- ASSERT_NE(nullptr, remoteServer.get());
- ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
-
- checkSystemStabilityBinder(remoteServer);
-}
-
-class NdkBadStabilityTestSub : public aidl::BnBinderStabilityTestSub {
- ScopedAStatus userDefinedTransaction() {
- return ScopedAStatus::ok();
- }
+struct NdkBinderStable_DataClass {
+ bool gotUserTransaction = false;
};
+void* NdkBadStableBinder_Class_onCreate(void* args) {
+ LOG_ALWAYS_FATAL_IF(args != nullptr, "Takes no args");
+ return static_cast<void*>(new NdkBinderStable_DataClass);
+}
+void NdkBadStableBinder_Class_onDestroy(void* userData) {
+ delete static_cast<NdkBinderStable_DataClass*>(userData);
+}
+NdkBinderStable_DataClass* NdkBadStableBinder_getUserData(AIBinder* binder) {
+ LOG_ALWAYS_FATAL_IF(binder == nullptr);
+ void* userData = AIBinder_getUserData(binder);
+ LOG_ALWAYS_FATAL_IF(userData == nullptr, "null data - binder is remote?");
+
+ return static_cast<NdkBinderStable_DataClass*>(userData);
+}
+binder_status_t NdkBadStableBinder_Class_onTransact(
+ AIBinder* binder, transaction_code_t code, const AParcel* /*in*/, AParcel* /*out*/) {
+
+ if (code == BadStableBinder::USER_TRANSACTION) {
+ ALOGE("ndk binder stability: Got user transaction");
+ NdkBadStableBinder_getUserData(binder)->gotUserTransaction = true;
+ return STATUS_OK;
+ }
+
+ return STATUS_UNKNOWN_TRANSACTION;
+}
+
+static AIBinder_Class* kNdkBadStableBinder =
+ AIBinder_Class_define(String8(BadStableBinder::kDescriptor).c_str(),
+ NdkBadStableBinder_Class_onCreate,
+ NdkBadStableBinder_Class_onDestroy,
+ NdkBadStableBinder_Class_onTransact);
+
// for testing only to get around __ANDROID_VNDK__ guard.
extern "C" void AIBinder_markVendorStability(AIBinder* binder); // <- BAD DO NOT COPY
-TEST(BinderStability, NdkClientOfRemoteServer) {
+TEST(BinderStability, NdkCantCallVendorBinderInSystemContext) {
SpAIBinder binder = SpAIBinder(AServiceManager_getService(
- String8(kCompilationUnitServer).c_str()));
+ String8(kSystemStabilityServer).c_str()));
std::shared_ptr<aidl::IBinderStabilityTest> remoteServer =
aidl::IBinderStabilityTest::fromBinder(binder);
ASSERT_NE(nullptr, remoteServer.get());
- std::shared_ptr<aidl::IBinderStabilityTestSub> vendor = SharedRefBase::make<NdkBadStabilityTestSub>();
+ SpAIBinder comp = SpAIBinder(AIBinder_new(kNdkBadStableBinder, nullptr /*args*/));
+ EXPECT_TRUE(remoteServer->sendBinder(comp).isOk());
+ EXPECT_TRUE(remoteServer->sendAndCallBinder(comp).isOk());
+ EXPECT_TRUE(NdkBadStableBinder_getUserData(comp.get())->gotUserTransaction);
- // TODO: not ideal: binder must be held once it is marked
- SpAIBinder vendorBinder = vendor->asBinder();
- AIBinder_markVendorStability(vendorBinder.get());
-
+ SpAIBinder vendor = SpAIBinder(AIBinder_new(kNdkBadStableBinder, nullptr /*args*/));
+ AIBinder_markVendorStability(vendor.get());
EXPECT_TRUE(remoteServer->sendBinder(vendor).isOk());
EXPECT_FALSE(remoteServer->sendAndCallBinder(vendor).isOk());
+ EXPECT_FALSE(NdkBadStableBinder_getUserData(vendor.get())->gotUserTransaction);
}
class MarksStabilityInConstructor : public BBinder {
@@ -234,16 +282,8 @@
// child process
prctl(PR_SET_PDEATHSIG, SIGHUP);
- sp<IBinder> noStability = new BadStabilityTester;
- android::defaultServiceManager()->addService(kNoStabilityServer, noStability);
-
- sp<IBinder> compil = new BadStabilityTester;
- Stability::markCompilationUnit(compil.get());
- android::defaultServiceManager()->addService(kCompilationUnitServer, compil);
-
- sp<IBinder> vintf = new BadStabilityTester;
- Stability::markVintf(vintf.get());
- android::defaultServiceManager()->addService(kVintfServer, vintf);
+ sp<IBinder> server = new MyBinderStabilityTest;
+ android::defaultServiceManager()->addService(kSystemStabilityServer, server);
IPCThreadState::self()->joinThreadPool(true);
exit(1); // should not reach