Extract run_rpc_server to libbinder_common
We have two very similar uses of RunRpcServerCallback; extract them to
a common place, using a closure to encapsulate the specific behavior
for notifiying readiness.
This also neatly encapsulates all the unsafe code, and justification
of it, in one place.
Bug: 187444679
Test: atest ComposTestCase
Change-Id: If23f862748b2f921690820eaa6ee80e4b2829274
diff --git a/authfs/fd_server/src/main.rs b/authfs/fd_server/src/main.rs
index 32ffc0a..395e2e9 100644
--- a/authfs/fd_server/src/main.rs
+++ b/authfs/fd_server/src/main.rs
@@ -25,14 +25,13 @@
mod fsverity;
use anyhow::{bail, Result};
-use binder::unstable_api::AsNative;
+use binder_common::rpc_server::run_rpc_server;
use log::{debug, error};
use std::cmp::min;
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::fs::File;
use std::io;
-use std::os::raw;
use std::os::unix::fs::FileExt;
use std::os::unix::io::{AsRawFd, FromRawFd};
@@ -342,23 +341,17 @@
);
let args = parse_args()?;
-
- let mut service = FdService::new_binder(args.fd_pool).as_binder();
- let mut ready_notifier = ReadyNotifier(args.ready_fd);
+ let service = FdService::new_binder(args.fd_pool).as_binder();
debug!("fd_server is starting as a rpc service.");
- // SAFETY: Service ownership is transferring to the server and won't be valid afterward.
- // Plus the binder objects are threadsafe.
- // RunRpcServerCallback does not retain a reference to ready_callback, and only ever
- // calls it with the param we provide during the lifetime of ready_notifier.
- let retval = unsafe {
- binder_rpc_unstable_bindgen::RunRpcServerCallback(
- service.as_native_mut() as *mut binder_rpc_unstable_bindgen::AIBinder,
- RPC_SERVICE_PORT,
- Some(ReadyNotifier::ready_callback),
- ready_notifier.as_void_ptr(),
- )
- };
+
+ let mut ready_fd = args.ready_fd;
+ let retval = run_rpc_server(service, RPC_SERVICE_PORT, || {
+ debug!("fd_server is ready");
+ // Close the ready-fd if we were given one to signal our readiness.
+ drop(ready_fd.take());
+ });
+
if retval {
debug!("RPC server has shut down gracefully");
Ok(())
@@ -366,25 +359,3 @@
bail!("Premature termination of RPC server");
}
}
-
-struct ReadyNotifier(Option<File>);
-
-impl ReadyNotifier {
- fn notify(&mut self) {
- debug!("fd_server is ready");
- // Close the ready-fd if we were given one to signal our readiness.
- drop(self.0.take());
- }
-
- fn as_void_ptr(&mut self) -> *mut raw::c_void {
- self as *mut _ as *mut raw::c_void
- }
-
- unsafe extern "C" fn ready_callback(param: *mut raw::c_void) {
- // SAFETY: This is only ever called by RunRpcServerCallback, within the lifetime of the
- // ReadyNotifier, with param taking the value returned by as_void_ptr (so a properly aligned
- // non-null pointer to an initialized instance).
- let ready_notifier = param as *mut Self;
- ready_notifier.as_mut().unwrap().notify()
- }
-}
diff --git a/binder_common/Android.bp b/binder_common/Android.bp
index 9e6d590..209955d 100644
--- a/binder_common/Android.bp
+++ b/binder_common/Android.bp
@@ -9,6 +9,7 @@
edition: "2018",
rustlibs: [
"libbinder_rs",
+ "libbinder_rpc_unstable_bindgen",
"liblazy_static",
],
apex_available: [
diff --git a/binder_common/lib.rs b/binder_common/lib.rs
index 055688a..f2391e3 100644
--- a/binder_common/lib.rs
+++ b/binder_common/lib.rs
@@ -17,6 +17,7 @@
//! Common items useful for binder clients and/or servers.
pub mod lazy_service;
+pub mod rpc_server;
use binder::public_api::{ExceptionCode, Status};
use std::ffi::CString;
diff --git a/binder_common/rpc_server.rs b/binder_common/rpc_server.rs
new file mode 100644
index 0000000..36075cf
--- /dev/null
+++ b/binder_common/rpc_server.rs
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+//! Helpers for implementing an RPC Binder server.
+
+use binder::public_api::SpIBinder;
+use binder::unstable_api::AsNative;
+use std::os::raw;
+
+/// Run a binder RPC server, serving the supplied binder service implementation on the given vsock
+/// port.
+/// If and when the server is ready for connections (it is listening on the port) on_ready
+/// is called to allow appropriate action to be taken - e.g. to notify clients they
+/// may now attempt to connect.
+/// The current thread is joined to the binder thread pool to handle incoming messages.
+/// Returns true if the server has shutdown normally, false if it failed in some way.
+pub fn run_rpc_server<F>(service: SpIBinder, port: u32, on_ready: F) -> bool
+where
+ F: FnOnce(),
+{
+ let mut ready_notifier = ReadyNotifier(Some(on_ready));
+ ready_notifier.run_server(service, port)
+}
+
+struct ReadyNotifier<F>(Option<F>)
+where
+ F: FnOnce();
+
+impl<F> ReadyNotifier<F>
+where
+ F: FnOnce(),
+{
+ fn run_server(&mut self, mut service: SpIBinder, port: u32) -> bool {
+ let service = service.as_native_mut() as *mut binder_rpc_unstable_bindgen::AIBinder;
+ let param = self.as_void_ptr();
+
+ // SAFETY: Service ownership is transferring to the server and won't be valid afterward.
+ // Plus the binder objects are threadsafe.
+ // RunRpcServerCallback does not retain a reference to ready_callback, and only ever
+ // calls it with the param we provide during the lifetime of self.
+ unsafe {
+ binder_rpc_unstable_bindgen::RunRpcServerCallback(
+ service,
+ port,
+ Some(Self::ready_callback),
+ param,
+ )
+ }
+ }
+
+ fn as_void_ptr(&mut self) -> *mut raw::c_void {
+ self as *mut _ as *mut raw::c_void
+ }
+
+ unsafe extern "C" fn ready_callback(param: *mut raw::c_void) {
+ // SAFETY: This is only ever called by RunRpcServerCallback, within the lifetime of the
+ // ReadyNotifier, with param taking the value returned by as_void_ptr (so a properly aligned
+ // non-null pointer to an initialized instance).
+ let ready_notifier = param as *mut Self;
+ ready_notifier.as_mut().unwrap().notify()
+ }
+
+ fn notify(&mut self) {
+ if let Some(on_ready) = self.0.take() {
+ on_ready();
+ }
+ }
+}
diff --git a/compos/src/compsvc_main.rs b/compos/src/compsvc_main.rs
index d0c920a..6887947 100644
--- a/compos/src/compsvc_main.rs
+++ b/compos/src/compsvc_main.rs
@@ -30,14 +30,14 @@
};
use anyhow::{anyhow, bail, Context, Result};
use binder::{
- unstable_api::{new_spibinder, AIBinder, AsNative},
+ unstable_api::{new_spibinder, AIBinder},
FromIBinder,
};
+use binder_common::rpc_server::run_rpc_server;
use compos_common::COMPOS_VSOCK_PORT;
use log::{debug, error};
use nix::ioctl_read_bad;
use std::fs::OpenOptions;
-use std::os::raw;
use std::os::unix::io::AsRawFd;
/// The CID representing the host VM
@@ -62,23 +62,17 @@
);
}
- let mut service = compsvc::new_binder()?.as_binder();
+ let service = compsvc::new_binder()?.as_binder();
+ let vm_service = get_vm_service()?;
+ let local_cid = get_local_cid()?;
+
debug!("compsvc is starting as a rpc service.");
- let mut ready_notifier = ReadyNotifier::new()?;
-
- // SAFETY: Service ownership is transferring to the server and won't be valid afterward.
- // Plus the binder objects are threadsafe.
- // RunRpcServerCallback does not retain a reference to ready_callback, and only ever
- // calls it with the param we provide during the lifetime of ready_notifier.
- let retval = unsafe {
- binder_rpc_unstable_bindgen::RunRpcServerCallback(
- service.as_native_mut() as *mut binder_rpc_unstable_bindgen::AIBinder,
- COMPOS_VSOCK_PORT,
- Some(ReadyNotifier::ready_callback),
- ready_notifier.as_void_ptr(),
- )
- };
+ let retval = run_rpc_server(service, COMPOS_VSOCK_PORT, || {
+ if let Err(e) = vm_service.notifyPayloadReady(local_cid as i32) {
+ error!("Unable to notify ready: {}", e);
+ }
+ });
if retval {
debug!("RPC server has shut down gracefully");
Ok(())
@@ -87,61 +81,32 @@
}
}
-struct ReadyNotifier {
- vm_service: Strong<dyn IVirtualMachineService>,
- local_cid: u32,
+fn get_vm_service() -> Result<Strong<dyn IVirtualMachineService>> {
+ // SAFETY: AIBinder returned by RpcClient has correct reference count, and the ownership
+ // can be safely taken by new_spibinder.
+ let ibinder = unsafe {
+ new_spibinder(binder_rpc_unstable_bindgen::RpcClient(
+ VMADDR_CID_HOST,
+ VM_BINDER_SERVICE_PORT as u32,
+ ) as *mut AIBinder)
+ }
+ .ok_or_else(|| anyhow!("Failed to connect to IVirtualMachineService"))?;
+
+ FromIBinder::try_from(ibinder).context("Connecting to IVirtualMachineService")
}
-impl ReadyNotifier {
- fn new() -> Result<Self> {
- Ok(Self { vm_service: Self::get_vm_service()?, local_cid: Self::get_local_cid()? })
- }
-
- fn notify(&self) {
- if let Err(e) = self.vm_service.notifyPayloadReady(self.local_cid as i32) {
- error!("Unable to notify ready: {}", e);
- }
- }
-
- fn as_void_ptr(&mut self) -> *mut raw::c_void {
- self as *mut _ as *mut raw::c_void
- }
-
- unsafe extern "C" fn ready_callback(param: *mut raw::c_void) {
- // SAFETY: This is only ever called by RunRpcServerCallback, within the lifetime of the
- // ReadyNotifier, with param taking the value returned by as_void_ptr (so a properly aligned
- // non-null pointer to an initialized instance).
- let ready_notifier = param as *mut Self;
- ready_notifier.as_ref().unwrap().notify()
- }
-
- fn get_vm_service() -> Result<Strong<dyn IVirtualMachineService>> {
- // SAFETY: AIBinder returned by RpcClient has correct reference count, and the ownership
- // can be safely taken by new_spibinder.
- let ibinder = unsafe {
- new_spibinder(binder_rpc_unstable_bindgen::RpcClient(
- VMADDR_CID_HOST,
- VM_BINDER_SERVICE_PORT as u32,
- ) as *mut AIBinder)
- }
- .ok_or_else(|| anyhow!("Failed to connect to IVirtualMachineService"))?;
-
- FromIBinder::try_from(ibinder).context("Connecting to IVirtualMachineService")
- }
-
- // TODO(b/199259751): remove this after VS can check the peer addresses of binder clients
- fn get_local_cid() -> Result<u32> {
- let f = OpenOptions::new()
- .read(true)
- .write(false)
- .open("/dev/vsock")
- .context("Failed to open /dev/vsock")?;
- let mut cid = 0;
- // SAFETY: the kernel only modifies the given u32 integer.
- unsafe { vm_sockets_get_local_cid(f.as_raw_fd(), &mut cid) }
- .context("Failed to get local CID")?;
- Ok(cid)
- }
+// TODO(b/199259751): remove this after VS can check the peer addresses of binder clients
+fn get_local_cid() -> Result<u32> {
+ let f = OpenOptions::new()
+ .read(true)
+ .write(false)
+ .open("/dev/vsock")
+ .context("Failed to open /dev/vsock")?;
+ let mut cid = 0;
+ // SAFETY: the kernel only modifies the given u32 integer.
+ unsafe { vm_sockets_get_local_cid(f.as_raw_fd(), &mut cid) }
+ .context("Failed to get local CID")?;
+ Ok(cid)
}
// TODO(b/199259751): remove this after VS can check the peer addresses of binder clients