Revert "libbinder_rs: Treat previously associated Binder as remote"
This reverts commit 29422bf9421560bf283c063ad13a82fa30d9a141.
Reason for revert: b/174694961
Change-Id: I3043d1c9b7b317c9bf9a0ebeedf0ef1e16025827
diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index 4375818..1cfb560 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -56,9 +56,6 @@
},
{
"name": "rustBinderTest"
- },
- {
- "name": "binderRustNdkInteropTest"
}
]
}
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index b558f27..037ee95 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -21,8 +21,7 @@
use crate::proxy::{DeathRecipient, SpIBinder};
use crate::sys;
-use std::ffi::{c_void, CStr, CString};
-use std::os::raw::c_char;
+use std::ffi::{c_void, CString};
use std::os::unix::io::AsRawFd;
use std::ptr;
@@ -206,22 +205,6 @@
pub(crate) unsafe fn from_ptr(ptr: *const sys::AIBinder_Class) -> InterfaceClass {
InterfaceClass(ptr)
}
-
- /// Get the interface descriptor string of this class.
- pub fn get_descriptor(&self) -> String {
- unsafe {
- // SAFETY: The descriptor returned by AIBinder_Class_getDescriptor
- // is always a two-byte null terminated sequence of u16s. Thus, we
- // can continue reading from the pointer until we hit a null value,
- // and this pointer can be a valid slice if the slice length is <=
- // the number of u16 elements before the null terminator.
-
- let raw_descriptor: *const c_char = sys::AIBinder_Class_getDescriptor(self.0);
- CStr::from_ptr(raw_descriptor).to_str()
- .expect("Expected valid UTF-8 string from AIBinder_Class_getDescriptor")
- .into()
- }
- }
}
impl From<InterfaceClass> for *const sys::AIBinder_Class {
@@ -524,7 +507,12 @@
}
fn from_binder(mut binder: $crate::SpIBinder) -> $crate::Result<Self> {
- Ok(Self { binder, $($fname: $finit),* })
+ use $crate::AssociateClass;
+ if binder.associate_class(<$native as $crate::Remotable>::get_class()) {
+ Ok(Self { binder, $($fname: $finit),* })
+ } else {
+ Err($crate::StatusCode::BAD_TYPE)
+ }
}
}
@@ -579,33 +567,16 @@
impl $crate::FromIBinder for dyn $interface {
fn try_from(mut ibinder: $crate::SpIBinder) -> $crate::Result<Box<dyn $interface>> {
use $crate::AssociateClass;
-
- let existing_class = ibinder.get_class();
- if let Some(class) = existing_class {
- if class != <$native as $crate::Remotable>::get_class() &&
- class.get_descriptor() == <$native as $crate::Remotable>::get_descriptor()
- {
- // The binder object's descriptor string matches what we
- // expect. We still need to treat this local or already
- // associated object as remote, because we can't cast it
- // into a Rust service object without a matching class
- // pointer.
- return Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?));
- }
- } else if ibinder.associate_class(<$native as $crate::Remotable>::get_class()) {
- let service: $crate::Result<$crate::Binder<$native>> =
- std::convert::TryFrom::try_from(ibinder.clone());
- if let Ok(service) = service {
- // We were able to associate with our expected class and
- // the service is local.
- return Ok(Box::new(service));
- } else {
- // Service is remote
- return Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?));
- }
+ if !ibinder.associate_class(<$native as $crate::Remotable>::get_class()) {
+ return Err($crate::StatusCode::BAD_TYPE.into());
}
- Err($crate::StatusCode::BAD_TYPE.into())
+ let service: $crate::Result<$crate::Binder<$native>> = std::convert::TryFrom::try_from(ibinder.clone());
+ if let Ok(service) = service {
+ Ok(Box::new(service))
+ } else {
+ Ok(Box::new(<$proxy as $crate::Proxy>::from_binder(ibinder)?))
+ }
}
}
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 485bb42..5002fc6 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -91,7 +91,7 @@
/// Return the interface class of this binder object, if associated with
/// one.
- pub fn get_class(&mut self) -> Option<InterfaceClass> {
+ pub(crate) fn get_class(&mut self) -> Option<InterfaceClass> {
unsafe {
// Safety: `SpIBinder` guarantees that it always contains a valid
// `AIBinder` pointer. `AIBinder_getClass` returns either a null
diff --git a/libs/binder/rust/tests/Android.bp b/libs/binder/rust/tests/Android.bp
index 5ae9c53..3db40ba 100644
--- a/libs/binder/rust/tests/Android.bp
+++ b/libs/binder/rust/tests/Android.bp
@@ -30,52 +30,3 @@
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
deleted file mode 100644
index 7f5e837..0000000
--- a/libs/binder/rust/tests/IBinderRustNdkInteropTest.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * 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
deleted file mode 100644
index 82a0323..0000000
--- a/libs/binder/rust/tests/IBinderRustNdkInteropTestOther.aidl
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * 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
deleted file mode 100644
index 59ca6ed..0000000
--- a/libs/binder/rust/tests/binderRustNdkInteropTest.cpp
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * 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 497361a..953d328 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -173,30 +173,6 @@
}
}
-/// 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;
@@ -209,9 +185,9 @@
use std::thread;
use std::time::Duration;
- use binder::{Binder, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, StatusCode};
+ use binder::{DeathRecipient, FromIBinder, IBinder, SpIBinder, StatusCode};
- use super::{BnTest, ITest, ITestSameDescriptor, RUST_SERVICE_BINARY, TestService};
+ use super::{ITest, RUST_SERVICE_BINARY};
pub struct ScopedServiceProcess(Child);
@@ -459,27 +435,4 @@
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");
- }
}
diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs
deleted file mode 100644
index 70a6dc0..0000000
--- a/libs/binder/rust/tests/ndk_rust_interop.rs
+++ /dev/null
@@ -1,96 +0,0 @@
-/*
- * 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,
- }
-}