Revert "Revert "libbinder_rs: Treat previously associated Binder as remote""
This reverts commit f6b9ad98ee61149f42b6ca0bf84068d4411a92f8.
Reason for revert: Includes fix for aidl_integration_test failure that prompted the initial revert
Original commit message:
libbinder_rs: Treat previously associated Binder as remote
A binder object may have already been associated by another module (e.g.
NDK code that created the object if it is local) and therefore already
have an NDK AIBinder_Class. In this case we still want to transact with
the object, but must treat it as remote if its class does not match the
class expected for a compatible Rust service.
Re-landing this change with the fix for aidl_integration_test failure
that prompted its reversion.
Bug: 167723746
Test: atest rustBinderTest binderRustNdkInteropTest
Change-Id: I9a848bcc4b8003a22ebe59c89a95be70ef6ee7ec
diff --git a/libs/binder/rust/tests/Android.bp b/libs/binder/rust/tests/Android.bp
index 3db40ba..5ae9c53 100644
--- a/libs/binder/rust/tests/Android.bp
+++ b/libs/binder/rust/tests/Android.bp
@@ -30,3 +30,52 @@
auto_gen_config: false,
test_suites: ["general-tests"],
}
+
+cc_test {
+ name: "binderRustNdkInteropTest",
+ srcs: [
+ "binderRustNdkInteropTest.cpp",
+ ],
+ shared_libs: [
+ "libbinder",
+ "libbinder_ndk",
+ ],
+ static_libs: [
+ "IBinderRustNdkInteropTest-ndk_platform",
+ "libbinder_ndk_rust_interop",
+ ],
+ test_suites: ["general-tests"],
+ require_root: true,
+
+ // rustBinderTestService uses a custom config
+ auto_gen_config: true,
+}
+
+aidl_interface {
+ name: "IBinderRustNdkInteropTest",
+ unstable: true,
+ srcs: [
+ "IBinderRustNdkInteropTest.aidl",
+ "IBinderRustNdkInteropTestOther.aidl",
+ ],
+ backend: {
+ ndk: {
+ enabled: true,
+ },
+ rust: {
+ enabled: true,
+ },
+ },
+}
+
+rust_ffi_static {
+ name: "libbinder_ndk_rust_interop",
+ crate_name: "binder_ndk_rust_interop",
+ srcs: [
+ "ndk_rust_interop.rs",
+ ],
+ rustlibs: [
+ "libbinder_rs",
+ "IBinderRustNdkInteropTest-rust",
+ ],
+}
diff --git a/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl b/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl
new file mode 100644
index 0000000..7f5e837
--- /dev/null
+++ b/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2020 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 IBinderRustNdkInteropTest {
+ @utf8InCpp String echo(@utf8InCpp String str);
+}
diff --git a/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl b/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl
new file mode 100644
index 0000000..82a0323
--- /dev/null
+++ b/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2020 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 IBinderRustNdkInteropTestOther {
+ @utf8InCpp String echo(@utf8InCpp String str);
+}
diff --git a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp b/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
new file mode 100644
index 0000000..59ca6ed
--- /dev/null
+++ b/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+#include <aidl/BnBinderRustNdkInteropTest.h>
+#include <aidl/IBinderRustNdkInteropTest.h>
+#include <android/binder_ibinder.h>
+#include <android/binder_manager.h>
+#include <android/binder_process.h>
+#include <binder/Status.h>
+#include <gtest/gtest.h>
+
+using namespace android;
+using ::ndk::ScopedAStatus;
+using ::ndk::SharedRefBase;
+using ::ndk::SpAIBinder;
+
+static const char* kNdkServerName = "NdkServer-BinderRustNdkInteropTest";
+static const char* kRustServerName = "RustServer-BinderRustNdkInteropTest";
+
+extern "C" {
+int rust_call_ndk(const char* service_name);
+int rust_start_service(const char* service_name);
+}
+
+class NdkServer : public aidl::BnBinderRustNdkInteropTest {
+ ScopedAStatus echo(const std::string& in, std::string* out) override {
+ *out = in;
+ return ScopedAStatus::ok();
+ }
+};
+
+TEST(RustNdkInterop, RustCanCallNdk) {
+ ASSERT_EQ(STATUS_OK, rust_call_ndk(kNdkServerName));
+}
+
+TEST(RustNdkInterop, NdkCanCallRust) {
+ ASSERT_EQ(STATUS_OK, rust_start_service(kRustServerName));
+
+ SpAIBinder binder = SpAIBinder(AServiceManager_checkService(kRustServerName));
+ ASSERT_NE(nullptr, binder.get());
+ EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get()));
+
+ auto interface = aidl::IBinderRustNdkInteropTest::fromBinder(binder);
+ // TODO(b/167723746): this test requires that fromBinder allow association
+ // with an already associated local binder by treating it as remote.
+ EXPECT_EQ(interface, nullptr);
+
+ // std::string in("testing");
+ // std::string out;
+ // EXPECT_TRUE(interface->echo(in, &out).isOk());
+ // EXPECT_EQ(in, out);
+}
+
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+
+ // so we can host a client and service concurrently
+ ABinderProcess_setThreadPoolMaxThreadCount(1);
+ ABinderProcess_startThreadPool();
+
+ std::shared_ptr<NdkServer> ndkServer = SharedRefBase::make<NdkServer>();
+ EXPECT_EQ(STATUS_OK, AServiceManager_addService(ndkServer->asBinder().get(), kNdkServerName));
+
+ return RUN_ALL_TESTS();
+}
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index 953d328..bb8c492 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -173,6 +173,30 @@
}
}
+/// Trivial testing binder interface
+pub trait ITestSameDescriptor: Interface {}
+
+declare_binder_interface! {
+ ITestSameDescriptor["android.os.ITest"] {
+ native: BnTestSameDescriptor(on_transact_same_descriptor),
+ proxy: BpTestSameDescriptor,
+ }
+}
+
+fn on_transact_same_descriptor(
+ _service: &dyn ITestSameDescriptor,
+ _code: TransactionCode,
+ _data: &Parcel,
+ _reply: &mut Parcel,
+) -> binder::Result<()> {
+ Ok(())
+}
+
+impl ITestSameDescriptor for BpTestSameDescriptor {}
+
+impl ITestSameDescriptor for Binder<BnTestSameDescriptor> {}
+
+
#[cfg(test)]
mod tests {
use selinux_bindgen as selinux_sys;
@@ -185,9 +209,9 @@
use std::thread;
use std::time::Duration;
- use binder::{DeathRecipient, FromIBinder, IBinder, SpIBinder, StatusCode};
+ use binder::{Binder, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, StatusCode};
- use super::{ITest, RUST_SERVICE_BINARY};
+ use super::{BnTest, ITest, ITestSameDescriptor, RUST_SERVICE_BINARY, TestService};
pub struct ScopedServiceProcess(Child);
@@ -435,4 +459,40 @@
assert_eq!(extension.test().unwrap(), extension_name);
}
}
+
+ /// Test re-associating a local binder object with a different class.
+ ///
+ /// This is needed because different binder service (e.g. NDK vs Rust)
+ /// implementations are incompatible and must not be interchanged. A local
+ /// service with the same descriptor string but a different class pointer
+ /// may have been created by an NDK service and is therefore incompatible
+ /// with the Rust service implementation. It must be treated as remote and
+ /// all API calls parceled and sent through transactions.
+ ///
+ /// Further tests of this behavior with the C NDK and Rust API are in
+ /// rust_ndk_interop.rs
+ #[test]
+ fn associate_existing_class() {
+ let service = Binder::new(BnTest(Box::new(TestService {
+ s: "testing_service".to_string(),
+ })));
+
+ // This should succeed although we will have to treat the service as
+ // remote.
+ let _interface: Box<dyn ITestSameDescriptor> = FromIBinder::try_from(service.as_binder())
+ .expect("Could not re-interpret service as the ITestSameDescriptor interface");
+ }
+
+ /// Test that we can round-trip a rust service through a generic IBinder
+ #[test]
+ fn reassociate_rust_binder() {
+ let service_name = "testing_service";
+ let service_ibinder = BnTest::new_binder(TestService { s: service_name.to_string() })
+ .as_binder();
+
+ let service: Box<dyn ITest> = service_ibinder.into_interface()
+ .expect("Could not reassociate the generic ibinder");
+
+ assert_eq!(service.test().unwrap(), service_name);
+ }
}
diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs
new file mode 100644
index 0000000..70a6dc0
--- /dev/null
+++ b/libs/binder/rust/tests/ndk_rust_interop.rs
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+//! Rust Binder NDK interop tests
+
+use std::ffi::CStr;
+use std::os::raw::{c_char, c_int};
+use ::IBinderRustNdkInteropTest::binder::{self, Interface, StatusCode};
+use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTest::{
+ BnBinderRustNdkInteropTest, IBinderRustNdkInteropTest,
+};
+use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::{
+ IBinderRustNdkInteropTestOther,
+};
+
+/// Look up the provided AIDL service and call its echo method.
+///
+/// # Safety
+///
+/// service_name must be a valid, non-null C-style string (null-terminated).
+#[no_mangle]
+pub unsafe extern "C" fn rust_call_ndk(service_name: *const c_char) -> c_int {
+ let service_name = CStr::from_ptr(service_name).to_str().unwrap();
+
+ // The Rust class descriptor pointer will not match the NDK one, but the
+ // descriptor strings match so this needs to still associate.
+ let service: Box<dyn IBinderRustNdkInteropTest> = match binder::get_interface(service_name) {
+ Err(e) => {
+ eprintln!("Could not find Ndk service {}: {:?}", service_name, e);
+ return StatusCode::NAME_NOT_FOUND as c_int;
+ }
+ Ok(service) => service,
+ };
+
+ match service.echo("testing") {
+ Ok(s) => if s != "testing" {
+ return StatusCode::BAD_VALUE as c_int;
+ },
+ Err(e) => return e.into(),
+ }
+
+ // Try using the binder service through the wrong interface type
+ let wrong_service: Result<Box<dyn IBinderRustNdkInteropTestOther>, StatusCode> =
+ binder::get_interface(service_name);
+ match wrong_service {
+ Err(e) if e == StatusCode::BAD_TYPE => {}
+ Err(e) => {
+ eprintln!("Trying to use a service via the wrong interface errored with unexpected error {:?}", e);
+ return e as c_int;
+ }
+ Ok(_) => {
+ eprintln!("We should not be allowed to use a service via the wrong interface");
+ return StatusCode::BAD_TYPE as c_int;
+ }
+ }
+
+ StatusCode::OK as c_int
+}
+
+struct Service;
+
+impl Interface for Service {}
+
+impl IBinderRustNdkInteropTest for Service {
+ fn echo(&self, s: &str) -> binder::Result<String> {
+ Ok(s.to_string())
+ }
+}
+
+/// Start the interop Echo test service with the given service name.
+///
+/// # Safety
+///
+/// service_name must be a valid, non-null C-style string (null-terminated).
+#[no_mangle]
+pub unsafe extern "C" fn rust_start_service(service_name: *const c_char) -> c_int {
+ let service_name = CStr::from_ptr(service_name).to_str().unwrap();
+ let service = BnBinderRustNdkInteropTest::new_binder(Service);
+ match binder::add_service(&service_name, service.as_binder()) {
+ Ok(_) => StatusCode::OK as c_int,
+ Err(e) => e as c_int,
+ }
+}