From a757ac848d2a87e568a38e174e4d9548dd87a3ed Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Fri, 9 Feb 2024 23:57:21 -0500 Subject: [PATCH 1/8] Holy cow -- the custom (unstructured) parcelable worked in one shot... =^) --- .gitignore | 4 +- Cargo.lock | 22 +- Cargo.toml | 3 +- binder/src/binder.rs | 150 +++-- binder/src/error.rs | 331 ++++++--- binder/src/lib.rs | 9 +- binder/src/native.rs | 220 +++--- binder/src/parcel.rs | 171 +++-- binder/src/parcel/file_descriptor.rs | 78 +-- binder/src/parcel/parcelable.rs | 626 ++++++++++++++---- binder/src/parcel/parcelable_holder.rs | 31 +- binder/src/proxy.rs | 470 +++++++------ binder/src/state.rs | 63 +- example/src/IRemoteService.rs | 77 ++- unstructured_parcelable_example/Cargo.toml | 13 + .../aidl/IMySimpleParcelableService.aidl | 8 + .../aidl/MySimpleParcelable.aidl | 3 + .../src/IMySimpleParcelableService.rs | 142 ++++ unstructured_parcelable_example/src/cli.rs | 33 + unstructured_parcelable_example/src/client.rs | 13 + unstructured_parcelable_example/src/main.rs | 10 + .../src/my_simple_parcelable.rs | 49 ++ unstructured_parcelable_example/src/server.rs | 33 + 23 files changed, 1725 insertions(+), 834 deletions(-) create mode 100644 unstructured_parcelable_example/Cargo.toml create mode 100644 unstructured_parcelable_example/aidl/IMySimpleParcelableService.aidl create mode 100644 unstructured_parcelable_example/aidl/MySimpleParcelable.aidl create mode 100644 unstructured_parcelable_example/src/IMySimpleParcelableService.rs create mode 100644 unstructured_parcelable_example/src/cli.rs create mode 100644 unstructured_parcelable_example/src/client.rs create mode 100644 unstructured_parcelable_example/src/main.rs create mode 100644 unstructured_parcelable_example/src/my_simple_parcelable.rs create mode 100644 unstructured_parcelable_example/src/server.rs diff --git a/.gitignore b/.gitignore index 6ec4261..33292ff 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ target -.DS_Store \ No newline at end of file +.DS_Store + +.idea/ \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 65f3ca8..8aed0d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,15 +38,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "binder-tests" -version = "0.1.0" -dependencies = [ - "binder_ndk", - "binder_tokio", - "tokio", -] - [[package]] name = "binder_ndk" version = "0.2.0" @@ -594,6 +585,19 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" +[[package]] +name = "unstructured-parcelable-binder-example" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "binder_ndk", + "binder_tokio", + "clap", + "lazy_static", + "tokio", +] + [[package]] name = "version_check" version = "0.9.4" diff --git a/Cargo.toml b/Cargo.toml index ac91a20..8081d2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,8 @@ members = [ "binder_tokio", "binder_ndk_sys", "example", - "tests", + "unstructured_parcelable_example" +# "tests", ] exclude = ["target/*"] diff --git a/binder/src/binder.rs b/binder/src/binder.rs index 976f54d..e34d31e 100644 --- a/binder/src/binder.rs +++ b/binder/src/binder.rs @@ -21,16 +21,17 @@ use crate::parcel::{BorrowedParcel, Parcel}; use crate::proxy::{DeathRecipient, SpIBinder, WpIBinder}; use crate::sys; +use downcast_rs::{impl_downcast, DowncastSync}; use std::borrow::Borrow; use std::cmp::Ordering; use std::convert::TryFrom; use std::ffi::{c_void, CStr, CString}; use std::fmt; -use std::fs::File; +use std::io::Write; use std::marker::PhantomData; use std::ops::Deref; +use std::os::fd::AsRawFd; use std::os::raw::c_char; -use std::os::unix::io::AsRawFd; use std::ptr; /// Binder action to perform. @@ -51,7 +52,7 @@ pub type TransactionFlags = u32; /// interfaces) must implement this trait. /// /// This is equivalent `IInterface` in C++. -pub trait Interface: Send + Sync { +pub trait Interface: Send + Sync + DowncastSync { /// Convert this binder object into a generic [`SpIBinder`] reference. fn as_binder(&self) -> SpIBinder { panic!("This object was not a Binder object and cannot be converted into an SpIBinder.") @@ -61,11 +62,13 @@ pub trait Interface: Send + Sync { /// /// This handler is a no-op by default and should be implemented for each /// Binder service struct that wishes to respond to dump transactions. - fn dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + fn dump(&self, _writer: &mut dyn Write, _args: &[&CStr]) -> Result<()> { Ok(()) } } +impl_downcast!(sync Interface); + /// Implemented by sync interfaces to specify what the associated async interface is. /// Generic to handle the fact that async interfaces are generic over a thread pool. /// @@ -97,25 +100,20 @@ where /// Interface stability promise /// -/// An interface can promise to be a stable vendor interface ([`Vintf`]), or -/// makes no stability guarantees ([`Local`]). [`Local`] is +/// An interface can promise to be a stable vendor interface ([`Stability::Vintf`]), +/// or makes no stability guarantees ([`Stability::Local`]). [`Stability::Local`] is /// currently the default stability. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Default)] pub enum Stability { /// Default stability, visible to other modules in the same compilation /// context (e.g. modules on system.img) + #[default] Local, /// A Vendor Interface Object, which promises to be stable Vintf, } -impl Default for Stability { - fn default() -> Self { - Stability::Local - } -} - impl From for i32 { fn from(stability: Stability) -> i32 { use Stability::*; @@ -144,11 +142,11 @@ impl TryFrom for Stability { /// via `Binder::new(object)`. /// /// This is a low-level interface that should normally be automatically -/// generated from AIDL via the [`declare_binder_interface!`] macro. When using -/// the AIDL backend, users need only implement the high-level AIDL-defined +/// generated from AIDL via the [`crate::declare_binder_interface!`] macro. +/// When using the AIDL backend, users need only implement the high-level AIDL-defined /// interface. The AIDL compiler then generates a container struct that wraps /// the user-defined service and implements `Remotable`. -pub trait Remotable: Send + Sync { +pub trait Remotable: Send + Sync + 'static { /// The Binder interface descriptor string. /// /// This string is a unique identifier for a Binder interface, and should be @@ -167,7 +165,7 @@ pub trait Remotable: Send + Sync { /// Handle a request to invoke the dump transaction on this /// object. - fn on_dump(&self, file: &File, args: &[&CStr]) -> Result<()>; + fn on_dump(&self, file: &mut dyn Write, args: &[&CStr]) -> Result<()>; /// Retrieve the class of this remote object. /// @@ -265,7 +263,14 @@ pub trait IBinder { /// Trying to use this function on a local binder will result in an /// INVALID_OPERATION code being returned and nothing happening. /// - /// This link always holds a weak reference to its recipient. + /// This link only holds a weak reference to its recipient. If the + /// `DeathRecipient` is dropped then it will be unlinked. + /// + /// Note that the notifications won't work if you don't first start at least + /// one Binder thread by calling + /// [`ProcessState::start_thread_pool`](crate::ProcessState::start_thread_pool) + /// or + /// [`ProcessState::join_thread_pool`](crate::ProcessState::join_thread_pool). fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()>; /// Remove a previously registered death notification. @@ -295,18 +300,17 @@ impl InterfaceClass { /// Note: the returned pointer will not be constant. Calling this method /// multiple times for the same type will result in distinct class /// pointers. A static getter for this value is implemented in - /// [`declare_binder_interface!`]. + /// [`crate::declare_binder_interface!`]. pub fn new() -> InterfaceClass { let descriptor = CString::new(I::get_descriptor()).unwrap(); + // Safety: `AIBinder_Class_define` expects a valid C string, and three + // valid callback functions, all non-null pointers. The C string is + // copied and need not be valid for longer than the call, so we can drop + // it after the call. We can safely assign null to the onDump and + // handleShellCommand callbacks as long as the class pointer was + // non-null. Rust None for a Option is guaranteed to be a NULL + // pointer. Rust retains ownership of the pointer after it is defined. let ptr = unsafe { - // Safety: `AIBinder_Class_define` expects a valid C string, and - // three valid callback functions, all non-null pointers. The C - // string is copied and need not be valid for longer than the call, - // so we can drop it after the call. We can safely assign null to - // the onDump and handleShellCommand callbacks as long as the class - // pointer was non-null. Rust None for a Option is guaranteed to - // be a NULL pointer. Rust retains ownership of the pointer after it - // is defined. let class = sys::AIBinder_Class_define( descriptor.as_ptr(), Some(I::on_create), @@ -336,13 +340,12 @@ impl InterfaceClass { /// Get the interface descriptor string of this class. pub fn get_descriptor(&self) -> String { + // 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. 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() @@ -436,7 +439,7 @@ impl Ord for Strong { impl PartialOrd for Strong { fn partial_cmp(&self, other: &Self) -> Option { - self.0.as_binder().partial_cmp(&other.0.as_binder()) + Some(self.cmp(other)) } } @@ -483,7 +486,7 @@ impl Ord for Weak { impl PartialOrd for Weak { fn partial_cmp(&self, other: &Self) -> Option { - self.weak_binder.partial_cmp(&other.weak_binder) + Some(self.cmp(other)) } } @@ -540,17 +543,15 @@ macro_rules! binder_fn_get_class { static CLASS_INIT: std::sync::Once = std::sync::Once::new(); static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None; + // Safety: This assignment is guarded by the `CLASS_INIT` `Once` + // variable, and therefore is thread-safe, as it can only occur + // once. CLASS_INIT.call_once(|| unsafe { - // Safety: This assignment is guarded by the `CLASS_INIT` `Once` - // variable, and therefore is thread-safe, as it can only occur - // once. CLASS = Some($constructor); }); - unsafe { - // Safety: The `CLASS` variable can only be mutated once, above, - // and is subsequently safe to read from any thread. - CLASS.unwrap() - } + // Safety: The `CLASS` variable can only be mutated once, above, and + // is subsequently safe to read from any thread. + unsafe { CLASS.unwrap() } } }; } @@ -662,6 +663,8 @@ pub unsafe trait AsNative { fn as_native_mut(&mut self) -> *mut T; } +// Safety: If V is a valid Android C++ type then we can either use that or a +// null pointer. unsafe impl> AsNative for Option { fn as_native(&self) -> *const T { self.as_ref().map_or(ptr::null(), |v| v.as_native()) @@ -893,6 +896,23 @@ macro_rules! declare_binder_interface { $crate::binder_impl::IBinderInternal::set_requesting_sid(&mut binder, features.set_requesting_sid); $crate::Strong::new(Box::new(binder)) } + + /// Tries to downcast the interface to another type. + /// When receiving this object from a binder call, make sure that the object received is + /// a binder native object and that is of the right type for the Downcast: + /// + /// let binder = received_object.as_binder(); + /// if !binder.is_remote() { + /// let binder_native: Binder = binder.try_into()?; + /// let original_object = binder_native.downcast_binder::(); + /// // Check that returned type is not None before using it + /// } + /// + /// Handle the error cases instead of just calling `unwrap` or `expect` to prevent a + /// malicious caller to mount a Denial of Service attack. + pub fn downcast_binder(&self) -> Option<&T> { + self.0.as_any().downcast_ref::() + } } impl $crate::binder_impl::Remotable for $native { @@ -914,23 +934,23 @@ macro_rules! declare_binder_interface { } } - fn on_dump(&self, file: &std::fs::File, args: &[&std::ffi::CStr]) -> std::result::Result<(), $crate::StatusCode> { - self.0.dump(file, args) + fn on_dump(&self, writer: &mut dyn std::io::Write, args: &[&std::ffi::CStr]) -> std::result::Result<(), $crate::StatusCode> { + self.0.dump(writer, args) } fn get_class() -> $crate::binder_impl::InterfaceClass { static CLASS_INIT: std::sync::Once = std::sync::Once::new(); static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None; + // Safety: This assignment is guarded by the `CLASS_INIT` `Once` + // variable, and therefore is thread-safe, as it can only occur + // once. CLASS_INIT.call_once(|| unsafe { - // Safety: This assignment is guarded by the `CLASS_INIT` `Once` - // variable, and therefore is thread-safe, as it can only occur - // once. CLASS = Some($crate::binder_impl::InterfaceClass::new::<$crate::binder_impl::Binder<$native>>()); }); + // Safety: The `CLASS` variable can only be mutated once, above, + // and is subsequently safe to read from any thread. unsafe { - // Safety: The `CLASS` variable can only be mutated once, above, - // and is subsequently safe to read from any thread. CLASS.unwrap() } } @@ -1004,7 +1024,7 @@ macro_rules! declare_binder_interface { $( // Async interface trait implementations. - impl $crate::FromIBinder for dyn $async_interface

{ + impl $crate::FromIBinder for dyn $async_interface

{ fn try_from(mut ibinder: $crate::SpIBinder) -> std::result::Result<$crate::Strong>, $crate::StatusCode> { use $crate::binder_impl::AssociateClass; @@ -1023,44 +1043,34 @@ macro_rules! declare_binder_interface { } if ibinder.associate_class(<$native as $crate::binder_impl::Remotable>::get_class()) { - let service: std::result::Result<$crate::binder_impl::Binder<$native>, $crate::StatusCode> = - 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. - todo!() - //return Ok($crate::Strong::new(Box::new(service))); - } else { - // Service is remote - return Ok($crate::Strong::new(Box::new(<$proxy as $crate::binder_impl::Proxy>::from_binder(ibinder)?))); - } + return Ok($crate::Strong::new(Box::new(<$proxy as $crate::binder_impl::Proxy>::from_binder(ibinder)?))); } Err($crate::StatusCode::BAD_TYPE.into()) } } - impl $crate::binder_impl::Serialize for dyn $async_interface

+ '_ { + impl $crate::binder_impl::Serialize for dyn $async_interface

+ '_ { fn serialize(&self, parcel: &mut $crate::binder_impl::BorrowedParcel<'_>) -> std::result::Result<(), $crate::StatusCode> { let binder = $crate::Interface::as_binder(self); parcel.write(&binder) } } - impl $crate::binder_impl::SerializeOption for dyn $async_interface

+ '_ { + impl $crate::binder_impl::SerializeOption for dyn $async_interface

+ '_ { fn serialize_option(this: Option<&Self>, parcel: &mut $crate::binder_impl::BorrowedParcel<'_>) -> std::result::Result<(), $crate::StatusCode> { parcel.write(&this.map($crate::Interface::as_binder)) } } - impl std::fmt::Debug for dyn $async_interface

+ '_ { + impl std::fmt::Debug for dyn $async_interface

+ '_ { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.pad(stringify!($async_interface)) } } /// Convert a &dyn $async_interface to Strong - impl std::borrow::ToOwned for dyn $async_interface

{ + impl std::borrow::ToOwned for dyn $async_interface

{ type Owned = $crate::Strong>; fn to_owned(&self) -> Self::Owned { self.as_binder().into_interface() @@ -1068,11 +1078,11 @@ macro_rules! declare_binder_interface { } } - impl $crate::binder_impl::ToAsyncInterface

for dyn $interface { + impl $crate::binder_impl::ToAsyncInterface

for dyn $interface { type Target = dyn $async_interface

; } - impl $crate::binder_impl::ToSyncInterface for dyn $async_interface

{ + impl $crate::binder_impl::ToSyncInterface for dyn $async_interface

{ type Target = dyn $interface; } )? @@ -1127,6 +1137,10 @@ macro_rules! declare_binder_enum { } impl $crate::binder_impl::Deserialize for $enum { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } + fn deserialize(parcel: &$crate::binder_impl::BorrowedParcel<'_>) -> std::result::Result { parcel.read().map(Self) } diff --git a/binder/src/error.rs b/binder/src/error.rs index f6b09ed..eb04cc3 100644 --- a/binder/src/error.rs +++ b/binder/src/error.rs @@ -20,6 +20,7 @@ use crate::sys; use std::error; use std::ffi::{CStr, CString}; use std::fmt::{Debug, Display, Formatter, Result as FmtResult}; +use std::ptr; use std::result; pub use sys::binder_status_t as status_t; @@ -92,7 +93,7 @@ fn parse_exception_code(code: i32) -> ExceptionCode { /// track of and chain binder errors along with service specific errors. /// /// Used in AIDL transactions to represent failed transactions. -pub struct Status(*mut sys::AStatus); +pub struct Status(ptr::NonNull); // Safety: The `AStatus` that the `Status` points to must have an entirely thread-safe API for the // duration of the `Status` object's lifetime. We ensure this by not allowing mutation of a `Status` @@ -111,43 +112,37 @@ fn to_cstring>(message: T) -> Option { impl Status { /// Create a status object representing a successful transaction. pub fn ok() -> Self { - let ptr = unsafe { - // Safety: `AStatus_newOk` always returns a new, heap allocated - // pointer to an `ASTatus` object, so we know this pointer will be - // valid. - // - // Rust takes ownership of the returned pointer. - sys::AStatus_newOk() - }; - Self(ptr) + // Safety: `AStatus_newOk` always returns a new, heap allocated + // pointer to an `ASTatus` object, so we know this pointer will be + // valid. + // + // Rust takes ownership of the returned pointer. + let ptr = unsafe { sys::AStatus_newOk() }; + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Create a status object from a service specific error pub fn new_service_specific_error(err: i32, message: Option<&CStr>) -> Status { let ptr = if let Some(message) = message { - unsafe { - // Safety: Any i32 is a valid service specific error for the - // error code parameter. We construct a valid, null-terminated - // `CString` from the message, which must be a valid C-style - // string to pass as the message. This function always returns a - // new, heap allocated pointer to an `AStatus` object, so we - // know the returned pointer will be valid. - // - // Rust takes ownership of the returned pointer. - sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr()) - } + // Safety: Any i32 is a valid service specific error for the + // error code parameter. We construct a valid, null-terminated + // `CString` from the message, which must be a valid C-style + // string to pass as the message. This function always returns a + // new, heap allocated pointer to an `AStatus` object, so we + // know the returned pointer will be valid. + // + // Rust takes ownership of the returned pointer. + unsafe { sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr()) } } else { - unsafe { - // Safety: Any i32 is a valid service specific error for the - // error code parameter. This function always returns a new, - // heap allocated pointer to an `AStatus` object, so we know the - // returned pointer will be valid. - // - // Rust takes ownership of the returned pointer. - sys::AStatus_fromServiceSpecificError(err) - } + // Safety: Any i32 is a valid service specific error for the + // error code parameter. This function always returns a new, + // heap allocated pointer to an `AStatus` object, so we know the + // returned pointer will be valid. + // + // Rust takes ownership of the returned pointer. + unsafe { sys::AStatus_fromServiceSpecificError(err) } }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Creates a status object from a service specific error. @@ -158,10 +153,12 @@ impl Status { /// Create a status object from an exception code pub fn new_exception(exception: ExceptionCode, message: Option<&CStr>) -> Status { if let Some(message) = message { + // Safety: the C string pointer is valid and not retained by the + // function. let ptr = unsafe { sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr()) }; - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } else { exception.into() } @@ -181,42 +178,36 @@ impl Status { /// /// This constructor is safe iff `ptr` is a valid pointer to an `AStatus`. pub(crate) unsafe fn from_ptr(ptr: *mut sys::AStatus) -> Self { - Self(ptr) + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } /// Returns `true` if this status represents a successful transaction. pub fn is_ok(&self) -> bool { - unsafe { - // Safety: `Status` always contains a valid `AStatus` pointer, so we - // are always passing a valid pointer to `AStatus_isOk` here. - sys::AStatus_isOk(self.as_native()) - } + // Safety: `Status` always contains a valid `AStatus` pointer, so we + // are always passing a valid pointer to `AStatus_isOk` here. + unsafe { sys::AStatus_isOk(self.as_native()) } } /// Returns a description of the status. pub fn get_description(&self) -> String { - let description_ptr = unsafe { - // Safety: `Status` always contains a valid `AStatus` pointer, so we - // are always passing a valid pointer to `AStatus_getDescription` - // here. - // - // `AStatus_getDescription` always returns a valid pointer to a null - // terminated C string. Rust is responsible for freeing this pointer - // via `AStatus_deleteDescription`. - sys::AStatus_getDescription(self.as_native()) - }; - let description = unsafe { - // Safety: `AStatus_getDescription` always returns a valid C string, - // which can be safely converted to a `CStr`. - CStr::from_ptr(description_ptr) - }; + // Safety: `Status` always contains a valid `AStatus` pointer, so we + // are always passing a valid pointer to `AStatus_getDescription` + // here. + // + // `AStatus_getDescription` always returns a valid pointer to a null + // terminated C string. Rust is responsible for freeing this pointer + // via `AStatus_deleteDescription`. + let description_ptr = unsafe { sys::AStatus_getDescription(self.as_native()) }; + // Safety: `AStatus_getDescription` always returns a valid C string, + // which can be safely converted to a `CStr`. + let description = unsafe { CStr::from_ptr(description_ptr) }; let description = description.to_string_lossy().to_string(); + // Safety: `description_ptr` was returned from + // `AStatus_getDescription` above, and must be freed via + // `AStatus_deleteDescription`. We must not access the pointer after + // this call, so we copy it into an owned string above and return + // that string. unsafe { - // Safety: `description_ptr` was returned from - // `AStatus_getDescription` above, and must be freed via - // `AStatus_deleteDescription`. We must not access the pointer after - // this call, so we copy it into an owned string above and return - // that string. sys::AStatus_deleteDescription(description_ptr); } description @@ -224,12 +215,10 @@ impl Status { /// Returns the exception code of the status. pub fn exception_code(&self) -> ExceptionCode { - let code = unsafe { - // Safety: `Status` always contains a valid `AStatus` pointer, so we - // are always passing a valid pointer to `AStatus_getExceptionCode` - // here. - sys::AStatus_getExceptionCode(self.as_native()) - }; + // Safety: `Status` always contains a valid `AStatus` pointer, so we + // are always passing a valid pointer to `AStatus_getExceptionCode` + // here. + let code = unsafe { sys::AStatus_getExceptionCode(self.as_native()) }; parse_exception_code(code) } @@ -240,11 +229,9 @@ impl Status { /// exception or a service specific error. To find out if this transaction /// as a whole is okay, use [`is_ok`](Self::is_ok) instead. pub fn transaction_error(&self) -> StatusCode { - let code = unsafe { - // Safety: `Status` always contains a valid `AStatus` pointer, so we - // are always passing a valid pointer to `AStatus_getStatus` here. - sys::AStatus_getStatus(self.as_native()) - }; + // Safety: `Status` always contains a valid `AStatus` pointer, so we + // are always passing a valid pointer to `AStatus_getStatus` here. + let code = unsafe { sys::AStatus_getStatus(self.as_native()) }; parse_status_code(code) } @@ -257,12 +244,10 @@ impl Status { /// find out if this transaction as a whole is okay, use /// [`is_ok`](Self::is_ok) instead. pub fn service_specific_error(&self) -> i32 { - unsafe { - // Safety: `Status` always contains a valid `AStatus` pointer, so we - // are always passing a valid pointer to - // `AStatus_getServiceSpecificError` here. - sys::AStatus_getServiceSpecificError(self.as_native()) - } + // Safety: `Status` always contains a valid `AStatus` pointer, so we + // are always passing a valid pointer to + // `AStatus_getServiceSpecificError` here. + unsafe { sys::AStatus_getServiceSpecificError(self.as_native()) } } /// Calls `op` if the status was ok, otherwise returns an `Err` value of @@ -320,25 +305,21 @@ impl From for Status { impl From for Status { fn from(status: status_t) -> Status { - let ptr = unsafe { - // Safety: `AStatus_fromStatus` expects any `status_t` integer, so - // this is a safe FFI call. Unknown values will be coerced into - // UNKNOWN_ERROR. - sys::AStatus_fromStatus(status) - }; - Self(ptr) + // Safety: `AStatus_fromStatus` expects any `status_t` integer, so + // this is a safe FFI call. Unknown values will be coerced into + // UNKNOWN_ERROR. + let ptr = unsafe { sys::AStatus_fromStatus(status) }; + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } } impl From for Status { fn from(code: ExceptionCode) -> Status { - let ptr = unsafe { - // Safety: `AStatus_fromExceptionCode` expects any - // `binder_exception_t` (i32) integer, so this is a safe FFI call. - // Unknown values will be coerced into EX_TRANSACTION_FAILED. - sys::AStatus_fromExceptionCode(code as i32) - }; - Self(ptr) + // Safety: `AStatus_fromExceptionCode` expects any + // `binder_exception_t` (i32) integer, so this is a safe FFI call. + // Unknown values will be coerced into EX_TRANSACTION_FAILED. + let ptr = unsafe { sys::AStatus_fromExceptionCode(code as i32) }; + Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer")) } } @@ -362,30 +343,118 @@ impl From for status_t { impl Drop for Status { fn drop(&mut self) { + // Safety: `Status` manages the lifetime of its inner `AStatus` + // pointee, so we need to delete it here. We know that the pointer + // will be valid here since `Status` always contains a valid pointer + // while it is alive. unsafe { - // Safety: `Status` manages the lifetime of its inner `AStatus` - // pointee, so we need to delete it here. We know that the pointer - // will be valid here since `Status` always contains a valid pointer - // while it is alive. - sys::AStatus_delete(self.0); + sys::AStatus_delete(self.0.as_mut()); } } } -/// # Safety -/// -/// `Status` always contains a valid pointer to an `AStatus` object, so we can -/// trivially convert it to a correctly-typed raw pointer. +/// Safety: `Status` always contains a valid pointer to an `AStatus` object, so +/// we can trivially convert it to a correctly-typed raw pointer. /// /// Care must be taken that the returned pointer is only dereferenced while the /// `Status` object is still alive. unsafe impl AsNative for Status { fn as_native(&self) -> *const sys::AStatus { - self.0 + self.0.as_ptr() } fn as_native_mut(&mut self) -> *mut sys::AStatus { - self.0 + // Safety: The pointer will be valid here since `Status` always contains + // a valid and initialized pointer while it is alive. + unsafe { self.0.as_mut() } + } +} + +/// A conversion from `std::result::Result` to `binder::Result`. If this type is `Ok(T)`, +/// it's returned as is. If this type is `Err(E)`, `E` is converted into `Status` which can be +/// either a general binder exception, or a service-specific exception. +/// +/// # Examples +/// +/// ``` +/// // std::io::Error is formatted as the exception's message +/// fn file_exists(name: &str) -> binder::Result { +/// std::fs::metadata(name) +/// .or_service_specific_exception(NOT_FOUND)? +/// } +/// +/// // A custom function is used to create the exception's message +/// fn file_exists(name: &str) -> binder::Result { +/// std::fs::metadata(name) +/// .or_service_specific_exception_with(NOT_FOUND, +/// |e| format!("file {} not found: {:?}", name, e))? +/// } +/// +/// // anyhow::Error is formatted as the exception's message +/// use anyhow::{Context, Result}; +/// fn file_exists(name: &str) -> binder::Result { +/// std::fs::metadata(name) +/// .context("file {} not found") +/// .or_service_specific_exception(NOT_FOUND)? +/// } +/// +/// // General binder exceptions can be created similarly +/// fn file_exists(name: &str) -> binder::Result { +/// std::fs::metadata(name) +/// .or_binder_exception(ExceptionCode::ILLEGAL_ARGUMENT)? +/// } +/// ``` +pub trait IntoBinderResult { + /// Converts the embedded error into a general binder exception of code `exception`. The + /// message of the exception is set by formatting the error for debugging. + fn or_binder_exception(self, exception: ExceptionCode) -> result::Result; + + /// Converts the embedded error into a general binder exception of code `exception`. The + /// message of the exception is set by lazily evaluating the `op` function. + fn or_binder_exception_with, O: FnOnce(E) -> M>( + self, + exception: ExceptionCode, + op: O, + ) -> result::Result; + + /// Converts the embedded error into a service-specific binder exception. `error_code` is used + /// to distinguish different service-specific binder exceptions. The message of the exception + /// is set by formatting the error for debugging. + fn or_service_specific_exception(self, error_code: i32) -> result::Result; + + /// Converts the embedded error into a service-specific binder exception. `error_code` is used + /// to distinguish different service-specific binder exceptions. The message of the exception + /// is set by lazily evaluating the `op` function. + fn or_service_specific_exception_with, O: FnOnce(E) -> M>( + self, + error_code: i32, + op: O, + ) -> result::Result; +} + +impl IntoBinderResult for result::Result { + fn or_binder_exception(self, exception: ExceptionCode) -> result::Result { + self.or_binder_exception_with(exception, |e| format!("{:?}", e)) + } + + fn or_binder_exception_with, O: FnOnce(E) -> M>( + self, + exception: ExceptionCode, + op: O, + ) -> result::Result { + self.map_err(|e| Status::new_exception_str(exception, Some(op(e)))) + } + + fn or_service_specific_exception(self, error_code: i32) -> result::Result { + self.or_service_specific_exception_with(error_code, |e| format!("{:?}", e)) + } + + fn or_service_specific_exception_with, O: FnOnce(E) -> M>( + self, + error_code: i32, + op: O, + ) -> result::Result { + self.map_err(|e| Status::new_service_specific_error_str(error_code, Some(op(e)))) } } @@ -425,4 +494,66 @@ mod tests { assert_eq!(status.service_specific_error(), 0); assert_eq!(status.get_description(), "Status(-5, EX_ILLEGAL_STATE): ''".to_string()); } + + #[test] + fn convert_to_service_specific_exception() { + let res: std::result::Result<(), Status> = + Err("message").or_service_specific_exception(-42); + + assert!(res.is_err()); + let status = res.unwrap_err(); + assert_eq!(status.exception_code(), ExceptionCode::SERVICE_SPECIFIC); + assert_eq!(status.service_specific_error(), -42); + assert_eq!( + status.get_description(), + "Status(-8, EX_SERVICE_SPECIFIC): '-42: \"message\"'".to_string() + ); + } + + #[test] + fn convert_to_service_specific_exception_with() { + let res: std::result::Result<(), Status> = Err("message") + .or_service_specific_exception_with(-42, |e| format!("outer message: {:?}", e)); + + assert!(res.is_err()); + let status = res.unwrap_err(); + assert_eq!(status.exception_code(), ExceptionCode::SERVICE_SPECIFIC); + assert_eq!(status.service_specific_error(), -42); + assert_eq!( + status.get_description(), + "Status(-8, EX_SERVICE_SPECIFIC): '-42: outer message: \"message\"'".to_string() + ); + } + + #[test] + fn convert_to_binder_exception() { + let res: std::result::Result<(), Status> = + Err("message").or_binder_exception(ExceptionCode::ILLEGAL_STATE); + + assert!(res.is_err()); + let status = res.unwrap_err(); + assert_eq!(status.exception_code(), ExceptionCode::ILLEGAL_STATE); + assert_eq!(status.service_specific_error(), 0); + assert_eq!( + status.get_description(), + "Status(-5, EX_ILLEGAL_STATE): '\"message\"'".to_string() + ); + } + + #[test] + fn convert_to_binder_exception_with() { + let res: std::result::Result<(), Status> = Err("message") + .or_binder_exception_with(ExceptionCode::ILLEGAL_STATE, |e| { + format!("outer message: {:?}", e) + }); + + assert!(res.is_err()); + let status = res.unwrap_err(); + assert_eq!(status.exception_code(), ExceptionCode::ILLEGAL_STATE); + assert_eq!(status.service_specific_error(), 0); + assert_eq!( + status.get_description(), + "Status(-5, EX_ILLEGAL_STATE): 'outer message: \"message\"'".to_string() + ); + } } diff --git a/binder/src/lib.rs b/binder/src/lib.rs index 0c8b48f..16049f2 100644 --- a/binder/src/lib.rs +++ b/binder/src/lib.rs @@ -100,13 +100,14 @@ mod error; mod native; mod parcel; mod proxy; +#[cfg(not(target_os = "trusty"))] mod state; use binder_ndk_sys as sys; pub use crate::binder_async::{BinderAsyncPool, BoxFuture}; pub use binder::{BinderFeatures, FromIBinder, IBinder, Interface, Strong, Weak}; -pub use error::{ExceptionCode, Status, StatusCode}; +pub use error::{ExceptionCode, IntoBinderResult, Status, StatusCode}; pub use native::{ add_service, force_lazy_services_persist, is_handling_transaction, register_lazy_service, LazyServiceGuard, @@ -116,6 +117,7 @@ pub use proxy::{ get_declared_instances, get_interface, get_service, is_declared, wait_for_interface, wait_for_service, DeathRecipient, SpIBinder, WpIBinder, }; +#[cfg(not(target_os = "trusty"))] pub use state::{ProcessState, ThreadState}; /// Binder result containing a [`Status`] on error. @@ -134,8 +136,8 @@ pub mod binder_impl { pub use crate::native::Binder; pub use crate::parcel::{ BorrowedParcel, Deserialize, DeserializeArray, DeserializeOption, Parcel, - ParcelableMetadata, Serialize, SerializeArray, SerializeOption, NON_NULL_PARCELABLE_FLAG, - NULL_PARCELABLE_FLAG, + ParcelableMetadata, Serialize, SerializeArray, SerializeOption, UnstructuredParcelable, + NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, }; pub use crate::proxy::{AssociateClass, Proxy}; } @@ -144,6 +146,7 @@ pub mod binder_impl { #[doc(hidden)] pub mod unstable_api { pub use crate::binder::AsNative; + pub use crate::error::status_result; pub use crate::proxy::unstable_api::new_spibinder; pub use crate::sys::AIBinder; pub use crate::sys::AParcel; diff --git a/binder/src/native.rs b/binder/src/native.rs index 6f686fb..8ae010e 100644 --- a/binder/src/native.rs +++ b/binder/src/native.rs @@ -24,12 +24,10 @@ use crate::sys; use std::convert::TryFrom; use std::ffi::{c_void, CStr, CString}; -use std::fs::File; +use std::io::Write; use std::mem::ManuallyDrop; use std::ops::Deref; use std::os::raw::c_char; -use std::os::unix::io::FromRawFd; -use std::slice; use std::sync::Mutex; /// Rust wrapper around Binder remotable objects. @@ -42,7 +40,7 @@ pub struct Binder { rust_object: *mut T, } -/// # Safety +/// Safety: /// /// A `Binder` is a pair of unique owning pointers to two values: /// * a C++ ABBinder which the C++ API guarantees can be passed between threads @@ -54,7 +52,7 @@ pub struct Binder { /// to how `Box` is `Send` if `T` is `Send`. unsafe impl Send for Binder {} -/// # Safety +/// Safety: /// /// A `Binder` is a pair of unique owning pointers to two values: /// * a C++ ABBinder which is thread-safe, i.e. `Send + Sync` @@ -89,15 +87,13 @@ impl Binder { pub fn new_with_stability(rust_object: T, stability: Stability) -> Binder { let class = T::get_class(); let rust_object = Box::into_raw(Box::new(rust_object)); - let ibinder = unsafe { - // Safety: `AIBinder_new` expects a valid class pointer (which we - // initialize via `get_class`), and an arbitrary pointer - // argument. The caller owns the returned `AIBinder` pointer, which - // is a strong reference to a `BBinder`. This reference should be - // decremented via `AIBinder_decStrong` when the reference lifetime - // ends. - sys::AIBinder_new(class.into(), rust_object as *mut c_void) - }; + // Safety: `AIBinder_new` expects a valid class pointer (which we + // initialize via `get_class`), and an arbitrary pointer + // argument. The caller owns the returned `AIBinder` pointer, which + // is a strong reference to a `BBinder`. This reference should be + // decremented via `AIBinder_decStrong` when the reference lifetime + // ends. + let ibinder = unsafe { sys::AIBinder_new(class.into(), rust_object as *mut c_void) }; let mut binder = Binder { ibinder, rust_object }; binder.mark_stability(stability); binder @@ -176,15 +172,14 @@ impl Binder { /// } /// # } pub fn set_extension(&mut self, extension: &mut SpIBinder) -> Result<()> { - let status = unsafe { - // Safety: `AIBinder_setExtension` expects two valid, mutable - // `AIBinder` pointers. We are guaranteed that both `self` and - // `extension` contain valid `AIBinder` pointers, because they - // cannot be initialized without a valid - // pointer. `AIBinder_setExtension` does not take ownership of - // either parameter. - sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut()) - }; + let status = + // Safety: `AIBinder_setExtension` expects two valid, mutable + // `AIBinder` pointers. We are guaranteed that both `self` and + // `extension` contain valid `AIBinder` pointers, because they + // cannot be initialized without a valid + // pointer. `AIBinder_setExtension` does not take ownership of + // either parameter. + unsafe { sys::AIBinder_setExtension(self.as_native_mut(), extension.as_native_mut()) }; status_result(status) } @@ -199,9 +194,9 @@ impl Binder { match stability { Stability::Local => self.mark_local_stability(), Stability::Vintf => { + // Safety: Self always contains a valid `AIBinder` pointer, so + // we can always call this C API safely. unsafe { - // Safety: Self always contains a valid `AIBinder` pointer, so - // we can always call this C API safely. sys::AIBinder_markVintfStability(self.as_native_mut()); } } @@ -209,23 +204,23 @@ impl Binder { } /// Mark this binder object with local stability, which is vendor if we are - /// building for the VNDK and system otherwise. - #[cfg(any(vendor_ndk, android_vndk))] + /// building for android_vendor and system otherwise. + #[cfg(android_vendor)] fn mark_local_stability(&mut self) { + // Safety: Self always contains a valid `AIBinder` pointer, so we can + // always call this C API safely. unsafe { - // Safety: Self always contains a valid `AIBinder` pointer, so - // we can always call this C API safely. sys::AIBinder_markVendorStability(self.as_native_mut()); } } /// Mark this binder object with local stability, which is vendor if we are - /// building for the VNDK and system otherwise. - #[cfg(not(any(vendor_ndk, android_vndk)))] + /// building for android_vendor and system otherwise. + #[cfg(not(android_vendor))] fn mark_local_stability(&mut self) { + // Safety: Self always contains a valid `AIBinder` pointer, so we can + // always call this C API safely. unsafe { - // Safety: Self always contains a valid `AIBinder` pointer, so - // we can always call this C API safely. sys::AIBinder_markSystemStability(self.as_native_mut()); } } @@ -239,13 +234,13 @@ impl Interface for Binder { /// remotable object, which will prevent the object from being dropped while /// the `SpIBinder` is alive. fn as_binder(&self) -> SpIBinder { + // Safety: `self.ibinder` is guaranteed to always be a valid pointer + // to an `AIBinder` by the `Binder` constructor. We are creating a + // copy of the `self.ibinder` strong reference, but + // `SpIBinder::from_raw` assumes it receives an owned pointer with + // its own strong reference. We first increment the reference count, + // so that the new `SpIBinder` will be tracked as a new reference. unsafe { - // Safety: `self.ibinder` is guaranteed to always be a valid pointer - // to an `AIBinder` by the `Binder` constructor. We are creating a - // copy of the `self.ibinder` strong reference, but - // `SpIBinder::from_raw` assumes it receives an owned pointer with - // its own strong reference. We first increment the reference count, - // so that the new `SpIBinder` will be tracked as a new reference. sys::AIBinder_incStrong(self.ibinder); SpIBinder::from_raw(self.ibinder).unwrap() } @@ -275,10 +270,20 @@ impl InterfaceClassMethods for Binder { reply: *mut sys::AParcel, ) -> status_t { let res = { - let mut reply = BorrowedParcel::from_raw(reply).unwrap(); - let data = BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap(); - let object = sys::AIBinder_getUserData(binder); - let binder: &T = &*(object as *const T); + // Safety: The caller must give us a parcel pointer which is either + // null or valid at least for the duration of this function call. We + // don't keep the resulting value beyond the function. + let mut reply = unsafe { BorrowedParcel::from_raw(reply).unwrap() }; + // Safety: The caller must give us a parcel pointer which is either + // null or valid at least for the duration of this function call. We + // don't keep the resulting value beyond the function. + let data = unsafe { BorrowedParcel::from_raw(data as *mut sys::AParcel).unwrap() }; + // Safety: Our caller promised that `binder` is a non-null, valid + // pointer to a local `AIBinder`. + let object = unsafe { sys::AIBinder_getUserData(binder) }; + // Safety: Our caller promised that the binder has a `T` pointer in + // its user data. + let binder: &T = unsafe { &*(object as *const T) }; binder.on_transact(code, &data, &mut reply) }; match res { @@ -295,7 +300,9 @@ impl InterfaceClassMethods for Binder { /// Must be called with a valid pointer to a `T` object. After this call, /// the pointer will be invalid and should not be dereferenced. unsafe extern "C" fn on_destroy(object: *mut c_void) { - drop(Box::from_raw(object as *mut T)); + // Safety: Our caller promised that `object` is a valid pointer to a + // `T`. + drop(unsafe { Box::from_raw(object as *mut T) }); } /// Called whenever a new, local `AIBinder` object is needed of a specific @@ -320,7 +327,8 @@ impl InterfaceClassMethods for Binder { /// Must be called with a non-null, valid pointer to a local `AIBinder` that /// contains a `T` pointer in its user data. fd should be a non-owned file /// descriptor, and args must be an array of null-terminated string - /// poiinters with length num_args. + /// pointers with length num_args. + #[cfg(not(target_os = "trusty"))] unsafe extern "C" fn on_dump( binder: *mut sys::AIBinder, fd: i32, @@ -330,8 +338,10 @@ impl InterfaceClassMethods for Binder { if fd < 0 { return StatusCode::UNEXPECTED_NULL as status_t; } - // We don't own this file, so we need to be careful not to drop it. - let file = ManuallyDrop::new(File::from_raw_fd(fd)); + use std::os::fd::FromRawFd; + // Safety: Our caller promised that fd is a file descriptor. We don't + // own this file descriptor, so we need to be careful not to drop it. + let mut file = unsafe { ManuallyDrop::new(std::fs::File::from_raw_fd(fd)) }; if args.is_null() && num_args != 0 { return StatusCode::UNEXPECTED_NULL as status_t; @@ -340,21 +350,42 @@ impl InterfaceClassMethods for Binder { let args = if args.is_null() || num_args == 0 { vec![] } else { - slice::from_raw_parts(args, num_args as usize) - .iter() - .map(|s| CStr::from_ptr(*s)) - .collect() + // Safety: Our caller promised that `args` is an array of + // null-terminated string pointers with length `num_args`. + unsafe { + std::slice::from_raw_parts(args, num_args as usize) + .iter() + .map(|s| CStr::from_ptr(*s)) + .collect() + } }; - let object = sys::AIBinder_getUserData(binder); - let binder: &T = &*(object as *const T); - let res = binder.on_dump(&file, &args); + // Safety: Our caller promised that `binder` is a non-null, valid + // pointer to a local `AIBinder`. + let object = unsafe { sys::AIBinder_getUserData(binder) }; + // Safety: Our caller promised that the binder has a `T` pointer in its + // user data. + let binder: &T = unsafe { &*(object as *const T) }; + let res = binder.on_dump(&mut *file, &args); match res { Ok(()) => 0, Err(e) => e as status_t, } } + + /// Called to handle the `dump` transaction. + #[cfg(target_os = "trusty")] + unsafe extern "C" fn on_dump( + _binder: *mut sys::AIBinder, + _fd: i32, + _args: *mut *const c_char, + _num_args: u32, + ) -> status_t { + // This operation is not supported on Trusty right now + // because we do not have a uniform way of writing to handles + StatusCode::INVALID_OPERATION as status_t + } } impl Drop for Binder { @@ -363,11 +394,11 @@ impl Drop for Binder { // actually destroys the object, it calls `on_destroy` and we can drop the // `rust_object` then. fn drop(&mut self) { + // Safety: When `self` is dropped, we can no longer access the + // reference, so can decrement the reference count. `self.ibinder` is + // always a valid `AIBinder` pointer, so is valid to pass to + // `AIBinder_decStrong`. unsafe { - // Safety: When `self` is dropped, we can no longer access the - // reference, so can decrement the reference count. `self.ibinder` - // is always a valid `AIBinder` pointer, so is valid to pass to - // `AIBinder_decStrong`. sys::AIBinder_decStrong(self.ibinder); } } @@ -377,14 +408,11 @@ impl Deref for Binder { type Target = T; fn deref(&self) -> &Self::Target { - unsafe { - // Safety: While `self` is alive, the reference count of the - // underlying object is > 0 and therefore `on_destroy` cannot be - // called. Therefore while `self` is alive, we know that - // `rust_object` is still a valid pointer to a heap allocated object - // of type `T`. - &*self.rust_object - } + // Safety: While `self` is alive, the reference count of the underlying + // object is > 0 and therefore `on_destroy` cannot be called. Therefore + // while `self` is alive, we know that `rust_object` is still a valid + // pointer to a heap allocated object of type `T`. + unsafe { &*self.rust_object } } } @@ -405,13 +433,10 @@ impl TryFrom for Binder { if Some(class) != ibinder.get_class() { return Err(StatusCode::BAD_TYPE); } - let userdata = unsafe { - // Safety: `SpIBinder` always holds a valid pointer pointer to an - // `AIBinder`, which we can safely pass to - // `AIBinder_getUserData`. `ibinder` retains ownership of the - // returned pointer. - sys::AIBinder_getUserData(ibinder.as_native_mut()) - }; + // Safety: `SpIBinder` always holds a valid pointer pointer to an + // `AIBinder`, which we can safely pass to `AIBinder_getUserData`. + // `ibinder` retains ownership of the returned pointer. + let userdata = unsafe { sys::AIBinder_getUserData(ibinder.as_native_mut()) }; if userdata.is_null() { return Err(StatusCode::UNEXPECTED_NULL); } @@ -422,12 +447,10 @@ impl TryFrom for Binder { } } -/// # Safety -/// -/// The constructor for `Binder` guarantees that `self.ibinder` will contain a -/// valid, non-null pointer to an `AIBinder`, so this implementation is type -/// safe. `self.ibinder` will remain valid for the entire lifetime of `self` -/// because we hold a strong reference to the `AIBinder` until `self` is +/// Safety: The constructor for `Binder` guarantees that `self.ibinder` will +/// contain a valid, non-null pointer to an `AIBinder`, so this implementation +/// is type safe. `self.ibinder` will remain valid for the entire lifetime of +/// `self` because we hold a strong reference to the `AIBinder` until `self` is /// dropped. unsafe impl AsNative for Binder { fn as_native(&self) -> *const sys::AIBinder { @@ -447,14 +470,12 @@ unsafe impl AsNative for Binder { /// This function will panic if the identifier contains a 0 byte (NUL). pub fn add_service(identifier: &str, mut binder: SpIBinder) -> Result<()> { let instance = CString::new(identifier).unwrap(); - let status = unsafe { - // Safety: `AServiceManager_addService` expects valid `AIBinder` and C - // string pointers. Caller retains ownership of both - // pointers. `AServiceManager_addService` creates a new strong reference - // and copies the string, so both pointers need only be valid until the - // call returns. - sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr()) - }; + let status = + // Safety: `AServiceManager_addService` expects valid `AIBinder` and C + // string pointers. Caller retains ownership of both pointers. + // `AServiceManager_addService` creates a new strong reference and copies + // the string, so both pointers need only be valid until the call returns. + unsafe { sys::AServiceManager_addService(binder.as_native_mut(), instance.as_ptr()) }; status_result(status) } @@ -470,13 +491,12 @@ pub fn add_service(identifier: &str, mut binder: SpIBinder) -> Result<()> { /// This function will panic if the identifier contains a 0 byte (NUL). pub fn register_lazy_service(identifier: &str, mut binder: SpIBinder) -> Result<()> { let instance = CString::new(identifier).unwrap(); + // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C + // string pointers. Caller retains ownership of both + // pointers. `AServiceManager_registerLazyService` creates a new strong reference + // and copies the string, so both pointers need only be valid until the + // call returns. let status = unsafe { - // Safety: `AServiceManager_registerLazyService` expects valid `AIBinder` and C - // string pointers. Caller retains ownership of both - // pointers. `AServiceManager_registerLazyService` creates a new strong reference - // and copies the string, so both pointers need only be valid until the - // call returns. - sys::AServiceManager_registerLazyService(binder.as_native_mut(), instance.as_ptr()) }; status_result(status) @@ -491,10 +511,8 @@ pub fn register_lazy_service(identifier: &str, mut binder: SpIBinder) -> Result< /// /// Consider using [`LazyServiceGuard`] rather than calling this directly. pub fn force_lazy_services_persist(persist: bool) { - unsafe { - // Safety: No borrowing or transfer of ownership occurs here. - sys::AServiceManager_forceLazyServicesPersist(persist) - } + // Safety: No borrowing or transfer of ownership occurs here. + unsafe { sys::AServiceManager_forceLazyServicesPersist(persist) } } /// An RAII object to ensure a process which registers lazy services is not killed. During the @@ -564,7 +582,7 @@ impl Remotable for () { Ok(()) } - fn on_dump(&self, _file: &File, _args: &[&CStr]) -> Result<()> { + fn on_dump(&self, _writer: &mut dyn Write, _args: &[&CStr]) -> Result<()> { Ok(()) } @@ -576,8 +594,6 @@ impl Interface for () {} /// Determine whether the current thread is currently executing an incoming /// transaction. pub fn is_handling_transaction() -> bool { - unsafe { - // Safety: This method is always safe to call. - sys::AIBinder_isHandlingTransaction() - } + // Safety: This method is always safe to call. + unsafe { sys::AIBinder_isHandlingTransaction() } } diff --git a/binder/src/parcel.rs b/binder/src/parcel.rs index e4c568e..3bfc425 100644 --- a/binder/src/parcel.rs +++ b/binder/src/parcel.rs @@ -34,7 +34,7 @@ mod parcelable_holder; pub use self::file_descriptor::ParcelFileDescriptor; pub use self::parcelable::{ Deserialize, DeserializeArray, DeserializeOption, Parcelable, Serialize, SerializeArray, - SerializeOption, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, + SerializeOption, UnstructuredParcelable, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, }; pub use self::parcelable_holder::{ParcelableHolder, ParcelableMetadata}; @@ -52,11 +52,12 @@ pub struct Parcel { ptr: NonNull, } -/// # Safety +/// Safety: This type guarantees that it owns the AParcel and that all access to +/// the AParcel happens through the Parcel, so it is ok to send across threads. /// -/// This type guarantees that it owns the AParcel and that all access to -/// the AParcel happens through the Parcel, so it is ok to send across -/// threads. +/// It would not be okay to implement Sync, because that would allow you to call +/// the reading methods from several threads in parallel, which would be a data +/// race on the cursor position inside the AParcel. unsafe impl Send for Parcel {} /// Container for a message (data and object references) that can be sent @@ -73,11 +74,9 @@ pub struct BorrowedParcel<'a> { impl Parcel { /// Create a new empty `Parcel`. pub fn new() -> Parcel { - let ptr = unsafe { - // Safety: If `AParcel_create` succeeds, it always returns - // a valid pointer. If it fails, the process will crash. - sys::AParcel_create() - }; + // Safety: If `AParcel_create` succeeds, it always returns + // a valid pointer. If it fails, the process will crash. + let ptr = unsafe { sys::AParcel_create() }; Self { ptr: NonNull::new(ptr).expect("AParcel_create returned null pointer") } } @@ -171,10 +170,8 @@ impl<'a> BorrowedParcel<'a> { } } -/// # Safety -/// -/// The `Parcel` constructors guarantee that a `Parcel` object will always -/// contain a valid pointer to an `AParcel`. +/// Safety: The `Parcel` constructors guarantee that a `Parcel` object will +/// always contain a valid pointer to an `AParcel`. unsafe impl AsNative for Parcel { fn as_native(&self) -> *const sys::AParcel { self.ptr.as_ptr() @@ -185,10 +182,8 @@ unsafe impl AsNative for Parcel { } } -/// # Safety -/// -/// The `BorrowedParcel` constructors guarantee that a `BorrowedParcel` object -/// will always contain a valid pointer to an `AParcel`. +/// Safety: The `BorrowedParcel` constructors guarantee that a `BorrowedParcel` +/// object will always contain a valid pointer to an `AParcel`. unsafe impl<'a> AsNative for BorrowedParcel<'a> { fn as_native(&self) -> *const sys::AParcel { self.ptr.as_ptr() @@ -203,10 +198,8 @@ unsafe impl<'a> AsNative for BorrowedParcel<'a> { impl<'a> BorrowedParcel<'a> { /// Data written to parcelable is zero'd before being deleted or reallocated. pub fn mark_sensitive(&mut self) { - unsafe { - // Safety: guaranteed to have a parcel object, and this method never fails - sys::AParcel_markSensitive(self.as_native()) - } + // Safety: guaranteed to have a parcel object, and this method never fails + unsafe { sys::AParcel_markSensitive(self.as_native()) } } /// Write a type that implements [`Serialize`] to the parcel. @@ -265,11 +258,15 @@ impl<'a> BorrowedParcel<'a> { f(&mut subparcel)?; } let end = self.get_data_position(); + // Safety: start is less than the current size of the parcel data + // buffer, because we just got it with `get_data_position`. unsafe { self.set_data_position(start)?; } assert!(end >= start); self.write(&(end - start))?; + // Safety: end is less than the current size of the parcel data + // buffer, because we just got it with `get_data_position`. unsafe { self.set_data_position(end)?; } @@ -278,20 +275,16 @@ impl<'a> BorrowedParcel<'a> { /// Returns the current position in the parcel data. pub fn get_data_position(&self) -> i32 { - unsafe { - // Safety: `BorrowedParcel` always contains a valid pointer to an - // `AParcel`, and this call is otherwise safe. - sys::AParcel_getDataPosition(self.as_native()) - } + // Safety: `BorrowedParcel` always contains a valid pointer to an + // `AParcel`, and this call is otherwise safe. + unsafe { sys::AParcel_getDataPosition(self.as_native()) } } /// Returns the total size of the parcel. pub fn get_data_size(&self) -> i32 { - unsafe { - // Safety: `BorrowedParcel` always contains a valid pointer to an - // `AParcel`, and this call is otherwise safe. - sys::AParcel_getDataSize(self.as_native()) - } + // Safety: `BorrowedParcel` always contains a valid pointer to an + // `AParcel`, and this call is otherwise safe. + unsafe { sys::AParcel_getDataSize(self.as_native()) } } /// Move the current read/write position in the parcel. @@ -304,7 +297,9 @@ impl<'a> BorrowedParcel<'a> { /// accesses are bounds checked, this call is still safe, but we can't rely /// on that. pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> { - status_result(sys::AParcel_setDataPosition(self.as_native(), pos)) + // Safety: `BorrowedParcel` always contains a valid pointer to an + // `AParcel`, and the caller guarantees that `pos` is within bounds. + status_result(unsafe { sys::AParcel_setDataPosition(self.as_native(), pos) }) } /// Append a subset of another parcel. @@ -317,10 +312,10 @@ impl<'a> BorrowedParcel<'a> { start: i32, size: i32, ) -> Result<()> { + // Safety: `Parcel::appendFrom` from C++ checks that `start` + // and `size` are in bounds, and returns an error otherwise. + // Both `self` and `other` always contain valid pointers. let status = unsafe { - // Safety: `Parcel::appendFrom` from C++ checks that `start` - // and `size` are in bounds, and returns an error otherwise. - // Both `self` and `other` always contain valid pointers. sys::AParcel_appendFrom(other.as_native(), self.as_native_mut(), start, size) }; status_result(status) @@ -418,7 +413,9 @@ impl Parcel { /// accesses are bounds checked, this call is still safe, but we can't rely /// on that. pub unsafe fn set_data_position(&self, pos: i32) -> Result<()> { - self.borrowed_ref().set_data_position(pos) + // Safety: We have the same safety requirements as + // `BorrowedParcel::set_data_position`. + unsafe { self.borrowed_ref().set_data_position(pos) } } /// Append a subset of another parcel. @@ -461,7 +458,7 @@ impl<'a> BorrowedParcel<'a> { /// and call a closure with the sub-parcel as its parameter. /// The closure can keep reading data from the sub-parcel /// until it runs out of input data. The closure is responsible - /// for calling [`ReadableSubParcel::has_more_data`] to check for + /// for calling `ReadableSubParcel::has_more_data` to check for /// more data before every read, at least until Rust generators /// are stabilized. /// After the closure returns, skip to the end of the current @@ -504,7 +501,10 @@ impl<'a> BorrowedParcel<'a> { f(subparcel)?; // Advance the data position to the actual end, - // in case the closure read less data than was available + // in case the closure read less data than was available. + // + // Safety: end must be less than the current size of the parcel, because + // we checked above against `get_data_size`. unsafe { self.set_data_position(end)?; } @@ -595,7 +595,7 @@ impl Parcel { /// and call a closure with the sub-parcel as its parameter. /// The closure can keep reading data from the sub-parcel /// until it runs out of input data. The closure is responsible - /// for calling [`ReadableSubParcel::has_more_data`] to check for + /// for calling `ReadableSubParcel::has_more_data` to check for /// more data before every read, at least until Rust generators /// are stabilized. /// After the closure returns, skip to the end of the current @@ -649,17 +649,17 @@ impl Parcel { // Internal APIs impl<'a> BorrowedParcel<'a> { pub(crate) fn write_binder(&mut self, binder: Option<&SpIBinder>) -> Result<()> { + // Safety: `BorrowedParcel` always contains a valid pointer to an + // `AParcel`. `AsNative` for `Option will either return + // null or a valid pointer to an `AIBinder`, both of which are + // valid, safe inputs to `AParcel_writeStrongBinder`. + // + // This call does not take ownership of the binder. However, it does + // require a mutable pointer, which we cannot extract from an + // immutable reference, so we clone the binder, incrementing the + // refcount before the call. The refcount will be immediately + // decremented when this temporary is dropped. unsafe { - // Safety: `BorrowedParcel` always contains a valid pointer to an - // `AParcel`. `AsNative` for `Option will either return - // null or a valid pointer to an `AIBinder`, both of which are - // valid, safe inputs to `AParcel_writeStrongBinder`. - // - // This call does not take ownership of the binder. However, it does - // require a mutable pointer, which we cannot extract from an - // immutable reference, so we clone the binder, incrementing the - // refcount before the call. The refcount will be immediately - // decremented when this temporary is dropped. status_result(sys::AParcel_writeStrongBinder( self.as_native_mut(), binder.cloned().as_native_mut(), @@ -669,33 +669,28 @@ impl<'a> BorrowedParcel<'a> { pub(crate) fn read_binder(&self) -> Result> { let mut binder = ptr::null_mut(); - let status = unsafe { - // Safety: `BorrowedParcel` always contains a valid pointer to an - // `AParcel`. We pass a valid, mutable out pointer to the `binder` - // parameter. After this call, `binder` will be either null or a - // valid pointer to an `AIBinder` owned by the caller. - sys::AParcel_readStrongBinder(self.as_native(), &mut binder) - }; + // Safety: `BorrowedParcel` always contains a valid pointer to an + // `AParcel`. We pass a valid, mutable out pointer to the `binder` + // parameter. After this call, `binder` will be either null or a + // valid pointer to an `AIBinder` owned by the caller. + let status = unsafe { sys::AParcel_readStrongBinder(self.as_native(), &mut binder) }; status_result(status)?; - Ok(unsafe { - // Safety: `binder` is either null or a valid, owned pointer at this - // point, so can be safely passed to `SpIBinder::from_raw`. - SpIBinder::from_raw(binder) - }) + // Safety: `binder` is either null or a valid, owned pointer at this + // point, so can be safely passed to `SpIBinder::from_raw`. + Ok(unsafe { SpIBinder::from_raw(binder) }) } } impl Drop for Parcel { fn drop(&mut self) { // Run the C++ Parcel complete object destructor - unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. Since we own the parcel, we can safely delete it - // here. - sys::AParcel_delete(self.ptr.as_ptr()) - } + // + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. Since we own the parcel, we can safely delete it + // here. + unsafe { sys::AParcel_delete(self.ptr.as_ptr()) } } } @@ -732,6 +727,8 @@ fn test_read_write() { parcel.write(&1i32).unwrap(); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { parcel.set_data_position(start).unwrap(); } @@ -748,6 +745,8 @@ fn test_read_data() { parcel.write(&b"Hello, Binder!\0"[..]).unwrap(); // Skip over string length + // SAFETY: str_start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(str_start).is_ok()); } @@ -756,42 +755,56 @@ fn test_read_data() { assert!(parcel.read::().unwrap()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 72i8); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 25928); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 1819043144); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 1819043144); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 4764857262830019912); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 4764857262830019912); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -799,6 +812,8 @@ fn test_read_data() { assert_eq!(parcel.read::().unwrap(), 1143139100000000000000000000.0); assert_eq!(parcel.read::().unwrap(), 40.043392); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -806,6 +821,8 @@ fn test_read_data() { assert_eq!(parcel.read::().unwrap(), 34732488246.197815); // Skip back to before the string length + // SAFETY: str_start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(str_start).is_ok()); } @@ -819,15 +836,21 @@ fn test_utf8_utf16_conversions() { let start = parcel.get_data_position(); assert!(parcel.write("Hello, Binder!").is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::>().unwrap().unwrap(), "Hello, Binder!",); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(parcel.write("Embedded null \0 inside a string").is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -835,6 +858,8 @@ fn test_utf8_utf16_conversions() { parcel.read::>().unwrap().unwrap(), "Embedded null \0 inside a string", ); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -849,6 +874,8 @@ fn test_utf8_utf16_conversions() { let s3 = "Some more text here."; assert!(parcel.write(&[s1, s2, s3][..]).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -874,6 +901,8 @@ fn test_sized_write() { assert_eq!(parcel.get_data_position(), start + expected_len); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { parcel.set_data_position(start).unwrap(); } @@ -893,6 +922,8 @@ fn test_append_from() { assert_eq!(4, parcel2.get_data_size()); assert_eq!(Ok(()), parcel2.append_all_from(&parcel1)); assert_eq!(8, parcel2.get_data_size()); + // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not + // empty. unsafe { parcel2.set_data_position(0).unwrap(); } @@ -903,6 +934,8 @@ fn test_append_from() { assert_eq!(Ok(()), parcel2.append_from(&parcel1, 0, 2)); assert_eq!(Ok(()), parcel2.append_from(&parcel1, 2, 2)); assert_eq!(4, parcel2.get_data_size()); + // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not + // empty. unsafe { parcel2.set_data_position(0).unwrap(); } @@ -911,6 +944,8 @@ fn test_append_from() { let mut parcel2 = Parcel::new(); assert_eq!(Ok(()), parcel2.append_from(&parcel1, 0, 2)); assert_eq!(2, parcel2.get_data_size()); + // SAFETY: 0 is less than the current size of the parcel data buffer, because the parcel is not + // empty. unsafe { parcel2.set_data_position(0).unwrap(); } diff --git a/binder/src/parcel/file_descriptor.rs b/binder/src/parcel/file_descriptor.rs index de6d649..6afe5ab 100644 --- a/binder/src/parcel/file_descriptor.rs +++ b/binder/src/parcel/file_descriptor.rs @@ -22,29 +22,28 @@ use crate::binder::AsNative; use crate::error::{status_result, Result, StatusCode}; use crate::sys; -use std::fs::File; -use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; /// Rust version of the Java class android.os.ParcelFileDescriptor #[derive(Debug)] -pub struct ParcelFileDescriptor(File); +pub struct ParcelFileDescriptor(OwnedFd); impl ParcelFileDescriptor { /// Create a new `ParcelFileDescriptor` - pub fn new(file: File) -> Self { - Self(file) + pub fn new>(fd: F) -> Self { + Self(fd.into()) } } -impl AsRef for ParcelFileDescriptor { - fn as_ref(&self) -> &File { +impl AsRef for ParcelFileDescriptor { + fn as_ref(&self) -> &OwnedFd { &self.0 } } -impl From for File { - fn from(file: ParcelFileDescriptor) -> File { - file.0 +impl From for OwnedFd { + fn from(fd: ParcelFileDescriptor) -> OwnedFd { + fd.0 } } @@ -73,14 +72,12 @@ impl Eq for ParcelFileDescriptor {} impl Serialize for ParcelFileDescriptor { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> { let fd = self.0.as_raw_fd(); - let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a - // valid file, so we can borrow a valid file - // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take - // ownership of the fd, so we need not duplicate it first. - sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd) - }; + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. Likewise, `ParcelFileDescriptor` always contains a + // valid file, so we can borrow a valid file + // descriptor. `AParcel_writeParcelFileDescriptor` does NOT take + // ownership of the fd, so we need not duplicate it first. + let status = unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), fd) }; status_result(status) } } @@ -92,13 +89,12 @@ impl SerializeOption for ParcelFileDescriptor { if let Some(f) = this { f.serialize(parcel) } else { - let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the - // value `-1` as the file descriptor to signify serializing a - // null file descriptor. - sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32) - }; + let status = + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. `AParcel_writeParcelFileDescriptor` accepts the + // value `-1` as the file descriptor to signify serializing a + // null file descriptor. + unsafe { sys::AParcel_writeParcelFileDescriptor(parcel.as_native_mut(), -1i32) }; status_result(status) } } @@ -107,31 +103,37 @@ impl SerializeOption for ParcelFileDescriptor { impl DeserializeOption for ParcelFileDescriptor { fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result> { let mut fd = -1i32; + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. We pass a valid mutable pointer to an i32, which + // `AParcel_readParcelFileDescriptor` assigns the valid file + // descriptor into, or `-1` if deserializing a null file + // descriptor. The read function passes ownership of the file + // descriptor to its caller if it was non-null, so we must take + // ownership of the file and ensure that it is eventually closed. unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. We pass a valid mutable pointer to an i32, which - // `AParcel_readParcelFileDescriptor` assigns the valid file - // descriptor into, or `-1` if deserializing a null file - // descriptor. The read function passes ownership of the file - // descriptor to its caller if it was non-null, so we must take - // ownership of the file and ensure that it is eventually closed. status_result(sys::AParcel_readParcelFileDescriptor(parcel.as_native(), &mut fd))?; } if fd < 0 { Ok(None) } else { - let file = unsafe { - // Safety: At this point, we know that the file descriptor was - // not -1, so must be a valid, owned file descriptor which we - // can safely turn into a `File`. - File::from_raw_fd(fd) - }; + // Safety: At this point, we know that the file descriptor was + // not -1, so must be a valid, owned file descriptor which we + // can safely turn into a `File`. + let file = unsafe { OwnedFd::from_raw_fd(fd) }; Ok(Some(ParcelFileDescriptor::new(file))) } } } impl Deserialize for ParcelFileDescriptor { + type UninitType = Option; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } diff --git a/binder/src/parcel/parcelable.rs b/binder/src/parcel/parcelable.rs index 6f4c375..33dfe19 100644 --- a/binder/src/parcel/parcelable.rs +++ b/binder/src/parcel/parcelable.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use crate::binder::{AsNative, FromIBinder, Stability, Strong}; +use crate::binder::{AsNative, FromIBinder, Interface, Stability, Strong}; use crate::error::{status_result, status_t, Result, Status, StatusCode}; use crate::parcel::BorrowedParcel; use crate::proxy::SpIBinder; @@ -22,12 +22,12 @@ use crate::sys; use std::convert::{TryFrom, TryInto}; use std::ffi::c_void; -use std::mem::{self, ManuallyDrop, MaybeUninit}; +use std::mem::{self, ManuallyDrop}; use std::os::raw::c_char; use std::ptr; use std::slice; -/// Super-trait for Binder parcelables. +/// Super-trait for structured Binder parcelables, i.e. those generated from AIDL. /// /// This trait is equivalent `android::Parcelable` in C++, /// and defines a common interface that all parcelables need @@ -50,20 +50,69 @@ pub trait Parcelable { fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()>; } -/// A struct whose instances can be written to a [`Parcel`]. +/// Super-trait for unstructured Binder parcelables, i.e. those implemented manually. +/// +/// These differ from structured parcelables in that they may not have a reasonable default value +/// and so aren't required to implement `Default`. +pub trait UnstructuredParcelable: Sized { + /// Internal serialization function for parcelables. + /// + /// This method is mainly for internal use. `Serialize::serialize` and its variants are + /// generally preferred over calling this function, since the former also prepend a header. + fn write_to_parcel(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()>; + + /// Internal deserialization function for parcelables. + /// + /// This method is mainly for internal use. `Deserialize::deserialize` and its variants are + /// generally preferred over calling this function, since the former also parse the additional + /// header. + fn from_parcel(parcel: &BorrowedParcel<'_>) -> Result; + + /// Internal deserialization function for parcelables. + /// + /// This method is mainly for internal use. `Deserialize::deserialize_from` and its variants are + /// generally preferred over calling this function, since the former also parse the additional + /// header. + fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()> { + *self = Self::from_parcel(parcel)?; + Ok(()) + } +} + +/// A struct whose instances can be written to a [`crate::parcel::Parcel`]. // Might be able to hook this up as a serde backend in the future? pub trait Serialize { - /// Serialize this instance into the given [`Parcel`]. + /// Serialize this instance into the given [`crate::parcel::Parcel`]. fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()>; } -/// A struct whose instances can be restored from a [`Parcel`]. +/// A struct whose instances can be restored from a [`crate::parcel::Parcel`]. // Might be able to hook this up as a serde backend in the future? pub trait Deserialize: Sized { - /// Deserialize an instance from the given [`Parcel`]. + /// Type for the uninitialized value of this type. Will be either `Self` + /// if the type implements `Default`, `Option` otherwise. + type UninitType; + + /// Assert at compile-time that `Self` and `Self::UninitType` have the same + /// size and alignment. This will either fail to compile or evaluate to `true`. + /// The only two macros that work here are `panic!` and `assert!`, so we cannot + /// use `assert_eq!`. + const ASSERT_UNINIT_SIZE_AND_ALIGNMENT: bool = { + assert!(std::mem::size_of::() == std::mem::size_of::()); + assert!(std::mem::align_of::() == std::mem::align_of::()); + true + }; + + /// Return an uninitialized or default-initialized value for this type. + fn uninit() -> Self::UninitType; + + /// Convert an initialized value of type `Self` into `Self::UninitType`. + fn from_init(value: Self) -> Self::UninitType; + + /// Deserialize an instance from the given [`crate::parcel::Parcel`]. fn deserialize(parcel: &BorrowedParcel<'_>) -> Result; - /// Deserialize an instance from the given [`Parcel`] onto the + /// Deserialize an instance from the given [`crate::parcel::Parcel`] onto the /// current object. This operation will overwrite the old value /// partially or completely, depending on how much data is available. fn deserialize_from(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()> { @@ -82,8 +131,8 @@ pub trait Deserialize: Sized { pub trait SerializeArray: Serialize + Sized { /// Serialize an array of this type into the given parcel. fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: Safe FFI, slice will always be a safe pointer to pass. let res = unsafe { - // Safety: Safe FFI, slice will always be a safe pointer to pass. sys::AParcel_writeParcelableArray( parcel.as_native_mut(), slice.as_ptr() as *const c_void, @@ -97,7 +146,9 @@ pub trait SerializeArray: Serialize + Sized { /// Callback to serialize an element of a generic parcelable array. /// -/// Safety: We are relying on binder_ndk to not overrun our slice. As long as it +/// # Safety +/// +/// We are relying on binder_ndk to not overrun our slice. As long as it /// doesn't provide an index larger than the length of the original slice in /// serialize_array, this operation is safe. The index provided is zero-based. unsafe extern "C" fn serialize_element( @@ -105,9 +156,14 @@ unsafe extern "C" fn serialize_element( array: *const c_void, index: usize, ) -> status_t { - let slice: &[T] = slice::from_raw_parts(array.cast(), index + 1); - - let mut parcel = match BorrowedParcel::from_raw(parcel) { + // Safety: The caller guarantees that `array` is a valid pointer of the + // appropriate type. + let slice: &[T] = unsafe { slice::from_raw_parts(array.cast(), index + 1) }; + + // Safety: The caller must give us a parcel pointer which is either null or + // valid at least for the duration of this function call. We don't keep the + // resulting value beyond the function. + let mut parcel = match unsafe { BorrowedParcel::from_raw(parcel) } { None => return StatusCode::UNEXPECTED_NULL as status_t, Some(p) => p, }; @@ -121,10 +177,10 @@ unsafe extern "C" fn serialize_element( pub trait DeserializeArray: Deserialize { /// Deserialize an array of type from the given parcel. fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result>> { - let mut vec: Option>> = None; + let mut vec: Option> = None; + // Safety: Safe FFI, vec is the correct opaque type expected by + // allocate_vec and deserialize_element. let res = unsafe { - // Safety: Safe FFI, vec is the correct opaque type expected by - // allocate_vec and deserialize_element. sys::AParcel_readParcelableArray( parcel.as_native(), &mut vec as *mut _ as *mut c_void, @@ -133,36 +189,41 @@ pub trait DeserializeArray: Deserialize { ) }; status_result(res)?; - let vec: Option> = unsafe { - // Safety: We are assuming that the NDK correctly initialized every - // element of the vector by now, so we know that all the - // MaybeUninits are now properly initialized. We can transmute from - // Vec> to Vec because MaybeUninit has the same - // alignment and size as T, so the pointer to the vector allocation - // will be compatible. - mem::transmute(vec) - }; + // Safety: We are assuming that the NDK correctly initialized every + // element of the vector by now, so we know that all the + // UninitTypes are now properly initialized. We can transmute from + // Vec to Vec because T::UninitType has the same + // alignment and size as T, so the pointer to the vector allocation + // will be compatible. + let vec: Option> = unsafe { mem::transmute(vec) }; Ok(vec) } } /// Callback to deserialize a parcelable element. /// +/// # Safety +/// /// The opaque array data pointer must be a mutable pointer to an -/// `Option>>` with at least enough elements for `index` to be valid +/// `Option>` with at least enough elements for `index` to be valid /// (zero-based). unsafe extern "C" fn deserialize_element( parcel: *const sys::AParcel, array: *mut c_void, index: usize, ) -> status_t { - let vec = &mut *(array as *mut Option>>); + // Safety: The caller guarantees that `array` is a valid pointer of the + // appropriate type. + let vec = unsafe { &mut *(array as *mut Option>) }; let vec = match vec { Some(v) => v, None => return StatusCode::BAD_INDEX as status_t, }; - let parcel = match BorrowedParcel::from_raw(parcel as *mut _) { + // Safety: The caller must give us a parcel pointer which is either null or + // valid at least for the duration of this function call. We don't keep the + // resulting value beyond the function. + let parcel = match unsafe { BorrowedParcel::from_raw(parcel as *mut _) } { None => return StatusCode::UNEXPECTED_NULL as status_t, Some(p) => p, }; @@ -170,7 +231,7 @@ unsafe extern "C" fn deserialize_element( Ok(e) => e, Err(code) => return code as status_t, }; - ptr::write(vec[index].as_mut_ptr(), element); + vec[index] = T::from_init(element); StatusCode::OK as status_t } @@ -233,17 +294,22 @@ pub trait DeserializeOption: Deserialize { /// # Safety /// /// The opaque data pointer passed to the array read function must be a mutable -/// pointer to an `Option>>`. `buffer` will be assigned a mutable pointer -/// to the allocated vector data if this function returns true. -unsafe extern "C" fn allocate_vec_with_buffer( +/// pointer to an `Option>`. `buffer` will be assigned a mutable pointer +/// to the allocated vector data if this function returns true. `buffer` must be a valid pointer. +unsafe extern "C" fn allocate_vec_with_buffer( data: *mut c_void, len: i32, buffer: *mut *mut T, ) -> bool { - let res = allocate_vec::(data, len); - let vec = &mut *(data as *mut Option>>); + // Safety: We have the same safety requirements as `allocate_vec` for `data`. + let res = unsafe { allocate_vec::(data, len) }; + // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type. + let vec = unsafe { &mut *(data as *mut Option>) }; if let Some(new_vec) = vec { - *buffer = new_vec.as_mut_ptr() as *mut T; + // Safety: The caller guarantees that `buffer` is a valid pointer. + unsafe { + *buffer = new_vec.as_mut_ptr() as *mut T; + } } res } @@ -253,22 +319,24 @@ unsafe extern "C" fn allocate_vec_with_buffer( /// # Safety /// /// The opaque data pointer passed to the array read function must be a mutable -/// pointer to an `Option>>`. -unsafe extern "C" fn allocate_vec(data: *mut c_void, len: i32) -> bool { - let vec = &mut *(data as *mut Option>>); +/// pointer to an `Option>`. +unsafe extern "C" fn allocate_vec(data: *mut c_void, len: i32) -> bool { + // Safety: The caller guarantees that `data` is a valid mutable pointer to the appropriate type. + let vec = unsafe { &mut *(data as *mut Option>) }; if len < 0 { *vec = None; return true; } - let mut new_vec: Vec> = Vec::with_capacity(len as usize); - // Safety: We are filling the vector with uninitialized data here, but this - // is safe because the vector contains MaybeUninit elements which can be - // uninitialized. We're putting off the actual unsafe bit, transmuting the - // vector to a Vec until the contents are initialized. - new_vec.set_len(len as usize); + // Assert at compile time that `T` and `T::UninitType` have the same size and alignment. + let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT; + let mut new_vec: Vec = Vec::with_capacity(len as usize); + new_vec.resize_with(len as usize, T::uninit); - ptr::write(vec, Some(new_vec)); + // Safety: The caller guarantees that vec is a valid mutable pointer to the appropriate type. + unsafe { + ptr::write(vec, Some(new_vec)); + } true } @@ -283,22 +351,25 @@ macro_rules! parcelable_primitives { } /// Safety: All elements in the vector must be properly initialized. -unsafe fn vec_assume_init(vec: Vec>) -> Vec { - // We can convert from Vec> to Vec because MaybeUninit - // has the same alignment and size as T, so the pointer to the vector - // allocation will be compatible. +unsafe fn vec_assume_init(vec: Vec) -> Vec { + // Assert at compile time that `T` and `T::UninitType` have the same size and alignment. + let _ = T::ASSERT_UNINIT_SIZE_AND_ALIGNMENT; + let mut vec = ManuallyDrop::new(vec); - Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity()) + // Safety: We can convert from Vec to Vec because + // T::UninitType has the same alignment and size as T, so the pointer to the + // vector allocation will be compatible. + unsafe { Vec::from_raw_parts(vec.as_mut_ptr().cast(), vec.len(), vec.capacity()) } } macro_rules! impl_parcelable { {Serialize, $ty:ty, $write_fn:path} => { impl Serialize for $ty { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`, and any `$ty` literal value is safe to pass to + // `$write_fn`. unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`, and any `$ty` literal value is safe to pass to - // `$write_fn`. status_result($write_fn(parcel.as_native_mut(), *self)) } } @@ -307,13 +378,16 @@ macro_rules! impl_parcelable { {Deserialize, $ty:ty, $read_fn:path} => { impl Deserialize for $ty { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let mut val = Self::default(); + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. We pass a valid, mutable pointer to `val`, a + // literal of type `$ty`, and `$read_fn` will write the + // value read into `val` if successful unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. We pass a valid, mutable pointer to `val`, a - // literal of type `$ty`, and `$read_fn` will write the - // value read into `val` if successful status_result($read_fn(parcel.as_native(), &mut val))? }; Ok(val) @@ -324,13 +398,13 @@ macro_rules! impl_parcelable { {SerializeArray, $ty:ty, $write_array_fn:path} => { impl SerializeArray for $ty { fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` + // will be a valid pointer to an array of elements of type + // `$ty`. If the slice length is 0, `slice.as_ptr()` may be + // dangling, but this is safe since the pointer is not + // dereferenced if the length parameter is 0. let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` - // will be a valid pointer to an array of elements of type - // `$ty`. If the slice length is 0, `slice.as_ptr()` may be - // dangling, but this is safe since the pointer is not - // dereferenced if the length parameter is 0. $write_array_fn( parcel.as_native_mut(), slice.as_ptr(), @@ -348,12 +422,12 @@ macro_rules! impl_parcelable { {DeserializeArray, $ty:ty, $read_array_fn:path} => { impl DeserializeArray for $ty { fn deserialize_array(parcel: &BorrowedParcel<'_>) -> Result>> { - let mut vec: Option>> = None; + let mut vec: Option> = None; + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. `allocate_vec` expects the opaque pointer to + // be of type `*mut Option>`, so `&mut vec` is + // correct for it. let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. `allocate_vec` expects the opaque pointer to - // be of type `*mut Option>>`, so `&mut vec` is - // correct for it. $read_array_fn( parcel.as_native(), &mut vec as *mut _ as *mut c_void, @@ -361,11 +435,11 @@ macro_rules! impl_parcelable { ) }; status_result(status)?; + // Safety: We are assuming that the NDK correctly + // initialized every element of the vector by now, so we + // know that all the UninitTypes are now properly + // initialized. let vec: Option> = unsafe { - // Safety: We are assuming that the NDK correctly - // initialized every element of the vector by now, so we - // know that all the MaybeUninits are now properly - // initialized. vec.map(|vec| vec_assume_init(vec)) }; Ok(vec) @@ -440,6 +514,14 @@ impl Serialize for u8 { } impl Deserialize for u8 { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { i8::deserialize(parcel).map(|v| v as u8) } @@ -447,13 +529,13 @@ impl Deserialize for u8 { impl SerializeArray for u8 { fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a + // valid pointer to an array of elements of type `$ty`. If the slice + // length is 0, `slice.as_ptr()` may be dangling, but this is safe + // since the pointer is not dereferenced if the length parameter is + // 0. let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a - // valid pointer to an array of elements of type `$ty`. If the slice - // length is 0, `slice.as_ptr()` may be dangling, but this is safe - // since the pointer is not dereferenced if the length parameter is - // 0. sys::AParcel_writeByteArray( parcel.as_native_mut(), slice.as_ptr() as *const i8, @@ -471,6 +553,14 @@ impl Serialize for i16 { } impl Deserialize for i16 { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { u16::deserialize(parcel).map(|v| v as i16) } @@ -478,13 +568,13 @@ impl Deserialize for i16 { impl SerializeArray for i16 { fn serialize_array(slice: &[Self], parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a + // valid pointer to an array of elements of type `$ty`. If the slice + // length is 0, `slice.as_ptr()` may be dangling, but this is safe + // since the pointer is not dereferenced if the length parameter is + // 0. let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. If the slice is > 0 length, `slice.as_ptr()` will be a - // valid pointer to an array of elements of type `$ty`. If the slice - // length is 0, `slice.as_ptr()` may be dangling, but this is safe - // since the pointer is not dereferenced if the length parameter is - // 0. sys::AParcel_writeCharArray( parcel.as_native_mut(), slice.as_ptr() as *const u16, @@ -498,22 +588,22 @@ impl SerializeArray for i16 { impl SerializeOption for str { fn serialize_option(this: Option<&Self>, parcel: &mut BorrowedParcel<'_>) -> Result<()> { match this { + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. If the string pointer is null, + // `AParcel_writeString` requires that the length is -1 to + // indicate that we want to serialize a null string. None => unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. If the string pointer is null, - // `AParcel_writeString` requires that the length is -1 to - // indicate that we want to serialize a null string. status_result(sys::AParcel_writeString(parcel.as_native_mut(), ptr::null(), -1)) }, + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. `AParcel_writeString` assumes that we pass a utf-8 + // string pointer of `length` bytes, which is what str in Rust + // is. The docstring for `AParcel_writeString` says that the + // string input should be null-terminated, but it doesn't + // actually rely on that fact in the code. If this ever becomes + // necessary, we will need to null-terminate the str buffer + // before sending it. Some(s) => unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. `AParcel_writeString` assumes that we pass a utf-8 - // string pointer of `length` bytes, which is what str in Rust - // is. The docstring for `AParcel_writeString` says that the - // string input should be null-terminated, but it doesn't - // actually rely on that fact in the code. If this ever becomes - // necessary, we will need to null-terminate the str buffer - // before sending it. status_result(sys::AParcel_writeString( parcel.as_native_mut(), s.as_ptr() as *const c_char, @@ -547,13 +637,21 @@ impl SerializeOption for String { } impl Deserialize for Option { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let mut vec: Option> = None; + // Safety: `Parcel` always contains a valid pointer to an `AParcel`. + // `Option>` is equivalent to the expected `Option>` + // for `allocate_vec`, so `vec` is safe to pass as the opaque data + // pointer on platforms where char is signed. let status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an `AParcel`. - // `Option>` is equivalent to the expected `Option>` - // for `allocate_vec`, so `vec` is safe to pass as the opaque data - // pointer on platforms where char is signed. sys::AParcel_readString( parcel.as_native(), &mut vec as *mut _ as *mut c_void, @@ -575,6 +673,14 @@ impl Deserialize for Option { impl DeserializeArray for Option {} impl Deserialize for String { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { Deserialize::deserialize(parcel).transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } @@ -611,6 +717,14 @@ impl SerializeOption for Vec { } impl Deserialize for Vec { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { DeserializeArray::deserialize_array(parcel) .transpose() @@ -640,6 +754,14 @@ impl SerializeOption for [T; N] { impl SerializeArray for [T; N] {} impl Deserialize for [T; N] { + type UninitType = [T::UninitType; N]; + fn uninit() -> Self::UninitType { + [(); N].map(|_| T::uninit()) + } + fn from_init(value: Self) -> Self::UninitType { + value.map(T::from_init) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let vec = DeserializeArray::deserialize_array(parcel) .transpose() @@ -664,6 +786,14 @@ impl Serialize for Stability { } impl Deserialize for Stability { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { i32::deserialize(parcel).and_then(Stability::try_from) } @@ -671,34 +801,39 @@ impl Deserialize for Stability { impl Serialize for Status { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> { + // Safety: `Parcel` always contains a valid pointer to an `AParcel` + // and `Status` always contains a valid pointer to an `AStatus`, so + // both parameters are valid and safe. This call does not take + // ownership of either of its parameters. unsafe { - // Safety: `Parcel` always contains a valid pointer to an `AParcel` - // and `Status` always contains a valid pointer to an `AStatus`, so - // both parameters are valid and safe. This call does not take - // ownership of either of its parameters. status_result(sys::AParcel_writeStatusHeader(parcel.as_native_mut(), self.as_native())) } } } impl Deserialize for Status { + type UninitType = Option; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let mut status_ptr = ptr::null_mut(); - let ret_status = unsafe { - // Safety: `Parcel` always contains a valid pointer to an - // `AParcel`. We pass a mutable out pointer which will be - // assigned a valid `AStatus` pointer if the function returns - // status OK. This function passes ownership of the status - // pointer to the caller, if it was assigned. - sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr) - }; + let ret_status = + // Safety: `Parcel` always contains a valid pointer to an + // `AParcel`. We pass a mutable out pointer which will be + // assigned a valid `AStatus` pointer if the function returns + // status OK. This function passes ownership of the status + // pointer to the caller, if it was assigned. + unsafe { sys::AParcel_readStatusHeader(parcel.as_native(), &mut status_ptr) }; status_result(ret_status)?; - Ok(unsafe { - // Safety: At this point, the return status of the read call was ok, - // so we know that `status_ptr` is a valid, owned pointer to an - // `AStatus`, from which we can safely construct a `Status` object. - Status::from_ptr(status_ptr) - }) + // Safety: At this point, the return status of the read call was ok, + // so we know that `status_ptr` is a valid, owned pointer to an + // `AStatus`, from which we can safely construct a `Status` object. + Ok(unsafe { Status::from_ptr(status_ptr) }) } } @@ -717,12 +852,29 @@ impl SerializeOption for Strong { impl SerializeArray for Strong {} impl Deserialize for Strong { + type UninitType = Option>; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let ibinder: SpIBinder = parcel.read()?; FromIBinder::try_from(ibinder) } } +struct AssertIBinder; +impl Interface for AssertIBinder {} +impl FromIBinder for AssertIBinder { + // This is only needed so we can assert on the size of Strong + fn try_from(_: SpIBinder) -> Result> { + unimplemented!() + } +} + impl DeserializeOption for Strong { fn deserialize_option(parcel: &BorrowedParcel<'_>) -> Result> { let ibinder: Option = parcel.read()?; @@ -752,6 +904,14 @@ impl Serialize for Option { } impl Deserialize for Option { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { DeserializeOption::deserialize_option(parcel) } @@ -767,11 +927,16 @@ impl Deserialize for Option { /// `Serialize`, `SerializeArray` and `SerializeOption` for /// structured parcelables. The target type must implement the /// `Parcelable` trait. -/// ``` #[macro_export] macro_rules! impl_serialize_for_parcelable { ($parcelable:ident) => { - impl $crate::binder_impl::Serialize for $parcelable { + $crate::impl_serialize_for_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_serialize_for_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param),* > $crate::binder_impl::Serialize for $parcelable < $($param),* > { fn serialize( &self, parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, @@ -780,9 +945,9 @@ macro_rules! impl_serialize_for_parcelable { } } - impl $crate::binder_impl::SerializeArray for $parcelable {} + impl < $($param),* > $crate::binder_impl::SerializeArray for $parcelable < $($param),* > {} - impl $crate::binder_impl::SerializeOption for $parcelable { + impl < $($param),* > $crate::binder_impl::SerializeOption for $parcelable < $($param),* > { fn serialize_option( this: Option<&Self>, parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, @@ -808,7 +973,16 @@ macro_rules! impl_serialize_for_parcelable { #[macro_export] macro_rules! impl_deserialize_for_parcelable { ($parcelable:ident) => { - impl $crate::binder_impl::Deserialize for $parcelable { + $crate::impl_deserialize_for_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_deserialize_for_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param: Default),* > $crate::binder_impl::Deserialize for $parcelable < $($param),* > { + type UninitType = Self; + fn uninit() -> Self::UninitType { Self::UninitType::default() } + fn from_init(value: Self) -> Self::UninitType { value } fn deserialize( parcel: &$crate::binder_impl::BorrowedParcel<'_>, ) -> std::result::Result { @@ -830,9 +1004,9 @@ macro_rules! impl_deserialize_for_parcelable { } } - impl $crate::binder_impl::DeserializeArray for $parcelable {} + impl < $($param: Default),* > $crate::binder_impl::DeserializeArray for $parcelable < $($param),* > {} - impl $crate::binder_impl::DeserializeOption for $parcelable { + impl < $($param: Default),* > $crate::binder_impl::DeserializeOption for $parcelable < $($param),* > { fn deserialize_option( parcel: &$crate::binder_impl::BorrowedParcel<'_>, ) -> std::result::Result, $crate::StatusCode> { @@ -857,6 +1031,125 @@ macro_rules! impl_deserialize_for_parcelable { }; } +/// Implements `Serialize` trait and friends for an unstructured parcelable. +/// +/// The target type must implement the `UnstructuredParcelable` trait. +#[macro_export] +macro_rules! impl_serialize_for_unstructured_parcelable { + ($parcelable:ident) => { + $crate::impl_serialize_for_unstructured_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_serialize_for_unstructured_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param),* > $crate::binder_impl::Serialize for $parcelable < $($param),* > { + fn serialize( + &self, + parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + ::serialize_option(Some(self), parcel) + } + } + + impl < $($param),* > $crate::binder_impl::SerializeArray for $parcelable < $($param),* > {} + + impl < $($param),* > $crate::binder_impl::SerializeOption for $parcelable < $($param),* > { + fn serialize_option( + this: Option<&Self>, + parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + if let Some(this) = this { + use $crate::binder_impl::UnstructuredParcelable; + parcel.write(&$crate::binder_impl::NON_NULL_PARCELABLE_FLAG)?; + this.write_to_parcel(parcel) + } else { + parcel.write(&$crate::binder_impl::NULL_PARCELABLE_FLAG) + } + } + } + }; +} + +/// Implement `Deserialize` trait and friends for an unstructured parcelable +/// +/// The target type must implement the `UnstructuredParcelable` trait. +#[macro_export] +macro_rules! impl_deserialize_for_unstructured_parcelable { + ($parcelable:ident) => { + $crate::impl_deserialize_for_unstructured_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_deserialize_for_unstructured_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param: Default),* > $crate::binder_impl::Deserialize for $parcelable < $($param),* > { + type UninitType = Option; + fn uninit() -> Self::UninitType { None } + fn from_init(value: Self) -> Self::UninitType { Some(value) } + fn deserialize( + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result { + $crate::binder_impl::DeserializeOption::deserialize_option(parcel) + .transpose() + .unwrap_or(Err($crate::StatusCode::UNEXPECTED_NULL)) + } + fn deserialize_from( + &mut self, + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + let status: i32 = parcel.read()?; + if status == $crate::binder_impl::NULL_PARCELABLE_FLAG { + Err($crate::StatusCode::UNEXPECTED_NULL) + } else { + use $crate::binder_impl::UnstructuredParcelable; + self.read_from_parcel(parcel) + } + } + } + + impl < $($param: Default),* > $crate::binder_impl::DeserializeArray for $parcelable < $($param),* > {} + + impl < $($param: Default),* > $crate::binder_impl::DeserializeOption for $parcelable < $($param),* > { + fn deserialize_option( + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result, $crate::StatusCode> { + let present: i32 = parcel.read()?; + match present { + $crate::binder_impl::NULL_PARCELABLE_FLAG => Ok(None), + $crate::binder_impl::NON_NULL_PARCELABLE_FLAG => { + use $crate::binder_impl::UnstructuredParcelable; + Ok(Some(Self::from_parcel(parcel)?)) + } + _ => Err(StatusCode::BAD_VALUE), + } + } + fn deserialize_option_from( + this: &mut Option, + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + let present: i32 = parcel.read()?; + match present { + $crate::binder_impl::NULL_PARCELABLE_FLAG => { + *this = None; + Ok(()) + } + $crate::binder_impl::NON_NULL_PARCELABLE_FLAG => { + use $crate::binder_impl::UnstructuredParcelable; + if let Some(this) = this { + this.read_from_parcel(parcel)?; + } else { + *this = Some(Self::from_parcel(parcel)?); + } + Ok(()) + } + _ => Err(StatusCode::BAD_VALUE), + } + } + } + }; +} + impl Serialize for Box { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> { Serialize::serialize(&**self, parcel) @@ -864,6 +1157,14 @@ impl Serialize for Box { } impl Deserialize for Box { + type UninitType = Option; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { Deserialize::deserialize(parcel).map(Box::new) } @@ -888,6 +1189,7 @@ mod tests { #[test] fn test_custom_parcelable() { + #[derive(Default)] struct Custom(u32, bool, String, Vec); impl Serialize for Custom { @@ -900,6 +1202,14 @@ mod tests { } impl Deserialize for Custom { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { Ok(Custom( parcel.read()?, @@ -925,6 +1235,8 @@ mod tests { assert!(custom.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -947,6 +1259,8 @@ mod tests { assert!(bools.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -956,6 +1270,8 @@ mod tests { assert_eq!(parcel.read::().unwrap(), 0); assert_eq!(parcel.read::().unwrap(), 0); assert_eq!(parcel.read::().unwrap(), 1); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -971,12 +1287,17 @@ mod tests { assert!(parcel.write(&u8s[..]).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 4); // 4 items assert_eq!(parcel.read::().unwrap(), 0x752aff65); // bytes + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -986,18 +1307,25 @@ mod tests { let i8s = [-128i8, 127, 42, -117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(parcel.write(&i8s[..]).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert_eq!(parcel.read::().unwrap(), 4); // 4 items assert_eq!(parcel.read::().unwrap(), 0x8b2a7f80); // bytes + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1007,10 +1335,14 @@ mod tests { let u16s = [u16::max_value(), 12_345, 42, 117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(u16s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1020,6 +1352,9 @@ mod tests { assert_eq!(parcel.read::().unwrap(), 12345); // 12,345 assert_eq!(parcel.read::().unwrap(), 42); // 42 assert_eq!(parcel.read::().unwrap(), 117); // 117 + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1030,10 +1365,14 @@ mod tests { let i16s = [i16::max_value(), i16::min_value(), 42, -117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(i16s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1043,6 +1382,9 @@ mod tests { assert_eq!(parcel.read::().unwrap(), 0x8000); // i16::min_value() assert_eq!(parcel.read::().unwrap(), 42); // 42 assert_eq!(parcel.read::().unwrap(), 0xff8b); // -117 + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1053,10 +1395,14 @@ mod tests { let u32s = [u32::max_value(), 12_345, 42, 117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(u32s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1066,6 +1412,9 @@ mod tests { assert_eq!(parcel.read::().unwrap(), 12345); // 12,345 assert_eq!(parcel.read::().unwrap(), 42); // 42 assert_eq!(parcel.read::().unwrap(), 117); // 117 + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1076,10 +1425,14 @@ mod tests { let i32s = [i32::max_value(), i32::min_value(), 42, -117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(i32s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1089,6 +1442,9 @@ mod tests { assert_eq!(parcel.read::().unwrap(), 0x80000000); // i32::min_value() assert_eq!(parcel.read::().unwrap(), 42); // 42 assert_eq!(parcel.read::().unwrap(), 0xffffff8b); // -117 + + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1099,10 +1455,14 @@ mod tests { let u64s = [u64::max_value(), 12_345, 42, 117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(u64s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1113,10 +1473,14 @@ mod tests { let i64s = [i64::max_value(), i64::min_value(), 42, -117]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(i64s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1127,10 +1491,14 @@ mod tests { let f32s = [std::f32::NAN, std::f32::INFINITY, 1.23456789, std::f32::EPSILON]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(f32s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1143,10 +1511,14 @@ mod tests { let f64s = [std::f64::NAN, std::f64::INFINITY, 1.234567890123456789, std::f64::EPSILON]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(f64s.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } @@ -1164,10 +1536,14 @@ mod tests { let strs = [s1, s2, s3, s4]; + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } assert!(strs.serialize(&mut parcel.borrowed()).is_ok()); + // SAFETY: start is less than the current size of the parcel data buffer, because we haven't + // made it any shorter since we got the position. unsafe { assert!(parcel.set_data_position(start).is_ok()); } diff --git a/binder/src/parcel/parcelable_holder.rs b/binder/src/parcel/parcelable_holder.rs index c829d37..f906113 100644 --- a/binder/src/parcel/parcelable_holder.rs +++ b/binder/src/parcel/parcelable_holder.rs @@ -133,8 +133,8 @@ impl ParcelableHolder { } } ParcelableHolderData::Parcel(ref mut parcel) => { + // Safety: 0 should always be a valid position. unsafe { - // Safety: 0 should always be a valid position. parcel.set_data_position(0)?; } @@ -161,6 +161,15 @@ impl ParcelableHolder { } } +impl Clone for ParcelableHolder { + fn clone(&self) -> ParcelableHolder { + ParcelableHolder { + data: Mutex::new(self.data.lock().unwrap().clone()), + stability: self.stability, + } + } +} + impl Serialize for ParcelableHolder { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<(), StatusCode> { parcel.write(&NON_NULL_PARCELABLE_FLAG)?; @@ -169,6 +178,14 @@ impl Serialize for ParcelableHolder { } impl Deserialize for ParcelableHolder { + type UninitType = Self; + fn uninit() -> Self::UninitType { + Self::new(Default::default()) + } + fn from_init(value: Self) -> Self::UninitType { + value + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { let status: i32 = parcel.read()?; if status == NULL_PARCELABLE_FLAG { @@ -197,15 +214,15 @@ impl Parcelable for ParcelableHolder { parcelable.write_to_parcel(parcel)?; let end = parcel.get_data_position(); + // Safety: we got the position from `get_data_position`. unsafe { - // Safety: we got the position from `get_data_position`. parcel.set_data_position(length_start)?; } assert!(end >= data_start); parcel.write(&(end - data_start))?; + // Safety: we got the position from `get_data_position`. unsafe { - // Safety: we got the position from `get_data_position`. parcel.set_data_position(end)?; } @@ -243,11 +260,11 @@ impl Parcelable for ParcelableHolder { new_parcel.append_from(parcel, data_start, data_size)?; *self.data.get_mut().unwrap() = ParcelableHolderData::Parcel(new_parcel); + // Safety: `append_from` checks if `data_size` overflows + // `parcel` and returns `BAD_VALUE` if that happens. We also + // explicitly check for negative and zero `data_size` above, + // so `data_end` is guaranteed to be greater than `data_start`. unsafe { - // Safety: `append_from` checks if `data_size` overflows - // `parcel` and returns `BAD_VALUE` if that happens. We also - // explicitly check for negative and zero `data_size` above, - // so `data_end` is guaranteed to be greater than `data_start`. parcel.set_data_position(data_end)?; } diff --git a/binder/src/proxy.rs b/binder/src/proxy.rs index 254efae..7434e9d 100644 --- a/binder/src/proxy.rs +++ b/binder/src/proxy.rs @@ -32,8 +32,8 @@ use std::convert::TryInto; use std::ffi::{c_void, CStr, CString}; use std::fmt; use std::mem; +use std::os::fd::AsRawFd; use std::os::raw::c_char; -use std::os::unix::io::AsRawFd; use std::ptr; use std::sync::Arc; @@ -49,14 +49,12 @@ impl fmt::Debug for SpIBinder { } } -/// # Safety -/// -/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe +/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is +/// thread-safe. unsafe impl Send for SpIBinder {} -/// # Safety -/// -/// An `SpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe +/// Safety: An `SpIBinder` is an immutable handle to a C++ IBinder, which is +/// thread-safe. unsafe impl Sync for SpIBinder {} impl SpIBinder { @@ -97,11 +95,9 @@ impl SpIBinder { /// Return true if this binder object is hosted in a different process than /// the current one. pub fn is_remote(&self) -> bool { - unsafe { - // Safety: `SpIBinder` guarantees that it always contains a valid - // `AIBinder` pointer. - sys::AIBinder_isRemote(self.as_native()) - } + // Safety: `SpIBinder` guarantees that it always contains a valid + // `AIBinder` pointer. + unsafe { sys::AIBinder_isRemote(self.as_native()) } } /// Try to convert this Binder object into a trait object for the given @@ -116,12 +112,12 @@ impl SpIBinder { /// Return the interface class of this binder object, if associated with /// one. pub fn get_class(&mut self) -> Option { + // Safety: `SpIBinder` guarantees that it always contains a valid + // `AIBinder` pointer. `AIBinder_getClass` returns either a null + // pointer or a valid pointer to an `AIBinder_Class`. After mapping + // null to None, we can safely construct an `InterfaceClass` if the + // pointer was non-null. unsafe { - // Safety: `SpIBinder` guarantees that it always contains a valid - // `AIBinder` pointer. `AIBinder_getClass` returns either a null - // pointer or a valid pointer to an `AIBinder_Class`. After mapping - // null to None, we can safely construct an `InterfaceClass` if the - // pointer was non-null. let class = sys::AIBinder_getClass(self.as_native_mut()); class.as_ref().map(|p| InterfaceClass::from_ptr(p)) } @@ -152,7 +148,8 @@ pub mod unstable_api { /// /// See `SpIBinder::from_raw`. pub unsafe fn new_spibinder(ptr: *mut sys::AIBinder) -> Option { - SpIBinder::from_raw(ptr) + // Safety: The caller makes the same guarantees as this requires. + unsafe { SpIBinder::from_raw(ptr) } } } @@ -171,30 +168,24 @@ pub trait AssociateClass { impl AssociateClass for SpIBinder { fn associate_class(&mut self, class: InterfaceClass) -> bool { - unsafe { - // Safety: `SpIBinder` guarantees that it always contains a valid - // `AIBinder` pointer. An `InterfaceClass` can always be converted - // into a valid `AIBinder_Class` pointer, so these parameters are - // always safe. - sys::AIBinder_associateClass(self.as_native_mut(), class.into()) - } + // Safety: `SpIBinder` guarantees that it always contains a valid + // `AIBinder` pointer. An `InterfaceClass` can always be converted + // into a valid `AIBinder_Class` pointer, so these parameters are + // always safe. + unsafe { sys::AIBinder_associateClass(self.as_native_mut(), class.into()) } } } impl Ord for SpIBinder { fn cmp(&self, other: &Self) -> Ordering { - let less_than = unsafe { - // Safety: SpIBinder always holds a valid `AIBinder` pointer, so - // this pointer is always safe to pass to `AIBinder_lt` (null is - // also safe to pass to this function, but we should never do that). - sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr()) - }; - let greater_than = unsafe { - // Safety: SpIBinder always holds a valid `AIBinder` pointer, so - // this pointer is always safe to pass to `AIBinder_lt` (null is - // also safe to pass to this function, but we should never do that). - sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr()) - }; + // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this + // pointer is always safe to pass to `AIBinder_lt` (null is also safe to + // pass to this function, but we should never do that). + let less_than = unsafe { sys::AIBinder_lt(self.0.as_ptr(), other.0.as_ptr()) }; + // Safety: SpIBinder always holds a valid `AIBinder` pointer, so this + // pointer is always safe to pass to `AIBinder_lt` (null is also safe to + // pass to this function, but we should never do that). + let greater_than = unsafe { sys::AIBinder_lt(other.0.as_ptr(), self.0.as_ptr()) }; if !less_than && !greater_than { Ordering::Equal } else if less_than { @@ -221,10 +212,10 @@ impl Eq for SpIBinder {} impl Clone for SpIBinder { fn clone(&self) -> Self { + // Safety: Cloning a strong reference must increment the reference + // count. We are guaranteed by the `SpIBinder` constructor + // invariants that `self.0` is always a valid `AIBinder` pointer. unsafe { - // Safety: Cloning a strong reference must increment the reference - // count. We are guaranteed by the `SpIBinder` constructor - // invariants that `self.0` is always a valid `AIBinder` pointer. sys::AIBinder_incStrong(self.0.as_ptr()); } Self(self.0) @@ -235,9 +226,9 @@ impl Drop for SpIBinder { // We hold a strong reference to the IBinder in SpIBinder and need to give up // this reference on drop. fn drop(&mut self) { + // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we + // know this pointer is safe to pass to `AIBinder_decStrong` here. unsafe { - // Safety: SpIBinder always holds a valid `AIBinder` pointer, so we - // know this pointer is safe to pass to `AIBinder_decStrong` here. sys::AIBinder_decStrong(self.as_native_mut()); } } @@ -246,26 +237,24 @@ impl Drop for SpIBinder { impl> IBinderInternal for T { fn prepare_transact(&self) -> Result { let mut input = ptr::null_mut(); + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. It is safe to cast from an + // immutable pointer to a mutable pointer here, because + // `AIBinder_prepareTransaction` only calls immutable `AIBinder` + // methods but the parameter is unfortunately not marked as const. + // + // After the call, input will be either a valid, owned `AParcel` + // pointer, or null. let status = unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. It is safe to cast from an - // immutable pointer to a mutable pointer here, because - // `AIBinder_prepareTransaction` only calls immutable `AIBinder` - // methods but the parameter is unfortunately not marked as const. - // - // After the call, input will be either a valid, owned `AParcel` - // pointer, or null. sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input) }; status_result(status)?; - unsafe { - // Safety: At this point, `input` is either a valid, owned `AParcel` - // pointer, or null. `OwnedParcel::from_raw` safely handles both cases, - // taking ownership of the parcel. - Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL) - } + // Safety: At this point, `input` is either a valid, owned `AParcel` + // pointer, or null. `OwnedParcel::from_raw` safely handles both cases, + // taking ownership of the parcel. + unsafe { Parcel::from_raw(input).ok_or(StatusCode::UNEXPECTED_NULL) } } fn submit_transact( @@ -275,23 +264,23 @@ impl> IBinderInternal for T { flags: TransactionFlags, ) -> Result { let mut reply = ptr::null_mut(); + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. Although `IBinder::transact` is + // not a const method, it is still safe to cast our immutable + // pointer to mutable for the call. First, `IBinder::transact` is + // thread-safe, so concurrency is not an issue. The only way that + // `transact` can affect any visible, mutable state in the current + // process is by calling `onTransact` for a local service. However, + // in order for transactions to be thread-safe, this method must + // dynamically lock its data before modifying it. We enforce this + // property in Rust by requiring `Sync` for remotable objects and + // only providing `on_transact` with an immutable reference to + // `self`. + // + // This call takes ownership of the `data` parcel pointer, and + // passes ownership of the `reply` out parameter to its caller. It + // does not affect ownership of the `binder` parameter. let status = unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. Although `IBinder::transact` is - // not a const method, it is still safe to cast our immutable - // pointer to mutable for the call. First, `IBinder::transact` is - // thread-safe, so concurrency is not an issue. The only way that - // `transact` can affect any visible, mutable state in the current - // process is by calling `onTransact` for a local service. However, - // in order for transactions to be thread-safe, this method must - // dynamically lock its data before modifying it. We enforce this - // property in Rust by requiring `Sync` for remotable objects and - // only providing `on_transact` with an immutable reference to - // `self`. - // - // This call takes ownership of the `data` parcel pointer, and - // passes ownership of the `reply` out parameter to its caller. It - // does not affect ownership of the `binder` parameter. sys::AIBinder_transact( self.as_native() as *mut sys::AIBinder, code, @@ -302,45 +291,45 @@ impl> IBinderInternal for T { }; status_result(status)?; - unsafe { - // Safety: `reply` is either a valid `AParcel` pointer or null - // after the call to `AIBinder_transact` above, so we can - // construct a `Parcel` out of it. `AIBinder_transact` passes - // ownership of the `reply` parcel to Rust, so we need to - // construct an owned variant. - Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL) - } + // Safety: `reply` is either a valid `AParcel` pointer or null + // after the call to `AIBinder_transact` above, so we can + // construct a `Parcel` out of it. `AIBinder_transact` passes + // ownership of the `reply` parcel to Rust, so we need to + // construct an owned variant. + unsafe { Parcel::from_raw(reply).ok_or(StatusCode::UNEXPECTED_NULL) } } fn is_binder_alive(&self) -> bool { - unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. - // - // This call does not affect ownership of its pointer parameter. - sys::AIBinder_isAlive(self.as_native()) - } + // Safety: `SpIBinder` guarantees that `self` always contains a valid + // pointer to an `AIBinder`. + // + // This call does not affect ownership of its pointer parameter. + unsafe { sys::AIBinder_isAlive(self.as_native()) } } #[cfg(not(android_vndk))] fn set_requesting_sid(&mut self, enable: bool) { + // Safety: `SpIBinder` guarantees that `self` always contains a valid + // pointer to an `AIBinder`. + // + // This call does not affect ownership of its pointer parameter. unsafe { sys::AIBinder_setRequestingSid(self.as_native_mut(), enable) }; } fn dump(&mut self, fp: &F, args: &[&str]) -> Result<()> { let args: Vec<_> = args.iter().map(|a| CString::new(*a).unwrap()).collect(); let mut arg_ptrs: Vec<_> = args.iter().map(|a| a.as_ptr()).collect(); + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the + // file descriptor parameter is always be a valid open file. The + // `args` pointer parameter is a valid pointer to an array of C + // strings that will outlive the call since `args` lives for the + // whole function scope. + // + // This call does not affect ownership of its binder pointer + // parameter and does not take ownership of the file or args array + // parameters. let status = unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. `AsRawFd` guarantees that the - // file descriptor parameter is always be a valid open file. The - // `args` pointer parameter is a valid pointer to an array of C - // strings that will outlive the call since `args` lives for the - // whole function scope. - // - // This call does not affect ownership of its binder pointer - // parameter and does not take ownership of the file or args array - // parameters. sys::AIBinder_dump( self.as_native_mut(), fp.as_raw_fd(), @@ -353,22 +342,18 @@ impl> IBinderInternal for T { fn get_extension(&mut self) -> Result> { let mut out = ptr::null_mut(); - let status = unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. After this call, the `out` - // parameter will be either null, or a valid pointer to an - // `AIBinder`. - // - // This call passes ownership of the out pointer to its caller - // (assuming it is set to a non-null value). - sys::AIBinder_getExtension(self.as_native_mut(), &mut out) - }; - let ibinder = unsafe { - // Safety: The call above guarantees that `out` is either null or a - // valid, owned pointer to an `AIBinder`, both of which are safe to - // pass to `SpIBinder::from_raw`. - SpIBinder::from_raw(out) - }; + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. After this call, the `out` + // parameter will be either null, or a valid pointer to an + // `AIBinder`. + // + // This call passes ownership of the out pointer to its caller + // (assuming it is set to a non-null value). + let status = unsafe { sys::AIBinder_getExtension(self.as_native_mut(), &mut out) }; + // Safety: The call above guarantees that `out` is either null or a + // valid, owned pointer to an `AIBinder`, both of which are safe to + // pass to `SpIBinder::from_raw`. + let ibinder = unsafe { SpIBinder::from_raw(out) }; status_result(status)?; Ok(ibinder) @@ -377,17 +362,17 @@ impl> IBinderInternal for T { impl> IBinder for T { fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> { + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. `recipient` can always be + // converted into a valid pointer to an + // `AIBinder_DeathRecipient`. + // + // The cookie is also the correct pointer, and by calling new_cookie, + // we have created a new ref-count to the cookie, which linkToDeath + // takes ownership of. Once the DeathRecipient is unlinked for any + // reason (including if this call fails), the onUnlinked callback + // will consume that ref-count. status_result(unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. `recipient` can always be - // converted into a valid pointer to an - // `AIBinder_DeathRecipient`. - // - // The cookie is also the correct pointer, and by calling new_cookie, - // we have created a new ref-count to the cookie, which linkToDeath - // takes ownership of. Once the DeathRecipient is unlinked for any - // reason (including if this call fails), the onUnlinked callback - // will consume that ref-count. sys::AIBinder_linkToDeath( self.as_native_mut(), recipient.as_native_mut(), @@ -397,13 +382,13 @@ impl> IBinder for T { } fn unlink_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> { + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. `recipient` can always be + // converted into a valid pointer to an + // `AIBinder_DeathRecipient`. Any value is safe to pass as the + // cookie, although we depend on this value being set by + // `get_cookie` when the death recipient callback is called. status_result(unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. `recipient` can always be - // converted into a valid pointer to an - // `AIBinder_DeathRecipient`. Any value is safe to pass as the - // cookie, although we depend on this value being set by - // `get_cookie` when the death recipient callback is called. sys::AIBinder_unlinkToDeath( self.as_native_mut(), recipient.as_native_mut(), @@ -413,13 +398,11 @@ impl> IBinder for T { } fn ping_binder(&mut self) -> Result<()> { - let status = unsafe { - // Safety: `SpIBinder` guarantees that `self` always contains a - // valid pointer to an `AIBinder`. - // - // This call does not affect ownership of its pointer parameter. - sys::AIBinder_ping(self.as_native_mut()) - }; + // Safety: `SpIBinder` guarantees that `self` always contains a + // valid pointer to an `AIBinder`. + // + // This call does not affect ownership of its pointer parameter. + let status = unsafe { sys::AIBinder_ping(self.as_native_mut()) }; status_result(status) } } @@ -439,6 +422,14 @@ impl SerializeOption for SpIBinder { impl SerializeArray for SpIBinder {} impl Deserialize for SpIBinder { + type UninitType = Option; + fn uninit() -> Self::UninitType { + Self::UninitType::default() + } + fn from_init(value: Self) -> Self::UninitType { + Some(value) + } + fn deserialize(parcel: &BorrowedParcel<'_>) -> Result { parcel.read_binder().transpose().unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) } @@ -464,35 +455,31 @@ impl fmt::Debug for WpIBinder { } } -/// # Safety -/// -/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe. +/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is +/// thread-safe. unsafe impl Send for WpIBinder {} -/// # Safety -/// -/// A `WpIBinder` is an immutable handle to a C++ IBinder, which is thread-safe. +/// Safety: A `WpIBinder` is an immutable handle to a C++ IBinder, which is +/// thread-safe. unsafe impl Sync for WpIBinder {} impl WpIBinder { /// Create a new weak reference from an object that can be converted into a /// raw `AIBinder` pointer. fn new>(binder: &mut B) -> WpIBinder { - let ptr = unsafe { - // Safety: `SpIBinder` guarantees that `binder` always contains a - // valid pointer to an `AIBinder`. - sys::AIBinder_Weak_new(binder.as_native_mut()) - }; + // Safety: `SpIBinder` guarantees that `binder` always contains a valid + // pointer to an `AIBinder`. + let ptr = unsafe { sys::AIBinder_Weak_new(binder.as_native_mut()) }; Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_new")) } /// Promote this weak reference to a strong reference to the binder object. pub fn promote(&self) -> Option { + // Safety: `WpIBinder` always contains a valid weak reference, so we can + // pass this pointer to `AIBinder_Weak_promote`. Returns either null or + // an AIBinder owned by the caller, both of which are valid to pass to + // `SpIBinder::from_raw`. unsafe { - // Safety: `WpIBinder` always contains a valid weak reference, so we - // can pass this pointer to `AIBinder_Weak_promote`. Returns either - // null or an AIBinder owned by the caller, both of which are valid - // to pass to `SpIBinder::from_raw`. let ptr = sys::AIBinder_Weak_promote(self.0.as_ptr()); SpIBinder::from_raw(ptr) } @@ -501,35 +488,27 @@ impl WpIBinder { impl Clone for WpIBinder { fn clone(&self) -> Self { - let ptr = unsafe { - // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, - // so this pointer is always safe to pass to `AIBinder_Weak_clone` - // (although null is also a safe value to pass to this API). - // - // We get ownership of the returned pointer, so can construct a new - // WpIBinder object from it. - sys::AIBinder_Weak_clone(self.0.as_ptr()) - }; + // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so + // this pointer is always safe to pass to `AIBinder_Weak_clone` + // (although null is also a safe value to pass to this API). + // + // We get ownership of the returned pointer, so can construct a new + // WpIBinder object from it. + let ptr = unsafe { sys::AIBinder_Weak_clone(self.0.as_ptr()) }; Self(ptr::NonNull::new(ptr).expect("Unexpected null pointer from AIBinder_Weak_clone")) } } impl Ord for WpIBinder { fn cmp(&self, other: &Self) -> Ordering { - let less_than = unsafe { - // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, - // so this pointer is always safe to pass to `AIBinder_Weak_lt` - // (null is also safe to pass to this function, but we should never - // do that). - sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr()) - }; - let greater_than = unsafe { - // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, - // so this pointer is always safe to pass to `AIBinder_Weak_lt` - // (null is also safe to pass to this function, but we should never - // do that). - sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr()) - }; + // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so + // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is + // also safe to pass to this function, but we should never do that). + let less_than = unsafe { sys::AIBinder_Weak_lt(self.0.as_ptr(), other.0.as_ptr()) }; + // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so + // this pointer is always safe to pass to `AIBinder_Weak_lt` (null is + // also safe to pass to this function, but we should never do that). + let greater_than = unsafe { sys::AIBinder_Weak_lt(other.0.as_ptr(), self.0.as_ptr()) }; if !less_than && !greater_than { Ordering::Equal } else if less_than { @@ -556,9 +535,9 @@ impl Eq for WpIBinder {} impl Drop for WpIBinder { fn drop(&mut self) { + // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we + // know this pointer is safe to pass to `AIBinder_Weak_delete` here. unsafe { - // Safety: WpIBinder always holds a valid `AIBinder_Weak` pointer, so we - // know this pointer is safe to pass to `AIBinder_Weak_delete` here. sys::AIBinder_Weak_delete(self.0.as_ptr()); } } @@ -566,7 +545,7 @@ impl Drop for WpIBinder { /// Rust wrapper around DeathRecipient objects. /// -/// The cookie in this struct represents an Arc for the owned callback. +/// The cookie in this struct represents an `Arc` for the owned callback. /// This struct owns a ref-count of it, and so does every binder that we /// have been linked with. /// @@ -584,17 +563,13 @@ struct DeathRecipientVtable { cookie_decr_refcount: unsafe extern "C" fn(*mut c_void), } -/// # Safety -/// -/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer -/// to a `Fn` which is `Sync` and `Send` (the cookie field). As +/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and +/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As /// `AIBinder_DeathRecipient` is threadsafe, this structure is too. unsafe impl Send for DeathRecipient {} -/// # Safety -/// -/// A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and a pointer -/// to a `Fn` which is `Sync` and `Send` (the cookie field). As +/// Safety: A `DeathRecipient` is a wrapper around `AIBinder_DeathRecipient` and +/// a pointer to a `Fn` which is `Sync` and `Send` (the cookie field). As /// `AIBinder_DeathRecipient` is threadsafe, this structure is too. unsafe impl Sync for DeathRecipient {} @@ -606,19 +581,17 @@ impl DeathRecipient { F: Fn() + Send + Sync + 'static, { let callback: *const F = Arc::into_raw(Arc::new(callback)); - let recipient = unsafe { - // Safety: The function pointer is a valid death recipient callback. - // - // This call returns an owned `AIBinder_DeathRecipient` pointer - // which must be destroyed via `AIBinder_DeathRecipient_delete` when - // no longer needed. - sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::)) - }; + // Safety: The function pointer is a valid death recipient callback. + // + // This call returns an owned `AIBinder_DeathRecipient` pointer which + // must be destroyed via `AIBinder_DeathRecipient_delete` when no longer + // needed. + let recipient = unsafe { sys::AIBinder_DeathRecipient_new(Some(Self::binder_died::)) }; + // Safety: The function pointer is a valid onUnlinked callback. + // + // All uses of linkToDeath in this file correctly increment the + // ref-count that this onUnlinked callback will decrement. unsafe { - // Safety: The function pointer is a valid onUnlinked callback. - // - // All uses of linkToDeath in this file correctly increment the - // ref-count that this onUnlinked callback will decrement. sys::AIBinder_DeathRecipient_setOnUnlinked( recipient, Some(Self::cookie_decr_refcount::), @@ -640,7 +613,12 @@ impl DeathRecipient { /// /// The caller must handle the returned ref-count correctly. unsafe fn new_cookie(&self) -> *mut c_void { - (self.vtable.cookie_incr_refcount)(self.cookie); + // Safety: `cookie_incr_refcount` points to + // `Self::cookie_incr_refcount`, and `self.cookie` is the cookie for an + // Arc. + unsafe { + (self.vtable.cookie_incr_refcount)(self.cookie); + } // Return a raw pointer with ownership of a ref-count self.cookie @@ -659,13 +637,14 @@ impl DeathRecipient { /// /// # Safety /// - /// The `cookie` parameter must be the cookie for an Arc and + /// The `cookie` parameter must be the cookie for an `Arc` and /// the caller must hold a ref-count to it. unsafe extern "C" fn binder_died(cookie: *mut c_void) where F: Fn() + Send + Sync + 'static, { - let callback = (cookie as *const F).as_ref().unwrap(); + // Safety: The caller promises that `cookie` is for an Arc. + let callback = unsafe { (cookie as *const F).as_ref().unwrap() }; callback(); } @@ -674,34 +653,34 @@ impl DeathRecipient { /// /// # Safety /// - /// The `cookie` parameter must be the cookie for an Arc and + /// The `cookie` parameter must be the cookie for an `Arc` and /// the owner must give up a ref-count to it. unsafe extern "C" fn cookie_decr_refcount(cookie: *mut c_void) where F: Fn() + Send + Sync + 'static, { - drop(Arc::from_raw(cookie as *const F)); + // Safety: The caller promises that `cookie` is for an Arc. + drop(unsafe { Arc::from_raw(cookie as *const F) }); } /// Callback that increments the ref-count. /// /// # Safety /// - /// The `cookie` parameter must be the cookie for an Arc and + /// The `cookie` parameter must be the cookie for an `Arc` and /// the owner must handle the created ref-count properly. unsafe extern "C" fn cookie_incr_refcount(cookie: *mut c_void) where F: Fn() + Send + Sync + 'static, { - let arc = mem::ManuallyDrop::new(Arc::from_raw(cookie as *const F)); + // Safety: The caller promises that `cookie` is for an Arc. + let arc = mem::ManuallyDrop::new(unsafe { Arc::from_raw(cookie as *const F) }); mem::forget(Arc::clone(&arc)); } } -/// # Safety -/// -/// A `DeathRecipient` is always constructed with a valid raw pointer to an -/// `AIBinder_DeathRecipient`, so it is always type-safe to extract this +/// Safety: A `DeathRecipient` is always constructed with a valid raw pointer to +/// an `AIBinder_DeathRecipient`, so it is always type-safe to extract this /// pointer. unsafe impl AsNative for DeathRecipient { fn as_native(&self) -> *const sys::AIBinder_DeathRecipient { @@ -715,18 +694,19 @@ unsafe impl AsNative for DeathRecipient { impl Drop for DeathRecipient { fn drop(&mut self) { + // Safety: `self.recipient` is always a valid, owned + // `AIBinder_DeathRecipient` pointer returned by + // `AIBinder_DeathRecipient_new` when `self` was created. This delete + // method can only be called once when `self` is dropped. unsafe { - // Safety: `self.recipient` is always a valid, owned - // `AIBinder_DeathRecipient` pointer returned by - // `AIBinder_DeathRecipient_new` when `self` was created. This - // delete method can only be called once when `self` is dropped. sys::AIBinder_DeathRecipient_delete(self.recipient); + } - // Safety: We own a ref-count to the cookie, and so does every - // linked binder. This call gives up our ref-count. The linked - // binders should already have given up their ref-count, or should - // do so shortly. - (self.vtable.cookie_decr_refcount)(self.cookie) + // Safety: We own a ref-count to the cookie, and so does every linked + // binder. This call gives up our ref-count. The linked binders should + // already have given up their ref-count, or should do so shortly. + unsafe { + (self.vtable.cookie_decr_refcount)(self.cookie); } } } @@ -746,11 +726,9 @@ pub trait Proxy: Sized + Interface { fn from_binder(binder: SpIBinder) -> Result; } -/// # Safety -/// -/// This is a convenience method that wraps `AsNative` for `SpIBinder` to allow -/// invocation of `IBinder` methods directly from `Interface` objects. It shares -/// the same safety as the implementation for `SpIBinder`. +/// Safety: This is a convenience method that wraps `AsNative` for `SpIBinder` +/// to allow invocation of `IBinder` methods directly from `Interface` objects. +/// It shares the same safety as the implementation for `SpIBinder`. unsafe impl AsNative for T { fn as_native(&self) -> *const sys::AIBinder { self.as_binder().as_native() @@ -765,24 +743,20 @@ unsafe impl AsNative for T { /// exist. pub fn get_service(name: &str) -> Option { let name = CString::new(name).ok()?; - unsafe { - // Safety: `AServiceManager_getService` returns either a null pointer or - // a valid pointer to an owned `AIBinder`. Either of these values is - // safe to pass to `SpIBinder::from_raw`. - SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr())) - } + // Safety: `AServiceManager_getService` returns either a null pointer or a + // valid pointer to an owned `AIBinder`. Either of these values is safe to + // pass to `SpIBinder::from_raw`. + unsafe { SpIBinder::from_raw(sys::AServiceManager_getService(name.as_ptr())) } } /// Retrieve an existing service, or start it if it is configured as a dynamic /// service and isn't yet started. pub fn wait_for_service(name: &str) -> Option { let name = CString::new(name).ok()?; - unsafe { - // Safety: `AServiceManager_waitforService` returns either a null - // pointer or a valid pointer to an owned `AIBinder`. Either of these - // values is safe to pass to `SpIBinder::from_raw`. - SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr())) - } + // Safety: `AServiceManager_waitforService` returns either a null pointer or + // a valid pointer to an owned `AIBinder`. Either of these values is safe to + // pass to `SpIBinder::from_raw`. + unsafe { SpIBinder::from_raw(sys::AServiceManager_waitForService(name.as_ptr())) } } /// Retrieve an existing service for a particular interface, blocking for a few @@ -801,12 +775,10 @@ pub fn wait_for_interface(name: &str) -> Result Result { let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?; - unsafe { - // Safety: `interface` is a valid null-terminated C-style string and is - // only borrowed for the lifetime of the call. The `interface` local - // outlives this call as it lives for the function scope. - Ok(sys::AServiceManager_isDeclared(interface.as_ptr())) - } + // Safety: `interface` is a valid null-terminated C-style string and is only + // borrowed for the lifetime of the call. The `interface` local outlives + // this call as it lives for the function scope. + unsafe { Ok(sys::AServiceManager_isDeclared(interface.as_ptr())) } } /// Retrieve all declared instances for a particular interface @@ -819,11 +791,13 @@ pub fn get_declared_instances(interface: &str) -> Result> { // CString, and outlives this callback. The null handling here is just // to avoid the possibility of unwinding across C code if this crate is // ever compiled with panic=unwind. - if let Some(instances) = opaque.cast::>().as_mut() { + if let Some(instances) = unsafe { opaque.cast::>().as_mut() } { // Safety: instance is a valid null-terminated C string with a // lifetime at least as long as this function, and we immediately // copy it into an owned CString. - instances.push(CStr::from_ptr(instance).to_owned()); + unsafe { + instances.push(CStr::from_ptr(instance).to_owned()); + } } else { eprintln!("Opaque pointer was null in get_declared_instances callback!"); } @@ -831,10 +805,10 @@ pub fn get_declared_instances(interface: &str) -> Result> { let interface = CString::new(interface).or(Err(StatusCode::UNEXPECTED_NULL))?; let mut instances: Vec = vec![]; + // Safety: `interface` and `instances` are borrowed for the length of this + // call and both outlive the call. `interface` is guaranteed to be a valid + // null-terminated C-style string. unsafe { - // Safety: `interface` and `instances` are borrowed for the length of - // this call and both outlive the call. `interface` is guaranteed to be - // a valid null-terminated C-style string. sys::AServiceManager_forEachDeclaredInstance( interface.as_ptr(), &mut instances as *mut _ as *mut c_void, @@ -852,10 +826,8 @@ pub fn get_declared_instances(interface: &str) -> Result> { }) } -/// # Safety -/// -/// `SpIBinder` guarantees that `binder` always contains a valid pointer to an -/// `AIBinder`, so we can trivially extract this pointer here. +/// Safety: `SpIBinder` guarantees that `binder` always contains a valid pointer +/// to an `AIBinder`, so we can trivially extract this pointer here. unsafe impl AsNative for SpIBinder { fn as_native(&self) -> *const sys::AIBinder { self.0.as_ptr() diff --git a/binder/src/state.rs b/binder/src/state.rs index cc18741..8a06274 100644 --- a/binder/src/state.rs +++ b/binder/src/state.rs @@ -22,30 +22,48 @@ use libc::{pid_t, uid_t}; pub struct ProcessState; impl ProcessState { - /// Start the Binder IPC thread pool + /// Starts the Binder IPC thread pool. + /// + /// Starts 1 thread, plus allows the kernel to lazily start up to + /// `num_threads` additional threads as specified by + /// [`set_thread_pool_max_thread_count`](Self::set_thread_pool_max_thread_count). + /// + /// This should be done before creating any Binder client or server. If + /// neither this nor [`join_thread_pool`](Self::join_thread_pool) are + /// called, then some things (such as callbacks and + /// [`IBinder::link_to_death`](crate::IBinder::link_to_death)) will silently + /// not work: the callbacks will be queued but never called as there is no + /// thread to call them on. pub fn start_thread_pool() { + // Safety: Safe FFI unsafe { - // Safety: Safe FFI sys::ABinderProcess_startThreadPool(); } } - /// Set the maximum number of threads that can be started in the threadpool. + /// Sets the maximum number of threads that can be started in the + /// threadpool. /// - /// By default, after startThreadPool is called, this is 15. If it is called - /// additional times, it will only prevent the kernel from starting new - /// threads and will not delete already existing threads. + /// By default, after [`start_thread_pool`](Self::start_thread_pool) is + /// called, this is 15. If it is called additional times, the thread pool + /// size can only be increased. pub fn set_thread_pool_max_thread_count(num_threads: u32) { + // Safety: Safe FFI unsafe { - // Safety: Safe FFI sys::ABinderProcess_setThreadPoolMaxThreadCount(num_threads); } } - /// Block on the Binder IPC thread pool + /// Blocks on the Binder IPC thread pool by adding the current thread to the + /// pool. + /// + /// Note that this adds the current thread in addition to those that are + /// created by + /// [`set_thread_pool_max_thread_count`](Self::set_thread_pool_max_thread_count) + /// and [`start_thread_pool`](Self::start_thread_pool). pub fn join_thread_pool() { + // Safety: Safe FFI unsafe { - // Safety: Safe FFI sys::ABinderProcess_joinThreadPool(); } } @@ -68,10 +86,8 @@ impl ThreadState { /// \return calling uid or the current process's UID if this thread isn't /// processing a transaction. pub fn get_calling_uid() -> uid_t { - unsafe { - // Safety: Safe FFI - sys::AIBinder_getCallingUid() - } + // Safety: Safe FFI + unsafe { sys::AIBinder_getCallingUid() } } /// This returns the calling PID assuming that this thread is called from a @@ -85,18 +101,19 @@ impl ThreadState { /// dies and is replaced with another process with elevated permissions and /// the same PID. /// + /// Warning: oneway transactions do not receive PID. Even if you expect + /// a transaction to be synchronous, a misbehaving client could send it + /// as a synchronous call and result in a 0 PID here. Additionally, if + /// there is a race and the calling process dies, the PID may still be + /// 0 for a synchronous call. + /// /// Available since API level 29. /// /// \return calling pid or the current process's PID if this thread isn't /// processing a transaction. - /// - /// If the transaction being processed is a oneway transaction, then this - /// method will return 0. pub fn get_calling_pid() -> pid_t { - unsafe { - // Safety: Safe FFI - sys::AIBinder_getCallingPid() - } + // Safety: Safe FFI + unsafe { sys::AIBinder_getCallingPid() } } /// Determine whether the current thread is currently executing an incoming transaction. @@ -104,10 +121,8 @@ impl ThreadState { /// \return true if the current thread is currently executing an incoming transaction, and false /// otherwise. pub fn is_handling_transaction() -> bool { - unsafe { - // Safety: Safe FFI - sys::AIBinder_isHandlingTransaction() - } + // Safety: Safe FFI + unsafe { sys::AIBinder_isHandlingTransaction() } } /// This function makes the client's security context available to the diff --git a/example/src/IRemoteService.rs b/example/src/IRemoteService.rs index a64270d..4b6eccc 100644 --- a/example/src/IRemoteService.rs +++ b/example/src/IRemoteService.rs @@ -1,6 +1,8 @@ +/* + * This file is auto-generated. DO NOT MODIFY. + * Using: aidl --lang=rust IRemoteService.aidl -o src -I . + */ #![forbid(unsafe_code)] -// NOTE: use cfg_attr(rustfmt, rustfmt_skip) instead of rustfmt::skip -// #![rustfmt::skip] #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(non_upper_case_globals)] #![allow(non_snake_case)] @@ -16,8 +18,8 @@ declare_binder_interface! { } pub trait IRemoteService: binder::Interface + Send { fn get_descriptor() -> &'static str where Self: Sized { "IRemoteService" } - fn getPid(&self) -> binder::Result; - fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()>; + fn r#getPid(&self) -> binder::Result; + fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()>; fn getDefaultImpl() -> IRemoteServiceDefaultRef where Self: Sized { DEFAULT_IMPL.lock().unwrap().clone() } @@ -27,14 +29,14 @@ pub trait IRemoteService: binder::Interface + Send { } pub trait IRemoteServiceAsync

