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)