Merge "Standardise safety comments for unsafe blocks, and add some more." into main (f56c8b1e) · Commits · e / os / android_frameworks_native

libs/binder/rust/rpcbinder/src/server.rs

+15 −6

Original line number	Diff line number	Diff line
		@@ -33,9 +33,9 @@ foreign_type! {
		pub struct RpcServerRef;
		}

		/// SAFETY - The opaque handle can be cloned freely.
		/// SAFETY: The opaque handle can be cloned freely.
		unsafe impl Send for RpcServer {}
		/// SAFETY - The underlying C++ RpcServer class is thread-safe.
		/// SAFETY: The underlying C++ RpcServer class is thread-safe.
		unsafe impl Sync for RpcServer {}

		impl RpcServer {
		@@ -59,7 +59,10 @@ impl RpcServer {
		/// Creates a binder RPC server, serving the supplied binder service implementation on the given
		/// socket file descriptor. The socket should be bound to an address before calling this
		/// function.
		pub fn new_bound_socket(mut service: SpIBinder, socket_fd: OwnedFd) -> Result<RpcServer, Error> {
		pub fn new_bound_socket(
		mut service: SpIBinder,
		socket_fd: OwnedFd,
		) -> Result<RpcServer, Error> {
		let service = service.as_native_mut();

		// SAFETY: Service ownership is transferring to the server and won't be valid afterward.
		@@ -67,7 +70,8 @@ impl RpcServer {
		// The server takes ownership of the socket FD.
		unsafe {
		Self::checked_from_ptr(binder_rpc_unstable_bindgen::ARpcServer_newBoundSocket(
		service, socket_fd.into_raw_fd(),
		service,
		socket_fd.into_raw_fd(),
		))
		}
		}
		@@ -120,7 +124,9 @@ impl RpcServer {
		if ptr.is_null() {
		return Err(Error::new(ErrorKind::Other, "Failed to start server"));
		}
		Ok(RpcServer::from_ptr(ptr))
		// SAFETY: Our caller must pass us a valid or null pointer, and we've checked that it's not
		// null.
		Ok(unsafe { RpcServer::from_ptr(ptr) })
		}
		}

		@@ -130,7 +136,7 @@ impl RpcServerRef {
		&self,
		modes: &[FileDescriptorTransportMode],
		) {
		// SAFETY - Does not keep the pointer after returning does, nor does it
		// SAFETY: Does not keep the pointer after returning does, nor does it
		// read past its boundary. Only passes the 'self' pointer as an opaque handle.
		unsafe {
		binder_rpc_unstable_bindgen::ARpcServer_setSupportedFileDescriptorTransportModes(
		@@ -143,18 +149,21 @@ impl RpcServerRef {

		/// Starts a new background thread and calls join(). Returns immediately.
		pub fn start(&self) {
		// SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
		unsafe { binder_rpc_unstable_bindgen::ARpcServer_start(self.as_ptr()) };
		}

		/// Joins the RpcServer thread. The call blocks until the server terminates.
		/// This must be called from exactly one thread.
		pub fn join(&self) {
		// SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
		unsafe { binder_rpc_unstable_bindgen::ARpcServer_join(self.as_ptr()) };
		}

		/// Shuts down the running RpcServer. Can be called multiple times and from
		/// multiple threads. Called automatically during drop().
		pub fn shutdown(&self) -> Result<(), Error> {
		// SAFETY: RpcServerRef wraps a valid pointer to an ARpcServer.
		if unsafe { binder_rpc_unstable_bindgen::ARpcServer_shutdown(self.as_ptr()) } {
		Ok(())
		} else {

libs/binder/rust/rpcbinder/src/session.rs

+8 −8

Original line number	Diff line number	Diff line
		@@ -36,15 +36,15 @@ foreign_type! {
		pub struct RpcSessionRef;
		}

		/// SAFETY - The opaque handle can be cloned freely.
		/// SAFETY: The opaque handle can be cloned freely.
		unsafe impl Send for RpcSession {}
		/// SAFETY - The underlying C++ RpcSession class is thread-safe.
		/// SAFETY: The underlying C++ RpcSession class is thread-safe.
		unsafe impl Sync for RpcSession {}

		impl RpcSession {
		/// Allocates a new RpcSession object.
		pub fn new() -> RpcSession {
		// SAFETY - Takes ownership of the returned handle, which has correct refcount.
		// SAFETY: Takes ownership of the returned handle, which has correct refcount.
		unsafe { RpcSession::from_ptr(binder_rpc_unstable_bindgen::ARpcSession_new()) }
		}
		}
		@@ -58,7 +58,7 @@ impl Default for RpcSession {
		impl RpcSessionRef {
		/// Sets the file descriptor transport mode for this session.
		pub fn set_file_descriptor_transport_mode(&self, mode: FileDescriptorTransportMode) {
		// SAFETY - Only passes the 'self' pointer as an opaque handle.
		// SAFETY: Only passes the 'self' pointer as an opaque handle.
		unsafe {
		binder_rpc_unstable_bindgen::ARpcSession_setFileDescriptorTransportMode(
		self.as_ptr(),
		@@ -69,7 +69,7 @@ impl RpcSessionRef {

		/// Sets the maximum number of incoming threads.
		pub fn set_max_incoming_threads(&self, threads: usize) {
		// SAFETY - Only passes the 'self' pointer as an opaque handle.
		// SAFETY: Only passes the 'self' pointer as an opaque handle.
		unsafe {
		binder_rpc_unstable_bindgen::ARpcSession_setMaxIncomingThreads(self.as_ptr(), threads)
		};
		@@ -77,7 +77,7 @@ impl RpcSessionRef {

		/// Sets the maximum number of outgoing connections.
		pub fn set_max_outgoing_connections(&self, connections: usize) {
		// SAFETY - Only passes the 'self' pointer as an opaque handle.
		// SAFETY: Only passes the 'self' pointer as an opaque handle.
		unsafe {
		binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections(
		self.as_ptr(),
		@@ -210,10 +210,10 @@ impl RpcSessionRef {
		type RequestFd<'a> = &'a mut dyn FnMut() -> Option<RawFd>;

		unsafe extern "C" fn request_fd_wrapper(param: *mut c_void) -> c_int {
		let request_fd_ptr = param as *mut RequestFd;
		// SAFETY: This is only ever called by RpcPreconnectedClient, within the lifetime of the
		// BinderFdFactory reference, with param being a properly aligned non-null pointer to an
		// initialized instance.
		let request_fd_ptr = param as *mut RequestFd;
		let request_fd = request_fd_ptr.as_mut().unwrap();
		let request_fd = unsafe { request_fd_ptr.as_mut().unwrap() };
		request_fd().unwrap_or(-1)
		}

libs/binder/rust/src/binder.rs

+25 −27

Original line number	Diff line number	Diff line
		@@ -300,15 +300,14 @@ impl InterfaceClass {
		/// [`crate::declare_binder_interface!`].
		pub fn new<I: InterfaceClassMethods>() -> 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<fn> 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<fn> 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),
		@@ -338,13 +337,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()
		@@ -542,17 +540,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;

		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_INIT.call_once(\|\| unsafe {
		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() }
		}
		};
		}
		@@ -664,6 +660,8 @@ pub unsafe trait AsNative<T> {
		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<T, V: AsNative<T>> AsNative<T> for Option<V> {
		fn as_native(&self) -> *const T {
		self.as_ref().map_or(ptr::null(), \|v\| v.as_native())
		@@ -924,15 +922,15 @@ macro_rules! declare_binder_interface {
		static CLASS_INIT: std::sync::Once = std::sync::Once::new();
		static mut CLASS: Option<$crate::binder_impl::InterfaceClass> = None;

		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_INIT.call_once(\|\| unsafe {
		CLASS = Some($crate::binder_impl::InterfaceClass::new::<$crate::binder_impl::Binder<$native>>());
		});
		unsafe {
		// Safety: The `CLASS` variable can only be mutated once, above,
		// and is subsequently safe to read from any thread.
		unsafe {
		CLASS.unwrap()
		}
		}

libs/binder/rust/src/error.rs

+71 −95

Original line number	Diff line number	Diff line
		@@ -112,21 +112,18 @@ fn to_cstring<T: AsRef<str>>(message: T) -> Option<CString> {
		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()
		};
		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
		@@ -135,18 +132,15 @@ impl Status {
		// know the returned pointer will be valid.
		//
		// Rust takes ownership of the returned pointer.
		sys::AStatus_fromServiceSpecificErrorWithMessage(err, message.as_ptr())
		}
		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)
		}
		unsafe { sys::AStatus_fromServiceSpecificError(err) }
		};
		Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
		}
		@@ -159,6 +153,8 @@ 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())
		};
		@@ -187,16 +183,13 @@ impl Status {

		/// 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())
		}
		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.
		@@ -204,20 +197,17 @@ impl Status {
		// `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 {
		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`.
		CStr::from_ptr(description_ptr)
		};
		let description = unsafe { CStr::from_ptr(description_ptr) };
		let description = description.to_string_lossy().to_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.
		unsafe {
		sys::AStatus_deleteDescription(description_ptr);
		}
		description
		@@ -225,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())
		};
		let code = unsafe { sys::AStatus_getExceptionCode(self.as_native()) };
		parse_exception_code(code)
		}

		@@ -241,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())
		};
		let code = unsafe { sys::AStatus_getStatus(self.as_native()) };
		parse_status_code(code)
		}

		@@ -258,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())
		}
		unsafe { sys::AStatus_getServiceSpecificError(self.as_native()) }
		}

		/// Calls `op` if the status was ok, otherwise returns an `Err` value of
		@@ -321,24 +305,20 @@ impl From<StatusCode> for Status {

		impl From<status_t> 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)
		};
		let ptr = unsafe { sys::AStatus_fromStatus(status) };
		Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
		}
		}

		impl From<ExceptionCode> 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)
		};
		let ptr = unsafe { sys::AStatus_fromExceptionCode(code as i32) };
		Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
		}
		}
		@@ -363,20 +343,18 @@ impl From<Status> for status_t {

		impl Drop for Status {
		fn drop(&mut self) {
		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.
		unsafe {
		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.
		@@ -386,11 +364,9 @@ unsafe impl AsNative<sys::AStatus> for Status {
		}

		fn as_native_mut(&mut self) -> *mut sys::AStatus {
		unsafe {
		// Safety: The pointer will be valid here since `Status` always
		// contains a valid and initialized pointer while it is alive.
		self.0.as_mut()
		}
		// 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() }
		}
		}