mailbox_client: revise message receive for race-free use
The previous structure for receiving mailbox messages was racey--it
allowed a reply to be sent without a receive posted. Rewrite the api
to ensure this cannot happen.
Fix: 300251123
Change-Id: Ied0e697f71c88de10e53217ff2ee02af28263673
diff --git a/app/src/mailbox_client.rs b/app/src/mailbox_client.rs
index 671542f..15e2f05 100644
--- a/app/src/mailbox_client.rs
+++ b/app/src/mailbox_client.rs
@@ -65,23 +65,47 @@
pub struct MailboxClient {}
+pub type WaitResult = (Option<bool>, usize, Option<usize>);
+
impl MailboxClient {
pub fn init() {
let _ = syscalls::command(CAPSULE_MAILBOX, CMD_MAILBOX_INIT, 0, 0);
let _ = syscalls::command(CAPSULE_STORAGE, CMD_STORAGE_INIT, 0, 0);
}
- // Waits for an inbound message that it writes to |buffer|. The msg
- // size (in bytes) is returned along with any associated page frame
- pub async fn wait_message_async(buffer: &mut [u8]) -> (usize, Option<usize>) {
- // Status, message size (bytes), attached page (or 0)
- type WaitResult = (Option<bool>, usize, Option<usize>);
- let message_result: Cell<WaitResult> = Cell::new((None, 0, None));
+ // Support for receiving inbound mailbox messages. This is split into
+ // multiple methods to support a race-free usage pattern:
+ //
+ // use MailboxClient::WaitResult;
+ // MailboxClient::init();
+ // let wait_result = Cell::new(WaitResult::default());
+ // MailboxClient::wait_message_init(&wait_result, unsafe { &mut MESSAGE_BUF });
+ // loop {
+ // MailboxClient::wait_message_async(&wait_result).await;
+ // // Status, message size (bytes), Option<attached page>
+ // let (_, request_size, opt_page) = wait_result.take();
+ //
+ // .. process received message ..
+ //
+ // MailboxClient::send_message_sync(reply_size, reply_slice);
+ // }
+ // MailboxClient::wait_message_cleanup();
+ //
+ // TODO(sleffler): maybe wrap in a struct w/ RAII cleanup
+ // Sets up the caller to receive a mailbox message in |buffer| using
+ // |wait_result| to pass data about the received message from the irq
+ // handler. The operation is completed asynchronously. The caller must
+ // poll the implied future until |wait_result| holds the received
+ // message data; typically using |wait_message_async|, Note that
+ // |wait_message_setup| registers a subscriber for callbacks from the
+ // irq handler; this must be cleared before |wait_result| is
+ // deallocated--usually by calling |wait_message_cleanup|.
+ pub fn wait_message_setup(wait_result: &Cell<WaitResult>, buffer: &mut [u8]) {
// Callback on recv complete.
- unsafe extern "C" fn subscribe_callback(ok: usize, len: usize, page: usize, data: usize) {
- let message_result = &*(data as *const Cell<WaitResult>);
- message_result.set((
+ unsafe extern "C" fn wait_callback(ok: usize, len: usize, page: usize, data: usize) {
+ let wait_result = &*(data as *const Cell<WaitResult>);
+ wait_result.set((
Some(ok != 0),
len,
if page == 0 { None } else { Some(page) },
@@ -101,22 +125,40 @@
syscalls::raw::subscribe(
CAPSULE_MAILBOX,
CMD_MAILBOX_RECV,
- subscribe_callback as *const _,
- &message_result as *const _ as usize,
+ wait_callback as *const _,
+ wait_result as *const _ as usize,
);
}
- futures::wait_until(|| (message_result.get().0.is_some())).await;
+ }
+ // Cleans up state setup by |wait_message_setup|.
+ pub fn wait_message_cleanup() {
unsafe {
syscalls::raw::subscribe(CAPSULE_MAILBOX, 0, ptr::null(), 0);
syscalls::raw::allow(CAPSULE_MAILBOX, 0, ptr::null_mut(), 0);
}
+ }
+ // Waits for a mailbox message to be received.
+ pub async fn wait_message_async(wait_result: &Cell<WaitResult>) {
+ futures::wait_until(|| (wait_result.get().0.is_some())).await;
+ }
- let (_, msg_size, attached_page) = message_result.into_inner();
+ // Waits for an inbound message that it writes to |buffer|. The msg
+ // size (in bytes) is returned along with any associated page frame
+ #[allow(dead_code)]
+ pub async fn wait_message_sync(buffer: &mut [u8]) -> (usize, Option<usize>) {
+ let wait_result = Cell::new(WaitResult::default());
+ Self::wait_message_setup(&wait_result, buffer);
+
+ Self::wait_message_async(&wait_result).await;
+ Self::wait_message_cleanup();
+
+ // Status, message size (bytes), attached page (or 0)
+ let (_, msg_size, attached_page) = wait_result.into_inner();
(msg_size, attached_page)
}
// Sends the message of |size| bytes that resides in |buffer|
- // XXX why isn't buffer.len() used?
+ // XXX why isn't buffer.len() used instead of |size|?
pub fn send_message_sync(size: usize, buffer: &[u8]) {
unsafe {
syscalls::raw::allow(
diff --git a/app/src/main.rs b/app/src/main.rs
index 874faa4..9ca6a24 100644
--- a/app/src/main.rs
+++ b/app/src/main.rs
@@ -1,8 +1,11 @@
#![no_std]
+use core::cell::Cell;
+
mod dprintf;
mod mailbox_client;
use mailbox_client::MailboxClient;
+use mailbox_client::WaitResult;
use libtock::result::TockResult;
use libtock::syscalls;
@@ -23,11 +26,14 @@
dprintf!("SEC: Booting seL4 from TockOS app done!\r\n");
MailboxClient::init();
+ let wait_result = Cell::new(WaitResult::default());
+ MailboxClient::wait_message_setup(&wait_result, unsafe { &mut MESSAGE_BUF });
unsafe {
loop {
- let (request_size, opt_page) =
- MailboxClient::wait_message_async(&mut MESSAGE_BUF).await;
+ MailboxClient::wait_message_async(&wait_result).await;
+ // Status, message size (bytes), Option<attached page>
+ let (_, request_size, opt_page) = wait_result.take();
let (request_slice, reply_slice) = MESSAGE_BUF.split_at_mut(request_size);
let reply_size = MailboxClient::dispatch(request_slice, opt_page, reply_slice)