: binder::Interface + Send { fn get_descriptor() -> &'static str where Self: Sized { "IRemoteService" } - fn getPid<'a>(&'a self) -> binder::BoxFuture<'a, binder::Result>; - fn basicTypes<'a>(&'a self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &'a str) -> binder::BoxFuture<'a, binder::Result<()>>; + fn r#getPid<'a>(&'a self) -> binder::BoxFuture<'a, binder::Result>; + fn r#basicTypes<'a>(&'a self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &'a str) -> binder::BoxFuture<'a, binder::Result<()>>; } #[::async_trait::async_trait] pub trait IRemoteServiceAsyncServer: binder::Interface + Send { fn get_descriptor() -> &'static str where Self: Sized { "IRemoteService" } - async fn getPid(&self) -> binder::Result; - async fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()>; + async fn r#getPid(&self) -> binder::Result; + async fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()>; } impl BnRemoteService { /// Create a new async binder service. @@ -47,20 +49,20 @@ impl BnRemoteService { _inner: T, _rt: R, } - impl binder::Interface for Wrapper where T: binder::Interface, R: Send + Sync { + impl binder::Interface for Wrapper where T: binder::Interface, R: Send + Sync + 'static { fn as_binder(&self) -> binder::SpIBinder { self._inner.as_binder() } - fn dump(&self, _file: &std::fs::File, _args: &[&std::ffi::CStr]) -> std::result::Result<(), binder::StatusCode> { self._inner.dump(_file, _args) } + fn dump(&self, _writer: &mut dyn std::io::Write, _args: &[&std::ffi::CStr]) -> std::result::Result<(), binder::StatusCode> { self._inner.dump(_writer, _args) } } impl IRemoteService for Wrapper where T: IRemoteServiceAsyncServer + Send + Sync + 'static, R: binder::binder_impl::BinderAsyncRuntime + Send + Sync + 'static, { - fn getPid(&self) -> binder::Result { - self._rt.block_on(self._inner.getPid()) + fn r#getPid(&self) -> binder::Result { + self._rt.block_on(self._inner.r#getPid()) } - fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { - self._rt.block_on(self._inner.basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString)) + fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { + self._rt.block_on(self._inner.r#basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString)) } } let wrapped = Wrapper { _inner: inner, _rt: rt }; @@ -68,22 +70,19 @@ impl BnRemoteService { } } pub trait IRemoteServiceDefault: Send + Sync { - fn getPid(&self) -> binder::Result { + fn r#getPid(&self) -> binder::Result { Err(binder::StatusCode::UNKNOWN_TRANSACTION.into()) } - fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { + fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { Err(binder::StatusCode::UNKNOWN_TRANSACTION.into()) } } pub mod transactions { - pub const getPid: binder::binder_impl::TransactionCode = binder::binder_impl::FIRST_CALL_TRANSACTION + 0; - pub const basicTypes: binder::binder_impl::TransactionCode = binder::binder_impl::FIRST_CALL_TRANSACTION + 1; + pub const r#getPid: binder::binder_impl::TransactionCode = binder::binder_impl::FIRST_CALL_TRANSACTION + 0; + pub const r#basicTypes: binder::binder_impl::TransactionCode = binder::binder_impl::FIRST_CALL_TRANSACTION + 1; } pub type IRemoteServiceDefaultRef = Option>; -use lazy_static::lazy_static; -lazy_static! { - static ref DEFAULT_IMPL: std::sync::Mutex = std::sync::Mutex::new(None); -} +static DEFAULT_IMPL: std::sync::Mutex = std::sync::Mutex::new(None); impl BpRemoteService { fn build_parcel_getPid(&self) -> binder::Result { let mut aidl_data = self.binder.prepare_transact()?; @@ -92,7 +91,7 @@ impl BpRemoteService { fn read_response_getPid(&self, _aidl_reply: std::result::Result) -> binder::Result { if let Err(binder::StatusCode::UNKNOWN_TRANSACTION) = _aidl_reply { if let Some(_aidl_default_impl) = ::getDefaultImpl() { - return _aidl_default_impl.getPid(); + return _aidl_default_impl.r#getPid(); } } let _aidl_reply = _aidl_reply?; @@ -114,7 +113,7 @@ impl BpRemoteService { fn read_response_basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str, _aidl_reply: std::result::Result) -> binder::Result<()> { if let Err(binder::StatusCode::UNKNOWN_TRANSACTION) = _aidl_reply { if let Some(_aidl_default_impl) = ::getDefaultImpl() { - return _aidl_default_impl.basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString); + return _aidl_default_impl.r#basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString); } } let _aidl_reply = _aidl_reply?; @@ -124,39 +123,39 @@ impl BpRemoteService { } } impl IRemoteService for BpRemoteService { - fn getPid(&self) -> binder::Result { + fn r#getPid(&self) -> binder::Result { let _aidl_data = self.build_parcel_getPid()?; - let _aidl_reply = self.binder.submit_transact(transactions::getPid, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL); + let _aidl_reply = self.binder.submit_transact(transactions::r#getPid, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL); self.read_response_getPid(_aidl_reply) } - fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { + fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { let _aidl_data = self.build_parcel_basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString)?; - let _aidl_reply = self.binder.submit_transact(transactions::basicTypes, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL); + let _aidl_reply = self.binder.submit_transact(transactions::r#basicTypes, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL); self.read_response_basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString, _aidl_reply) } } impl IRemoteServiceAsync

for BpRemoteService { - fn getPid<'a>(&'a self) -> binder::BoxFuture<'a, binder::Result> { + fn r#getPid<'a>(&'a self) -> binder::BoxFuture<'a, binder::Result> { let _aidl_data = match self.build_parcel_getPid() { Ok(_aidl_data) => _aidl_data, Err(err) => return Box::pin(std::future::ready(Err(err))), }; let binder = self.binder.clone(); P::spawn( - move || binder.submit_transact(transactions::getPid, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL), + move || binder.submit_transact(transactions::r#getPid, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL), move |_aidl_reply| async move { self.read_response_getPid(_aidl_reply) } ) } - fn basicTypes<'a>(&'a self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &'a str) -> binder::BoxFuture<'a, binder::Result<()>> { + fn r#basicTypes<'a>(&'a self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &'a str) -> binder::BoxFuture<'a, binder::Result<()>> { let _aidl_data = match self.build_parcel_basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString) { Ok(_aidl_data) => _aidl_data, Err(err) => return Box::pin(std::future::ready(Err(err))), }; let binder = self.binder.clone(); P::spawn( - move || binder.submit_transact(transactions::basicTypes, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL), + move || binder.submit_transact(transactions::r#basicTypes, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL), move |_aidl_reply| async move { self.read_response_basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString, _aidl_reply) } @@ -164,13 +163,13 @@ impl IRemoteServiceAsync

for BpRemoteService { } } impl IRemoteService for binder::binder_impl::Binder { - fn getPid(&self) -> binder::Result { self.0.getPid() } - fn basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { self.0.basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString) } + fn r#getPid(&self) -> binder::Result { self.0.r#getPid() } + fn r#basicTypes(&self, _arg_anInt: i32, _arg_aLong: i64, _arg_aBoolean: bool, _arg_aFloat: f32, _arg_aDouble: f64, _arg_aString: &str) -> binder::Result<()> { self.0.r#basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, _arg_aString) } } fn on_transact(_aidl_service: &dyn IRemoteService, _aidl_code: binder::binder_impl::TransactionCode, _aidl_data: &binder::binder_impl::BorrowedParcel<'_>, _aidl_reply: &mut binder::binder_impl::BorrowedParcel<'_>) -> std::result::Result<(), binder::StatusCode> { match _aidl_code { - transactions::getPid => { - let _aidl_return = _aidl_service.getPid(); + transactions::r#getPid => { + let _aidl_return = _aidl_service.r#getPid(); match &_aidl_return { Ok(_aidl_return) => { _aidl_reply.write(&binder::Status::from(binder::StatusCode::OK))?; @@ -180,14 +179,14 @@ fn on_transact(_aidl_service: &dyn IRemoteService, _aidl_code: binder::binder_im } Ok(()) } - transactions::basicTypes => { + transactions::r#basicTypes => { let _arg_anInt: i32 = _aidl_data.read()?; let _arg_aLong: i64 = _aidl_data.read()?; let _arg_aBoolean: bool = _aidl_data.read()?; let _arg_aFloat: f32 = _aidl_data.read()?; let _arg_aDouble: f64 = _aidl_data.read()?; let _arg_aString: String = _aidl_data.read()?; - let _aidl_return = _aidl_service.basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, &_arg_aString); + let _aidl_return = _aidl_service.r#basicTypes(_arg_anInt, _arg_aLong, _arg_aBoolean, _arg_aFloat, _arg_aDouble, &_arg_aString); match &_aidl_return { Ok(_aidl_return) => { _aidl_reply.write(&binder::Status::from(binder::StatusCode::OK))?; @@ -200,5 +199,5 @@ fn on_transact(_aidl_service: &dyn IRemoteService, _aidl_code: binder::binder_im } } pub(crate) mod mangled { - pub use super::IRemoteService as _14_IRemoteService; + pub use super::r#IRemoteService as _14_IRemoteService; } diff --git a/unstructured_parcelable_example/Cargo.toml b/unstructured_parcelable_example/Cargo.toml new file mode 100644 index 0000000..bf3986f --- /dev/null +++ b/unstructured_parcelable_example/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "unstructured-parcelable-binder-example" +version = "0.1.0" +edition = "2021" + +[dependencies] +anyhow = "1.0.69" +binder = { package = "binder_ndk", path = "../binder" } +binder_tokio = { path = "../binder_tokio" } +clap = { version = "4.1.7", features = ["derive"] } +tokio = { version = "1.25.0", features = ["full"] } +async-trait = "0.1.64" +lazy_static = "1.4.0" diff --git a/unstructured_parcelable_example/aidl/IMySimpleParcelableService.aidl b/unstructured_parcelable_example/aidl/IMySimpleParcelableService.aidl new file mode 100644 index 0000000..32bb6c9 --- /dev/null +++ b/unstructured_parcelable_example/aidl/IMySimpleParcelableService.aidl @@ -0,0 +1,8 @@ +package com.example.mysimpleparcelableservice; + +import com.example.mysimpleparcelableservice.MySimpleParcelable; + +interface IMySimpleParcelableService { + /** Generate a MySimpleParcelable message. */ + MySimpleParcelable sendMySimpleParcelable(String name, int years); +} \ No newline at end of file diff --git a/unstructured_parcelable_example/aidl/MySimpleParcelable.aidl b/unstructured_parcelable_example/aidl/MySimpleParcelable.aidl new file mode 100644 index 0000000..ba266bd --- /dev/null +++ b/unstructured_parcelable_example/aidl/MySimpleParcelable.aidl @@ -0,0 +1,3 @@ +package com.example.mysimpleparcelableservice; + +parcelable MySimpleParcelable rust_type "my_simple_parcelable::MySimpleParcelable"; \ No newline at end of file diff --git a/unstructured_parcelable_example/src/IMySimpleParcelableService.rs b/unstructured_parcelable_example/src/IMySimpleParcelableService.rs new file mode 100644 index 0000000..0de5426 --- /dev/null +++ b/unstructured_parcelable_example/src/IMySimpleParcelableService.rs @@ -0,0 +1,142 @@ +/* + * This file is auto-generated. DO NOT MODIFY. + * Using: out/host/linux-x86/bin/aidl --lang=rust -Weverything -Wno-missing-permission-annotation --min_sdk_version current --ninja -d out/soong/.intermediates/external/rust/my_simple_parcelable_service/aidl/com.example.mysimpleparcelableservice-rust-source/gen/com/example/mysimpleparcelableservice/IMySimpleParcelableService.rs.d -o out/soong/.intermediates/external/rust/my_simple_parcelable_service/aidl/com.example.mysimpleparcelableservice-rust-source/gen -Nexternal/rust/my_simple_parcelable_service/aidl external/rust/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl + */ +#![forbid(unsafe_code)] +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(non_upper_case_globals)] +#![allow(non_snake_case)] +#[allow(unused_imports)] use binder::binder_impl::IBinderInternal; +use binder::declare_binder_interface; +use crate::my_simple_parcelable; +declare_binder_interface! { + IMySimpleParcelableService["com.example.mysimpleparcelableservice.IMySimpleParcelableService"] { + native: BnMySimpleParcelableService(on_transact), + proxy: BpMySimpleParcelableService { + }, + async: IMySimpleParcelableServiceAsync, + } +} +pub trait IMySimpleParcelableService: binder::Interface + Send { + fn get_descriptor() -> &'static str where Self: Sized { "com.example.mysimpleparcelableservice.IMySimpleParcelableService" } + fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result; + fn getDefaultImpl() -> IMySimpleParcelableServiceDefaultRef where Self: Sized { + DEFAULT_IMPL.lock().unwrap().clone() + } + fn setDefaultImpl(d: IMySimpleParcelableServiceDefaultRef) -> IMySimpleParcelableServiceDefaultRef where Self: Sized { + std::mem::replace(&mut *DEFAULT_IMPL.lock().unwrap(), d) + } +} +pub trait IMySimpleParcelableServiceAsync

: binder::Interface + Send { + fn get_descriptor() -> &'static str where Self: Sized { "com.example.mysimpleparcelableservice.IMySimpleParcelableService" } + fn r#sendMySimpleParcelable<'a>(&'a self, _arg_name: &'a str, _arg_years: i32) -> binder::BoxFuture<'a, binder::Result>; +} +#[::async_trait::async_trait] +pub trait IMySimpleParcelableServiceAsyncServer: binder::Interface + Send { + fn get_descriptor() -> &'static str where Self: Sized { "com.example.mysimpleparcelableservice.IMySimpleParcelableService" } + async fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result; +} +impl BnMySimpleParcelableService { + /// Create a new async binder service. + pub fn new_async_binder(inner: T, rt: R, features: binder::BinderFeatures) -> binder::Strong + where + T: IMySimpleParcelableServiceAsyncServer + binder::Interface + Send + Sync + 'static, + R: binder::binder_impl::BinderAsyncRuntime + Send + Sync + 'static, + { + struct Wrapper { + _inner: T, + _rt: R, + } + impl binder::Interface for Wrapper where T: binder::Interface, R: Send + Sync + 'static { + fn as_binder(&self) -> binder::SpIBinder { self._inner.as_binder() } + fn dump(&self, _writer: &mut dyn std::io::Write, _args: &[&std::ffi::CStr]) -> std::result::Result<(), binder::StatusCode> { self._inner.dump(_writer, _args) } + } + impl IMySimpleParcelableService for Wrapper + where + T: IMySimpleParcelableServiceAsyncServer + Send + Sync + 'static, + R: binder::binder_impl::BinderAsyncRuntime + Send + Sync + 'static, + { + fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result { + self._rt.block_on(self._inner.r#sendMySimpleParcelable(_arg_name, _arg_years)) + } + } + let wrapped = Wrapper { _inner: inner, _rt: rt }; + Self::new_binder(wrapped, features) + } +} +pub trait IMySimpleParcelableServiceDefault: Send + Sync { + fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result { + Err(binder::StatusCode::UNKNOWN_TRANSACTION.into()) + } +} +pub mod transactions { + pub const r#sendMySimpleParcelable: binder::binder_impl::TransactionCode = binder::binder_impl::FIRST_CALL_TRANSACTION + 0; +} +pub type IMySimpleParcelableServiceDefaultRef = Option>; +static DEFAULT_IMPL: std::sync::Mutex = std::sync::Mutex::new(None); +impl BpMySimpleParcelableService { + fn build_parcel_sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result { + let mut aidl_data = self.binder.prepare_transact()?; + aidl_data.write(_arg_name)?; + aidl_data.write(&_arg_years)?; + Ok(aidl_data) + } + fn read_response_sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32, _aidl_reply: std::result::Result) -> binder::Result { + if let Err(binder::StatusCode::UNKNOWN_TRANSACTION) = _aidl_reply { + if let Some(_aidl_default_impl) = ::getDefaultImpl() { + return _aidl_default_impl.r#sendMySimpleParcelable(_arg_name, _arg_years); + } + } + let _aidl_reply = _aidl_reply?; + let _aidl_status: binder::Status = _aidl_reply.read()?; + if !_aidl_status.is_ok() { return Err(_aidl_status); } + let _aidl_return: my_simple_parcelable::MySimpleParcelable = _aidl_reply.read()?; + Ok(_aidl_return) + } +} +impl IMySimpleParcelableService for BpMySimpleParcelableService { + fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result { + let _aidl_data = self.build_parcel_sendMySimpleParcelable(_arg_name, _arg_years)?; + let _aidl_reply = self.binder.submit_transact(transactions::r#sendMySimpleParcelable, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL); + self.read_response_sendMySimpleParcelable(_arg_name, _arg_years, _aidl_reply) + } +} +impl IMySimpleParcelableServiceAsync

for BpMySimpleParcelableService { + fn r#sendMySimpleParcelable<'a>(&'a self, _arg_name: &'a str, _arg_years: i32) -> binder::BoxFuture<'a, binder::Result> { + let _aidl_data = match self.build_parcel_sendMySimpleParcelable(_arg_name, _arg_years) { + Ok(_aidl_data) => _aidl_data, + Err(err) => return Box::pin(std::future::ready(Err(err))), + }; + let binder = self.binder.clone(); + P::spawn( + move || binder.submit_transact(transactions::r#sendMySimpleParcelable, _aidl_data, binder::binder_impl::FLAG_PRIVATE_LOCAL), + move |_aidl_reply| async move { + self.read_response_sendMySimpleParcelable(_arg_name, _arg_years, _aidl_reply) + } + ) + } +} +impl IMySimpleParcelableService for binder::binder_impl::Binder { + fn r#sendMySimpleParcelable(&self, _arg_name: &str, _arg_years: i32) -> binder::Result { self.0.r#sendMySimpleParcelable(_arg_name, _arg_years) } +} +fn on_transact(_aidl_service: &dyn IMySimpleParcelableService, _aidl_code: binder::binder_impl::TransactionCode, _aidl_data: &binder::binder_impl::BorrowedParcel<'_>, _aidl_reply: &mut binder::binder_impl::BorrowedParcel<'_>) -> std::result::Result<(), binder::StatusCode> { + match _aidl_code { + transactions::r#sendMySimpleParcelable => { + let _arg_name: String = _aidl_data.read()?; + let _arg_years: i32 = _aidl_data.read()?; + let _aidl_return = _aidl_service.r#sendMySimpleParcelable(&_arg_name, _arg_years); + match &_aidl_return { + Ok(_aidl_return) => { + _aidl_reply.write(&binder::Status::from(binder::StatusCode::OK))?; + _aidl_reply.write(_aidl_return)?; + } + Err(_aidl_status) => _aidl_reply.write(_aidl_status)? + } + Ok(()) + } + _ => Err(binder::StatusCode::UNKNOWN_TRANSACTION) + } +} +pub(crate) mod mangled { + pub use super::r#IMySimpleParcelableService as _3_com_7_example_25_mysimpleparcelableservice_26_IMySimpleParcelableService; +} diff --git a/unstructured_parcelable_example/src/cli.rs b/unstructured_parcelable_example/src/cli.rs new file mode 100644 index 0000000..f88da23 --- /dev/null +++ b/unstructured_parcelable_example/src/cli.rs @@ -0,0 +1,33 @@ +use anyhow::Result; +use clap::Parser; + +use crate::{client, server}; + +#[derive(Parser, Debug)] +#[command(author, about, long_about = None)] +struct Args { + #[command(subcommand)] + command: Commands, +} + +#[derive(clap::Subcommand, Debug)] +enum Commands { + Server, + + Client, +} + +pub fn run() -> Result<()> { + let cli = Args::parse(); + + let result = match cli.command { + Commands::Server => server::run(), + Commands::Client => client::run(), + }; + + if let Err(e) = &result { + println!("Error: {}", e); + } + + result +} diff --git a/unstructured_parcelable_example/src/client.rs b/unstructured_parcelable_example/src/client.rs new file mode 100644 index 0000000..f4a88fc --- /dev/null +++ b/unstructured_parcelable_example/src/client.rs @@ -0,0 +1,13 @@ +use binder::Strong; +use crate::IMySimpleParcelableService::{IMySimpleParcelableService}; +use crate::my_simple_parcelable::MySimpleParcelable; + +pub fn run() -> anyhow::Result<()> { + let my_service: Strong = binder::get_interface("myservice").unwrap(); + println!("Do sendMySimpleParcelable()"); + let name = "Franklin"; + let my_simplest_parcelable = my_service.sendMySimpleParcelable(name, 99 as i32).expect("Failed to retrieve simple parcelable"); + println!("Got simple parcelable: {:?}", my_simplest_parcelable); + println!("Done!"); + Ok(()) +} diff --git a/unstructured_parcelable_example/src/main.rs b/unstructured_parcelable_example/src/main.rs new file mode 100644 index 0000000..b7020d3 --- /dev/null +++ b/unstructured_parcelable_example/src/main.rs @@ -0,0 +1,10 @@ +#![allow(non_snake_case)] +mod cli; +mod server; +mod client; +mod IMySimpleParcelableService; +mod my_simple_parcelable; + +fn main() -> anyhow::Result<()>{ + cli::run() +} \ No newline at end of file diff --git a/unstructured_parcelable_example/src/my_simple_parcelable.rs b/unstructured_parcelable_example/src/my_simple_parcelable.rs new file mode 100644 index 0000000..643e3af --- /dev/null +++ b/unstructured_parcelable_example/src/my_simple_parcelable.rs @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2023, 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 implementation of `MySimpleParcelable`. + +use binder::{ + binder_impl::{BorrowedParcel, UnstructuredParcelable}, + impl_deserialize_for_unstructured_parcelable, impl_serialize_for_unstructured_parcelable, + StatusCode, +}; + +/// Rust implementation of `MySimpleParcelable`. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct MySimpleParcelable { + /// The name of the parcelable. + pub name: String, + /// The number of the parcelable. + pub number: i32, +} + +impl UnstructuredParcelable for MySimpleParcelable { + fn write_to_parcel(&self, parcel: &mut BorrowedParcel) -> Result<(), StatusCode> { + parcel.write(&self.name)?; + parcel.write(&self.number)?; + Ok(()) + } + + fn from_parcel(parcel: &BorrowedParcel) -> Result { + let name = parcel.read()?; + let number = parcel.read()?; + Ok(Self { name, number }) + } +} + +impl_deserialize_for_unstructured_parcelable!(MySimpleParcelable); +impl_serialize_for_unstructured_parcelable!(MySimpleParcelable); diff --git a/unstructured_parcelable_example/src/server.rs b/unstructured_parcelable_example/src/server.rs new file mode 100644 index 0000000..900770b --- /dev/null +++ b/unstructured_parcelable_example/src/server.rs @@ -0,0 +1,33 @@ +use binder::BinderFeatures; +use crate::IMySimpleParcelableService::{BnMySimpleParcelableService, IMySimpleParcelableService}; +use crate::my_simple_parcelable::MySimpleParcelable; +use binder::{Interface, Result as BinderResult, + binder_impl::{BorrowedParcel, UnstructuredParcelable}, +}; + +pub struct MyService; + +impl Interface for MyService {} + +impl IMySimpleParcelableService for MyService { + fn sendMySimpleParcelable(&self, name: &str, years: i32) -> BinderResult { + println!("sending simple parcelable for name: {} and years: {}", name, years); + + let my_simplest_parcelable = MySimpleParcelable { + name: name.to_owned(), + number: years + }; + + Ok(my_simplest_parcelable) + } +} + +pub fn run() -> anyhow::Result<()> { + let my_service = MyService; + let my_service_binder = BnMySimpleParcelableService::new_binder(my_service, BinderFeatures::default()); + binder::add_service("myservice", my_service_binder.as_binder()) + .expect("Failed to register service?"); + println!("Running!"); + binder::ProcessState::join_thread_pool(); + anyhow::Ok(()) +} From f527beee5da3cd04668e221cff3cca0bebcd7747 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Sat, 10 Feb 2024 00:45:19 -0500 Subject: [PATCH 2/8] Adding documentation on how to generate AIDLs from AOSP and integrate them into a normal Cargo build. --- README.md | 72 ++++++++++++++++++- .../my_simple_parcelable_service/Android.bp | 10 +++ .../aidl/Android.bp | 24 +++++++ .../IMySimpleParcelableService.aidl | 8 +++ .../MySimpleParcelable.aidl | 3 + .../rust/my_simple_parcelable.rs | 50 +++++++++++++ .../my_simple_parcelable_service/src/lib.rs | 19 +++++ 7 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/Android.bp create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/Android.bp create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/MySimpleParcelable.aidl create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/rust/my_simple_parcelable.rs create mode 100644 unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/src/lib.rs diff --git a/README.md b/README.md index a5befa6..ce2129a 100644 --- a/README.md +++ b/README.md @@ -80,4 +80,74 @@ We use ndk-build to build the stub so, `ANDROID_NDK_HOME` must be set in your en CANNOT LINK EXECUTABLE "/data/local/tmp/binder_tests-acf830ec15b8864e": cannot locate symbol "AIBinder_DeathRecipient_setOnUnlinked" referenced by "/data/local/tmp/binder_tests-acf830ec15b8864e"... ``` - This is because your android version is too low, the source is from android-mainline. You can try in avd, with Android U \ No newline at end of file + This is because your android version is too low, the source is from android-mainline. You can try in avd, with Android U + +## Using AIDLs with UnstructuredParcelable + +### Generate Rust code from AIDL from the AOSP + +1. Create AIDL + any `UnstructuredParcelable` you want to use + in your AIDL interface + * Reference `unstructured_parcelable_example/aosp_module_to_generate_aidl` + +Note the structure: +* `my_simple_parcelable_service/` + * `Android.bp`: Soong build file + * `aidl/` + * `Android.bp`: Soong build file + * `com/example/mysimpleparcelableservice` + * `IMySimpleParcelableService.aidl`: The service interface, note + we are including `MySimpleParcelable` as a return parameter + * `MySimpleParcelable.aidl`: Essentially a bit of a "wrapper" + which points to the actual definition of the + UnstructuredParcelable in the `rust` folder, see below + * `rust/` + * `my_simple_parcelable.rs`: Note that we have to impl + the `UnstructuredParcelable` trait. + * `src/` + * `lib.rs`: Implementation of the service. We only need to impl + it because Soong will not let us make the AIDL directly. Note + that this is not actually used at all. + +2. Use Soong to build. You need to put the above folder under + `/external/rust/` + +```bash +m libmysimpleparcelableservice +``` + +Expected output +``` +============================================ +PLATFORM_VERSION_CODENAME=VanillaIceCream +PLATFORM_VERSION=VanillaIceCream +PRODUCT_INCLUDE_TAGS=com.android.mainline mainline_module_prebuilt_nightly +TARGET_PRODUCT=aosp_x86_64 +TARGET_BUILD_VARIANT=eng +TARGET_ARCH=x86_64 +TARGET_ARCH_VARIANT=x86_64 +TARGET_2ND_ARCH=x86 +TARGET_2ND_ARCH_VARIANT=x86_64 +HOST_OS=linux +HOST_OS_EXTRA=Linux-5.15.0-79-generic-x86_64-Ubuntu-20.04.6-LTS +HOST_CROSS_OS=windows +BUILD_ID=MAIN +OUT_DIR=out +============================================ +[ 22% 8/36 1m15s remaining] AIDL Rust external/rust/my_simple_parcelable_service/aidl/com/example/mysimp +Compiling IDL... +GenerateRust: out/soong/.intermediates/external/rust/my_simple_parcelable_service/aidl/com.example.mysim +pleparcelableservice-rust-source/gen/com/example/mysimpleparcelableservice/IMySimpleParcelableService.rs +[100% 36/36 1m14s remaining] Install: out/target/product/generic_x86_64/system/lib64/libmysimpleparcelab + +#### build completed successfully (02:19 (mm:ss)) #### +``` + +3. Navigate to Soong intermediate build folder and scoop up + generated Rust code. + * In this case it was located at: + * `/out/soong/.intermediates/external/rust/my_simple_parcelable_service/aidl/com.example.mysimpleparcelableservice-rust-source/gen/com/example/mysimpleparcelableservice/IMySimpleParcelableService.rs` + +4. Assemble the pieces in your Rust project + * Copy in your generated Rust source for the interface from step 3 + * Copy in the Rust source of your `UnstructuredParcelable`s from step 1 \ No newline at end of file diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/Android.bp b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/Android.bp new file mode 100644 index 0000000..e196113 --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/Android.bp @@ -0,0 +1,10 @@ +rust_library { + name: "libmysimpleparcelableservice", + srcs: ["src/lib.rs"], + crate_name: "mysimpleparcelableservice", + rustlibs: [ + "libmy_simple_parcelable_rust", + "com.example.mysimpleparcelableservice-rust", + "libbinder_rs", + ], +} diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/Android.bp b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/Android.bp new file mode 100644 index 0000000..ce5d098 --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/Android.bp @@ -0,0 +1,24 @@ +rust_library { + name: "libmy_simple_parcelable_rust", + crate_name: "my_simple_parcelable", + srcs: [ + "com/example/mysimpleparcelableservice/rust/my_simple_parcelable.rs", + ], + rustlibs: [ + "libbinder_rs", + ], +} + +aidl_interface { + name: "com.example.mysimpleparcelableservice", + srcs: ["com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl"], + unstable: true, + backend: { + rust: { + enabled: true, + additional_rustlibs: [ + "libmy_simple_parcelable_rust", + ], + }, + }, +} \ No newline at end of file diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl new file mode 100644 index 0000000..32bb6c9 --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/IMySimpleParcelableService.aidl @@ -0,0 +1,8 @@ +package com.example.mysimpleparcelableservice; + +import com.example.mysimpleparcelableservice.MySimpleParcelable; + +interface IMySimpleParcelableService { + /** Generate a MySimpleParcelable message. */ + MySimpleParcelable sendMySimpleParcelable(String name, int years); +} \ No newline at end of file diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/MySimpleParcelable.aidl b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/MySimpleParcelable.aidl new file mode 100644 index 0000000..ba266bd --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/MySimpleParcelable.aidl @@ -0,0 +1,3 @@ +package com.example.mysimpleparcelableservice; + +parcelable MySimpleParcelable rust_type "my_simple_parcelable::MySimpleParcelable"; \ No newline at end of file diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/rust/my_simple_parcelable.rs b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/rust/my_simple_parcelable.rs new file mode 100644 index 0000000..c928d81 --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/aidl/com/example/mysimpleparcelableservice/rust/my_simple_parcelable.rs @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2023, 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 implementation of `MySimpleParcelable`. + + +use binder::{ + binder_impl::{BorrowedParcel, UnstructuredParcelable}, + impl_deserialize_for_unstructured_parcelable, impl_serialize_for_unstructured_parcelable, + StatusCode, +}; + +/// Rust implementation of `MySimpleParcelable`. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct MySimpleParcelable { + /// The name of the parcelable. + pub name: String, + /// The number of the parcelable. + pub number: i32, +} + +impl UnstructuredParcelable for MySimpleParcelable { + fn write_to_parcel(&self, parcel: &mut BorrowedParcel) -> Result<(), StatusCode> { + parcel.write(&self.name)?; + parcel.write(&self.number)?; + Ok(()) + } + + fn from_parcel(parcel: &BorrowedParcel) -> Result { + let name = parcel.read()?; + let number = parcel.read()?; + Ok(Self { name, number }) + } +} + +impl_deserialize_for_unstructured_parcelable!(MySimpleParcelable); +impl_serialize_for_unstructured_parcelable!(MySimpleParcelable); diff --git a/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/src/lib.rs b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/src/lib.rs new file mode 100644 index 0000000..a69a7c7 --- /dev/null +++ b/unstructured_parcelable_example/aosp_module_to_generate_aidl/my_simple_parcelable_service/src/lib.rs @@ -0,0 +1,19 @@ +use my_simple_parcelable::MySimpleParcelable; + +/// Implementation of the `IMySimpleParcelableService` AIDL interface. +use com_example_mysimpleparcelableservice::aidl::com::example::mysimpleparcelableservice::IMySimpleParcelableService::IMySimpleParcelableService; +use com_example_mysimpleparcelableservice::binder; + +/// The `IMySimpleParcelableService` implementation. +pub struct MySimpleParcelableService; + +impl binder::Interface for MySimpleParcelableService {} + +impl IMySimpleParcelableService for MySimpleParcelableService { + fn sendMySimpleParcelable(&self, name: &str, years: i32) -> binder::Result { + Ok(MySimpleParcelable { + name: name.to_string(), + number: years + }) + } +} From 2db501c39ebc890f543a89a26c23ee5881002ec8 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Sat, 10 Feb 2024 01:06:55 -0500 Subject: [PATCH 3/8] Updated README.md with more detailed instructions on how to get necessary build tools (NDK, building own libbinder_ndk.so from AOSP, cargo-ndk). --- README.md | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index ce2129a..191295a 100644 --- a/README.md +++ b/README.md @@ -2,12 +2,57 @@ The stub `libbinder_ndk.so` in ndk's sysroot hides some apis, so that we cannot link our `libbiner_rs` to it. However, we can rebuild a stub so to make linker happy. The stub source is generated from symbols.txt, whose contents are extracted from prebuilt `libbinder_ndk.so` from aosp-mainline. +The libbinder_ndk.so is introduced in api-29, you need to compile it with specified target api level. +The compilation won't fail if api is lower than 29, because we build the dynamic library ourselves. -The libbinder_ndk.so is introduced in api-29, you need to compile it with specified target api level. The compilation won't failed if api is lower than 29, because we build the dynamic library on ourselves. +## Adding Android targets via rustup +For each Android target you wish to build for you must e.g. -We use ndk-build to build the stub so, `ANDROID_NDK_HOME` must be set in your env ! +```bash +rustup target add \ + aarch64-linux-android \ + armv7-linux-androideabi \ + x86_64-linux-android \ + i686-linux-android +``` + +## Building using the `cargo-ndk` tooling + +`cargo-ndk` simplifies the process of building libraries and binaries +for Android targets. + +1. Install Android Studio + +2. Install the NDK from within Android Studio + +3. Install `cargo-ndk` with: + +```bash +cargo install cargo-ndk +``` + +We use `cargo ndk` to build the stub so, `ANDROID_NDK_HOME` must be set in your env ! + +## Building your own `libbinder_ndk.so` from the AOSP + +If you follow the steps below in `Generate Rust code from AIDL from the AOSP` +you will get a `libbinder_ndk.so` build 'for free' for your target architecture. +Copy that `libbinder_ndk.so` from where it's built in the aosp root to where +it should belong in the path to your NDK install. + +In my case I copied it from: +``` +/out/target/product/generic_x86_64/system/lib64/libbinder_ndk.so +``` +to +``` +~/Android/Sdk/ndk//toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/x86_64-linux-android/29/libbinder_ndk.so +``` +since I was going to compile with platform API level 29 and for x86_64. + +## Side note on where the sources are from > `sys/src/include_*` from [platform/frameworks/native/libs/binder/ndk](https://android.googlesource.com/platform/frameworks/native/+/refs/heads/master/libs/binder/ndk/) @@ -111,6 +156,15 @@ Note the structure: 2. Use Soong to build. You need to put the above folder under `/external/rust/` + * As per normal when building out of the AOSP, you'll need to + choose your target architecture, so do the following steps first + from the aosp root: + * `source build/envsetup.sh` + * `lunch ` + * Recall you can type `lunch` by itself to get a listing + of available targets + +You may then proceed with building: ```bash m libmysimpleparcelableservice @@ -150,4 +204,12 @@ pleparcelableservice-rust-source/gen/com/example/mysimpleparcelableservice/IMySi 4. Assemble the pieces in your Rust project * Copy in your generated Rust source for the interface from step 3 - * Copy in the Rust source of your `UnstructuredParcelable`s from step 1 \ No newline at end of file + * Copy in the Rust source of your `UnstructuredParcelable`s from step 1 + +5. Build + +```bash +cargo ndk -t x86_64 --platform=29 --bindgen build +``` + +6. Remaining steps to copy / run example same as above \ No newline at end of file From 99e60f2a9f725f33feb20f4216fc6f0637f729c9 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Sat, 10 Feb 2024 14:33:19 -0500 Subject: [PATCH 4/8] Renamed binder_ndk -> binder --- Cargo.lock | 24 +++++++++++----------- binder/Cargo.toml | 2 +- binder_tokio/Cargo.toml | 2 +- example/Cargo.toml | 2 +- unstructured_parcelable_example/Cargo.toml | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8aed0d8..e76378f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -25,28 +25,28 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "binder" +version = "0.2.0" +dependencies = [ + "binder_ndk_sys", + "downcast-rs", + "libc", +] + [[package]] name = "binder-example" version = "0.1.0" dependencies = [ "anyhow", "async-trait", - "binder_ndk", + "binder", "binder_tokio", "clap", "lazy_static", "tokio", ] -[[package]] -name = "binder_ndk" -version = "0.2.0" -dependencies = [ - "binder_ndk_sys", - "downcast-rs", - "libc", -] - [[package]] name = "binder_ndk_sys" version = "0.2.0" @@ -59,7 +59,7 @@ dependencies = [ name = "binder_tokio" version = "0.2.0" dependencies = [ - "binder_ndk", + "binder", "tokio", "tokio-runtime", ] @@ -591,7 +591,7 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", - "binder_ndk", + "binder", "binder_tokio", "clap", "lazy_static", diff --git a/binder/Cargo.toml b/binder/Cargo.toml index 9f3da10..58c0102 100644 --- a/binder/Cargo.toml +++ b/binder/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "binder_ndk" +name = "binder" edition.workspace = true version.workspace = true diff --git a/binder_tokio/Cargo.toml b/binder_tokio/Cargo.toml index a54f18c..b1af4a6 100644 --- a/binder_tokio/Cargo.toml +++ b/binder_tokio/Cargo.toml @@ -10,6 +10,6 @@ license.workspace = true rust-version.workspace = true [dependencies] -binder = { package = "binder_ndk", path = "../binder", version = "0" } +binder = { package = "binder", path = "../binder", version = "0" } tokio = { version = "1.25.0", features = ["full"] } tokio-runtime = "0.0.0" diff --git a/example/Cargo.toml b/example/Cargo.toml index dad0254..473330e 100644 --- a/example/Cargo.toml +++ b/example/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] anyhow = "1.0.69" -binder = { package = "binder_ndk", path = "../binder" } +binder = { package = "binder", path = "../binder" } binder_tokio = { path = "../binder_tokio" } clap = { version = "4.1.7", features = ["derive"] } tokio = { version = "1.25.0", features = ["full"] } diff --git a/unstructured_parcelable_example/Cargo.toml b/unstructured_parcelable_example/Cargo.toml index bf3986f..5742f2c 100644 --- a/unstructured_parcelable_example/Cargo.toml +++ b/unstructured_parcelable_example/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] anyhow = "1.0.69" -binder = { package = "binder_ndk", path = "../binder" } +binder = { package = "binder", path = "../binder" } binder_tokio = { path = "../binder_tokio" } clap = { version = "4.1.7", features = ["derive"] } tokio = { version = "1.25.0", features = ["full"] } From 3880cc065f5c70dea7141a0aa0f564dd68962625 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Fri, 16 Feb 2024 12:45:38 -0500 Subject: [PATCH 5/8] Exposing Java Binder <-> NDK AIBinder functions within Rust. --- binder_ndk_sys/src/BinderBindings.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/binder_ndk_sys/src/BinderBindings.hpp b/binder_ndk_sys/src/BinderBindings.hpp index 65fa2ca..2d8af38 100644 --- a/binder_ndk_sys/src/BinderBindings.hpp +++ b/binder_ndk_sys/src/BinderBindings.hpp @@ -15,6 +15,7 @@ */ #include +#include #include #include #include From d12e094879002d97a2e5ace77aef5b44dcd40d26 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Fri, 16 Feb 2024 15:33:54 -0500 Subject: [PATCH 6/8] Adding functions to convert between Java Binder objects and NDK binder objects. --- Cargo.lock | 78 +++++++++++++++++++++++++++++++++++++++ binder_ndk_sys/Cargo.toml | 3 ++ binder_ndk_sys/build.rs | 67 +++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index e76378f..d4e9a54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -53,6 +53,7 @@ version = "0.2.0" dependencies = [ "anyhow", "bindgen", + "jni", ] [[package]] @@ -104,6 +105,12 @@ version = "1.0.79" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f" +[[package]] +name = "cesu8" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d43a04d8753f35258c91f8ec639f792891f748a1edbd759cf1dcea3382ad83c" + [[package]] name = "cexpr" version = "0.6.0" @@ -167,6 +174,16 @@ dependencies = [ "os_str_bytes", ] +[[package]] +name = "combine" +version = "4.6.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35ed6e9d84f0b51a7f52daf1c7d71dd136fd7a3f41a8462b8cdb8c78d920fad4" +dependencies = [ + "bytes", + "memchr", +] + [[package]] name = "downcast-rs" version = "1.2.0" @@ -249,6 +266,28 @@ dependencies = [ "windows-sys 0.45.0", ] +[[package]] +name = "jni" +version = "0.21.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a87aa2bb7d2af34197c04845522473242e1aa17c12f4935d5856491a7fb8c97" +dependencies = [ + "cesu8", + "cfg-if", + "combine", + "jni-sys", + "log", + "thiserror", + "walkdir", + "windows-sys 0.45.0", +] + +[[package]] +name = "jni-sys" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8eaf4bc02d17cbdd7ff4c7438cafcdf7fb9a4613313ad11b4f8fefe7d3fa0130" + [[package]] name = "lazy_static" version = "1.4.0" @@ -479,6 +518,15 @@ dependencies = [ "windows-sys 0.45.0", ] +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "scopeguard" version = "1.1.0" @@ -542,6 +590,26 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "thiserror" +version = "1.0.39" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a5ab016db510546d856297882807df8da66a16fb8c4101cb8b30054b0d5b2d9c" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.39" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5420d42e90af0c38c3290abcca25b9b3bdf379fc9f55c528f53a269d9c9a267e" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "tokio" version = "1.25.0" @@ -604,6 +672,16 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "walkdir" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d71d857dc86794ca4c280d616f7da00d2dbfd8cd788846559a6813e6aa4b54ee" +dependencies = [ + "same-file", + "winapi-util", +] + [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" diff --git a/binder_ndk_sys/Cargo.toml b/binder_ndk_sys/Cargo.toml index beaf43d..a25ce99 100644 --- a/binder_ndk_sys/Cargo.toml +++ b/binder_ndk_sys/Cargo.toml @@ -9,6 +9,9 @@ repository.workspace = true license.workspace = true rust-version.workspace = true +[dependencies] +jni = "0.21.1" + [build-dependencies] bindgen = "0.64.0" anyhow = "1" diff --git a/binder_ndk_sys/build.rs b/binder_ndk_sys/build.rs index 321818b..59c921f 100644 --- a/binder_ndk_sys/build.rs +++ b/binder_ndk_sys/build.rs @@ -17,6 +17,9 @@ rust-version = "1.67" [lib] crate-type = ["cdylib"] + +[dependencies] +jni = "0.21.1" "#; fn build_stub() -> Result<()> { @@ -96,6 +99,70 @@ fn main() { // Tell cargo to invalidate the built crate whenever any of the // included header files changed. .parse_callbacks(Box::new(bindgen::CargoCallbacks)) + // Tell bindgen to not pull in all the types from + // .blocklist_file("") + .blocklist_type("va_list") + .blocklist_type("jboolean") + .blocklist_type("jbyte") + .blocklist_type("jchar") + .blocklist_type("jshort") + .blocklist_type("jint") + .blocklist_type("jlong") + .blocklist_type("jfloat") + .blocklist_type("jdouble") + .blocklist_type("jsize") + .blocklist_type("jobject") + .blocklist_type("jclass") + .blocklist_type("jstring") + .blocklist_type("jarray") + .blocklist_type("jobjectArray") + .blocklist_type("jbooleanArray") + .blocklist_type("jbyteArray") + .blocklist_type("jcharArray") + .blocklist_type("jshortArray") + .blocklist_type("jintArray") + .blocklist_type("jlongArray") + .blocklist_type("jfloatArray") + .blocklist_type("jdoubleArray") + .blocklist_type("jthrowable") + .blocklist_type("jweak") + .blocklist_type("jfieldID") + .blocklist_type("jmethodID") + .blocklist_type("JNIEnv") + .blocklist_type("JavaVM") + .blocklist_type("_jobject") + .blocklist_type("_jclass") + .blocklist_type("_jstring") + .blocklist_type("_jarray") + .blocklist_type("_jobjectArray") + .blocklist_type("_jbooleanArray") + .blocklist_type("_jbyteArray") + .blocklist_type("_jcharArray") + .blocklist_type("_jshortArray") + .blocklist_type("_jintArray") + .blocklist_type("_jlongArray") + .blocklist_type("_jfloatArray") + .blocklist_type("_jdoubleArray") + .blocklist_type("_jthrowable") + .blocklist_type("_jfieldID") + .blocklist_type("_jmethodID") + .blocklist_type("jvalue") + .blocklist_type("JNINativeMethod") + .blocklist_type("JNINativeInterface") + .blocklist_type("_JNIEnv") + .blocklist_type("JNIInvokeInterface") + .blocklist_type("_JavaVM") + .blocklist_type("jobjectRefType") + .blocklist_function("JNI_GetDefaultJavaVMInitArgs") + .blocklist_function("JNI_CreateJavaVM") + .blocklist_function("JNI_GetCreatedJavaVMs") + .blocklist_function("JNI_OnLoad") + .blocklist_function("JNI_OnUnload") + // Tell bindgen to use the types from the jni crate + .raw_line("use jni::JNIEnv;") + .raw_line("use jni::sys::jobject;") + // Get verbose output + .clang_arg("-v") // Finish the builder and generate the bindings. .generate() // Unwrap the Result and panic on failure. From 8c9cba4d815061b536f8d91ebd9b5d1f0cb107d3 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Fri, 16 Feb 2024 15:54:51 -0500 Subject: [PATCH 7/8] Changing use jni::JNIEnv to jni::sys::JNIEnv to reflect what's needed by AIBinder_fromJavaBinder (I think...) --- binder_ndk_sys/build.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/binder_ndk_sys/build.rs b/binder_ndk_sys/build.rs index 59c921f..514f170 100644 --- a/binder_ndk_sys/build.rs +++ b/binder_ndk_sys/build.rs @@ -99,7 +99,8 @@ fn main() { // Tell cargo to invalidate the built crate whenever any of the // included header files changed. .parse_callbacks(Box::new(bindgen::CargoCallbacks)) - // Tell bindgen to not pull in all the types from + // Tell bindgen to not pull in all the types from would be ideal, but it's not working + // to keep moving we manually blocklist all conflicting types // .blocklist_file("") .blocklist_type("va_list") .blocklist_type("jboolean") @@ -159,7 +160,7 @@ fn main() { .blocklist_function("JNI_OnLoad") .blocklist_function("JNI_OnUnload") // Tell bindgen to use the types from the jni crate - .raw_line("use jni::JNIEnv;") + .raw_line("use jni::sys::JNIEnv;") .raw_line("use jni::sys::jobject;") // Get verbose output .clang_arg("-v") From 842d7225e605f97724365acfd688cabb8870b126 Mon Sep 17 00:00:00 2001 From: Peter LeVasseur Date: Tue, 20 Feb 2024 12:17:05 -0500 Subject: [PATCH 8/8] Updated unit test to use std::io::Write instead of File matching upstream change. --- Cargo.lock | 9 +++++++++ Cargo.toml | 4 ++-- tests/Cargo.toml | 2 +- tests/src/main.rs | 23 +++++++++++++++++++---- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4e9a54..e3a275e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -47,6 +47,15 @@ dependencies = [ "tokio", ] +[[package]] +name = "binder-tests" +version = "0.1.0" +dependencies = [ + "binder", + "binder_tokio", + "tokio", +] + [[package]] name = "binder_ndk_sys" version = "0.2.0" diff --git a/Cargo.toml b/Cargo.toml index 8081d2d..dc9ead4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,8 +4,8 @@ members = [ "binder_tokio", "binder_ndk_sys", "example", - "unstructured_parcelable_example" -# "tests", + "unstructured_parcelable_example", + "tests", ] exclude = ["target/*"] diff --git a/tests/Cargo.toml b/tests/Cargo.toml index b3c8ed9..2abbc82 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -4,6 +4,6 @@ version = "0.1.0" edition = "2021" [dependencies] -binder = { package = "binder_ndk", path = "../binder" } +binder = { package = "binder", path = "../binder" } binder_tokio = { path = "../binder_tokio" } tokio = { version = "1.25.0", features = ["full"] } diff --git a/tests/src/main.rs b/tests/src/main.rs index b002bf7..ae06408 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -26,19 +26,20 @@ use binder::binder_impl::{ use std::convert::{TryFrom, TryInto}; use std::ffi::CStr; -use std::fs::File; +use std::io::Write; use std::sync::Mutex; /// Name of service runner. /// /// Must match the binary name in Android.bp -const RUST_SERVICE_BINARY: &str = "binder_tests"; +/// FYI: There is a delta here in the stand-alone crate. We name it what we called in the Cargo.toml +const RUST_SERVICE_BINARY: &str = "binder-tests"; /// Binary to run a test service. /// /// This needs to be in a separate process from the tests, so we spawn this /// binary as a child, providing the service name as an argument. -pub fn main() -> Result<(), &'static str> { +fn main() -> Result<(), &'static str> { // Ensure that we can handle all transactions on the main thread. binder::ProcessState::set_thread_pool_max_thread_count(0); binder::ProcessState::start_thread_pool(); @@ -118,7 +119,7 @@ impl TryFrom for TestTransactionCode { } impl Interface for TestService { - fn dump(&self, _file: &File, args: &[&CStr]) -> Result<(), StatusCode> { + fn dump(&self, _writer: &mut dyn Write, args: &[&CStr]) -> Result<(), StatusCode> { let mut dump_args = self.dump_args.lock().unwrap(); dump_args.extend(args.iter().map(|s| s.to_str().unwrap().to_owned())); Ok(()) @@ -354,6 +355,15 @@ declare_binder_enum! { } } +declare_binder_enum! { + #[deprecated(since = "1.0.0")] + TestDeprecatedEnum : [i32; 3] { + FOO = 1, + BAR = 2, + BAZ = 3, + } +} + #[cfg(test)] mod tests { use std::ffi::CStr; @@ -536,6 +546,11 @@ mod tests { } fn get_expected_selinux_context() -> &'static str { + // SAFETY: The pointer we pass to `getcon` is valid because it comes from a reference, and + // `getcon` doesn't retain it after it returns. If `getcon` succeeds then `out_ptr` will + // point to a valid C string, otherwise it will remain null. We check for null, so the + // pointer we pass to `CStr::from_ptr` must be a valid pointer to a C string. There is a + // memory leak as we don't call `freecon`, but that's fine because this is just a test. // unsafe { // let mut out_ptr = ptr::null_mut(); // assert_eq!(selinux_sys::getcon(&mut out_ptr), 0);