diff options
| -rw-r--r-- | libs/binder/rust/src/binder.rs | 32 | ||||
| -rw-r--r-- | libs/binder/rust/src/parcel.rs | 15 | ||||
| -rw-r--r-- | libs/binder/rust/src/proxy.rs | 30 | ||||
| -rw-r--r-- | libs/binder/rust/tests/integration.rs | 23 |
4 files changed, 84 insertions, 16 deletions
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index dd0c7b82e0..41ceee5697 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -152,20 +152,46 @@ pub trait IBinderInternal: IBinder { /// available. fn get_extension(&mut self) -> Result<Option<SpIBinder>>; + /// Create a Parcel that can be used with `submit_transact`. + fn prepare_transact(&self) -> Result<Parcel>; + /// Perform a generic operation with the object. /// + /// The provided [`Parcel`] must have been created by a call to + /// `prepare_transact` on the same binder. + /// + /// # Arguments + /// + /// * `code` - Transaction code for the operation. + /// * `data` - [`Parcel`] with input data. + /// * `flags` - Transaction flags, e.g. marking the transaction as + /// asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY)). + fn submit_transact( + &self, + code: TransactionCode, + data: Parcel, + flags: TransactionFlags, + ) -> Result<Parcel>; + + /// Perform a generic operation with the object. This is a convenience + /// method that internally calls `prepare_transact` followed by + /// `submit_transact. + /// /// # Arguments /// * `code` - Transaction code for the operation - /// * `data` - [`Parcel`] with input data - /// * `reply` - Optional [`Parcel`] for reply data /// * `flags` - Transaction flags, e.g. marking the transaction as /// asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY)) + /// * `input_callback` A callback for building the `Parcel`. fn transact<F: FnOnce(&mut Parcel) -> Result<()>>( &self, code: TransactionCode, flags: TransactionFlags, input_callback: F, - ) -> Result<Parcel>; + ) -> Result<Parcel> { + let mut parcel = self.prepare_transact()?; + input_callback(&mut parcel)?; + self.submit_transact(code, parcel, flags) + } } /// Interface of binder local or remote objects. diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs index a0e991c9fd..dad89ec97d 100644 --- a/libs/binder/rust/src/parcel.rs +++ b/libs/binder/rust/src/parcel.rs @@ -25,6 +25,7 @@ use std::cell::RefCell; use std::convert::TryInto; use std::mem::ManuallyDrop; use std::ptr; +use std::fmt; mod file_descriptor; mod parcelable; @@ -96,6 +97,13 @@ impl Parcel { let _ = ManuallyDrop::new(self); ptr } + + pub(crate) fn is_owned(&self) -> bool { + match *self { + Self::Owned(_) => true, + Self::Borrowed(_) => false, + } + } } // Data serialization methods @@ -412,6 +420,13 @@ impl Drop for Parcel { } } +impl fmt::Debug for Parcel { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Parcel") + .finish() + } +} + #[cfg(test)] impl Parcel { /// Create a new parcel tied to a bogus binder. TESTING ONLY! diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs index cdd7c081d0..ce7370989d 100644 --- a/libs/binder/rust/src/proxy.rs +++ b/libs/binder/rust/src/proxy.rs @@ -233,13 +233,7 @@ impl Drop for SpIBinder { } impl<T: AsNative<sys::AIBinder>> IBinderInternal for T { - /// Perform a binder transaction - fn transact<F: FnOnce(&mut Parcel) -> Result<()>>( - &self, - code: TransactionCode, - flags: TransactionFlags, - input_callback: F, - ) -> Result<Parcel> { + fn prepare_transact(&self) -> Result<Parcel> { let mut input = ptr::null_mut(); let status = unsafe { // Safety: `SpIBinder` guarantees that `self` always contains a @@ -252,15 +246,25 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T { // pointer, or null. sys::AIBinder_prepareTransaction(self.as_native() as *mut sys::AIBinder, &mut input) }; + status_result(status)?; - let mut input = unsafe { + + unsafe { // Safety: At this point, `input` is either a valid, owned `AParcel` // pointer, or null. `Parcel::owned` safely handles both cases, // taking ownership of the parcel. - Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL)? - }; - input_callback(&mut input)?; + Parcel::owned(input).ok_or(StatusCode::UNEXPECTED_NULL) + } + } + + fn submit_transact( + &self, + code: TransactionCode, + data: Parcel, + flags: TransactionFlags, + ) -> Result<Parcel> { let mut reply = ptr::null_mut(); + assert!(data.is_owned()); let status = unsafe { // Safety: `SpIBinder` guarantees that `self` always contains a // valid pointer to an `AIBinder`. Although `IBinder::transact` is @@ -275,13 +279,13 @@ impl<T: AsNative<sys::AIBinder>> IBinderInternal for T { // only providing `on_transact` with an immutable reference to // `self`. // - // This call takes ownership of the `input` parcel pointer, and + // 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, - &mut input.into_raw(), + &mut data.into_raw(), &mut reply, flags, ) diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index da8907decb..777bd6ba2c 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -637,4 +637,27 @@ mod tests { assert!(!(service1 > service1)); assert_eq!(service1 < service2, !(service2 < service1)); } + + #[test] + fn binder_parcel_mixup() { + let service1 = BnTest::new_binder( + TestService::new("testing_service1"), + BinderFeatures::default(), + ); + let service2 = BnTest::new_binder( + TestService::new("testing_service2"), + BinderFeatures::default(), + ); + + let service1 = service1.as_binder(); + let service2 = service2.as_binder(); + + let parcel = service1.prepare_transact().unwrap(); + let res = service2.submit_transact(super::TestTransactionCode::Test as binder::TransactionCode, parcel, 0); + + match res { + Ok(_) => panic!("submit_transact should fail"), + Err(err) => assert_eq!(err, binder::StatusCode::BAD_VALUE), + } + } } |