From 7e0c0938bb17fb7fc89d4271d13f5e1d42d874a9 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 16 Jun 2022 23:44:33 +1000 Subject: [PATCH] Avoid app re-entry by passing App argument to relevant calls (#494) * Avoid app re-entry by passing App argument to relevant calls * Remove underscoring leading name * fixups * allows passing in capabilities to CtapHid (#496) * Fix libfido in configure (#499) * fix capitalization of Ctap2 in configure * changes setup to match new libfido2 version Co-authored-by: kaczmarczyck <43844792+kaczmarczyck@users.noreply.github.com> --- patches/tock/10-avoid-app-reentry.patch | 329 ++++++++++++++++++++++++ 1 file changed, 329 insertions(+) create mode 100644 patches/tock/10-avoid-app-reentry.patch diff --git a/patches/tock/10-avoid-app-reentry.patch b/patches/tock/10-avoid-app-reentry.patch new file mode 100644 index 0000000..cfc9939 --- /dev/null +++ b/patches/tock/10-avoid-app-reentry.patch @@ -0,0 +1,329 @@ +diff --git a/capsules/src/usb/app.rs b/capsules/src/usb/app.rs +new file mode 100644 +index 000000000..28dff3575 +--- /dev/null ++++ b/capsules/src/usb/app.rs +@@ -0,0 +1,65 @@ ++use kernel::{AppSlice, Callback, Shared}; ++ ++#[derive(Clone, Copy, PartialEq, Eq)] ++pub enum Side { ++ Transmit, ++ Receive, ++ TransmitOrReceive, ++} ++ ++impl Side { ++ pub fn can_transmit(&self) -> bool { ++ match self { ++ Side::Transmit | Side::TransmitOrReceive => true, ++ Side::Receive => false, ++ } ++ } ++ ++ pub fn can_receive(&self) -> bool { ++ match self { ++ Side::Receive | Side::TransmitOrReceive => true, ++ Side::Transmit => false, ++ } ++ } ++} ++ ++#[derive(Default)] ++pub struct App { ++ // Only one app can be connected to this driver, to avoid needing to route packets among apps. ++ // This field tracks this status. ++ pub connected: bool, ++ // Currently enabled transaction side. Subscribing to a callback or allowing a buffer ++ // automatically sets the corresponding side. Clearing both the callback and the buffer resets ++ // the side to None. ++ pub side: Option, ++ pub callback: Option, ++ pub buffer: Option>, ++ // Whether the app is waiting for the kernel signaling a packet transfer. ++ pub waiting: bool, ++} ++ ++impl App { ++ pub fn can_receive_packet(&self) -> bool { ++ self.waiting && self.side.map_or(false, |side| side.can_receive()) && self.buffer.is_some() ++ } ++ ++ pub fn check_side(&mut self) { ++ if self.callback.is_none() && self.buffer.is_none() && !self.waiting { ++ self.side = None; ++ } ++ } ++ ++ pub fn set_side(&mut self, side: Side) -> bool { ++ match self.side { ++ None => { ++ self.side = Some(side); ++ true ++ } ++ Some(app_side) => side == app_side, ++ } ++ } ++ ++ pub fn is_ready_for_command(&self, side: Side) -> bool { ++ self.buffer.is_some() && self.callback.is_some() && self.side == Some(side) ++ } ++} +diff --git a/capsules/src/usb/mod.rs b/capsules/src/usb/mod.rs +index 3f3a4f646..cb5e0af97 100644 +--- a/capsules/src/usb/mod.rs ++++ b/capsules/src/usb/mod.rs +@@ -1,3 +1,4 @@ ++pub mod app; + pub mod cdc; + pub mod descriptors; + pub mod usb_ctap; +diff --git a/capsules/src/usb/usb_ctap.rs b/capsules/src/usb/usb_ctap.rs +index da3d16d85..3a709aab5 100644 +--- a/capsules/src/usb/usb_ctap.rs ++++ b/capsules/src/usb/usb_ctap.rs +@@ -1,7 +1,7 @@ ++use super::app::{App, Side}; + use super::usbc_ctap_hid::ClientCtapHID; +-use kernel::hil; + use kernel::hil::usb::Client; +-use kernel::{AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared}; ++use kernel::{hil, AppId, AppSlice, Callback, Driver, Grant, ReturnCode, Shared}; + + /// Syscall number + use crate::driver; +@@ -25,73 +25,15 @@ pub const CTAP_SUBSCRIBE_TRANSMIT_OR_RECEIVE: usize = 3; + pub const CTAP_CALLBACK_TRANSMITED: usize = 1; + pub const CTAP_CALLBACK_RECEIVED: usize = 2; + +-#[derive(Clone, Copy, PartialEq, Eq)] +-enum Side { +- Transmit, +- Receive, +- TransmitOrReceive, +-} +- +-impl Side { +- fn can_transmit(&self) -> bool { +- match self { +- Side::Transmit | Side::TransmitOrReceive => true, +- Side::Receive => false, +- } +- } +- +- fn can_receive(&self) -> bool { +- match self { +- Side::Receive | Side::TransmitOrReceive => true, +- Side::Transmit => false, +- } +- } +-} +- +-#[derive(Default)] +-pub struct App { +- // Only one app can be connected to this driver, to avoid needing to route packets among apps. +- // This field tracks this status. +- connected: bool, +- // Currently enabled transaction side. Subscribing to a callback or allowing a buffer +- // automatically sets the corresponding side. Clearing both the callback and the buffer resets +- // the side to None. +- side: Option, +- callback: Option, +- buffer: Option>, +- // Whether the app is waiting for the kernel signaling a packet transfer. +- waiting: bool, +-} +- +-impl App { +- fn check_side(&mut self) { +- if self.callback.is_none() && self.buffer.is_none() && !self.waiting { +- self.side = None; +- } +- } +- +- fn set_side(&mut self, side: Side) -> bool { +- match self.side { +- None => { +- self.side = Some(side); +- true +- } +- Some(app_side) => side == app_side, +- } +- } +- +- fn is_ready_for_command(&self, side: Side) -> bool { +- self.buffer.is_some() && self.callback.is_some() && self.side == Some(side) +- } +-} +- + pub trait CtapUsbClient { + // Whether this client is ready to receive a packet. This must be checked before calling +- // packet_received(). +- fn can_receive_packet(&self) -> bool; ++ // packet_received(). If App is not supplied, it will be found from the implemntation's ++ // members. ++ fn can_receive_packet(&self, app: &Option<&mut App>) -> bool; + + // Signal to the client that a packet has been received. +- fn packet_received(&self, packet: &[u8; 64]); ++ // If App is not supplied, it will be found from the implemntation's members. ++ fn packet_received(&self, packet: &[u8; 64], app: Option<&mut App>); + + // Signal to the client that a packet has been transmitted. + fn packet_transmitted(&self); +@@ -106,38 +48,49 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbSyscallDriver<'a, 'b, C> { + pub fn new(usb_client: &'a ClientCtapHID<'a, 'b, C>, apps: Grant) -> Self { + CtapUsbSyscallDriver { usb_client, apps } + } ++ ++ fn app_packet_received(&self, packet: &[u8; 64], app: &mut App) { ++ if app.connected && app.waiting && app.side.map_or(false, |side| side.can_receive()) { ++ if let Some(buf) = &mut app.buffer { ++ // Copy the packet to the app's allowed buffer. ++ buf.as_mut().copy_from_slice(packet); ++ app.waiting = false; ++ // Signal to the app that a packet is ready. ++ app.callback ++ .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, 0, 0)); ++ } ++ } ++ } + } + + impl<'a, 'b, C: hil::usb::UsbController<'a>> CtapUsbClient for CtapUsbSyscallDriver<'a, 'b, C> { +- fn can_receive_packet(&self) -> bool { ++ fn can_receive_packet(&self, app: &Option<&mut App>) -> bool { + let mut result = false; +- for app in self.apps.iter() { +- app.enter(|app, _| { +- if app.connected { +- result = app.waiting +- && app.side.map_or(false, |side| side.can_receive()) +- && app.buffer.is_some(); ++ match app { ++ None => { ++ for app in self.apps.iter() { ++ app.enter(|a, _| { ++ if a.connected { ++ result = a.can_receive_packet(); ++ } ++ }) + } +- }); ++ } ++ Some(a) => result = a.can_receive_packet(), + } + result + } + +- fn packet_received(&self, packet: &[u8; 64]) { +- for app in self.apps.iter() { +- app.enter(|app, _| { +- if app.connected && app.waiting && app.side.map_or(false, |side| side.can_receive()) +- { +- if let Some(buf) = &mut app.buffer { +- // Copy the packet to the app's allowed buffer. +- buf.as_mut().copy_from_slice(packet); +- app.waiting = false; +- // Signal to the app that a packet is ready. +- app.callback +- .map(|mut cb| cb.schedule(CTAP_CALLBACK_RECEIVED, 0, 0)); +- } ++ fn packet_received(&self, packet: &[u8; 64], app: Option<&mut App>) { ++ match app { ++ None => { ++ for app in self.apps.iter() { ++ app.enter(|a, _| { ++ self.app_packet_received(packet, a); ++ }) + } +- }); ++ } ++ Some(a) => self.app_packet_received(packet, a), + } + } + +@@ -282,7 +235,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a, + ReturnCode::EALREADY + } else { + app.waiting = true; +- self.usb_client.receive_packet(); ++ self.usb_client.receive_packet(app); + ReturnCode::SUCCESS + } + } else { +@@ -303,7 +256,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> Driver for CtapUsbSyscallDriver<'a, + } else { + // Indicates to the driver that we can receive any pending packet. + app.waiting = true; +- self.usb_client.receive_packet(); ++ self.usb_client.receive_packet(app); + + if !app.waiting { + // The call to receive_packet() collected a pending packet. +diff --git a/capsules/src/usb/usbc_ctap_hid.rs b/capsules/src/usb/usbc_ctap_hid.rs +index 41d69752c..cf17d7942 100644 +--- a/capsules/src/usb/usbc_ctap_hid.rs ++++ b/capsules/src/usb/usbc_ctap_hid.rs +@@ -11,6 +11,7 @@ use super::descriptors::HIDSubordinateDescriptor; + use super::descriptors::InterfaceDescriptor; + use super::descriptors::ReportDescriptor; + use super::descriptors::TransferDirection; ++use super::app::App; + use super::usb_ctap::CtapUsbClient; + use super::usbc_client_ctrl::ClientCtrl; + use core::cell::Cell; +@@ -257,7 +258,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> { + } + } + +- pub fn receive_packet(&'a self) -> bool { ++ pub fn receive_packet(&'a self, app: &mut App) -> bool { + if self.pending_out.get() { + // The previous packet has not yet been received, reject the new one. + false +@@ -267,7 +268,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> { + // Otherwise, there's nothing to do, the controller will send us a packet_out when a + // packet arrives. + if self.delayed_out.take() { +- if self.send_packet_to_client() { ++ if self.send_packet_to_client(Some(app)) { + // If that succeeds, alert the controller that we can now + // receive data on the Interrupt OUT endpoint. + self.controller().endpoint_resume_out(1); +@@ -280,7 +281,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> { + // Send an OUT packet available in the controller back to the client. + // This returns false if the client is not ready to receive a packet, and true if the client + // successfully accepted the packet. +- fn send_packet_to_client(&'a self) -> bool { ++ fn send_packet_to_client(&'a self, app: Option<&mut App>) -> bool { + // Copy the packet into a buffer to send to the client. + let mut buf: [u8; 64] = [0; 64]; + for (i, x) in self.out_buffer.buf.iter().enumerate() { +@@ -292,7 +293,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> { + // Notify the client + if self + .client +- .map_or(false, |client| client.can_receive_packet()) ++ .map_or(false, |client| client.can_receive_packet(&app)) + { + assert!(self.pending_out.take()); + +@@ -301,7 +302,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> ClientCtapHID<'a, 'b, C> { + // should be re-transmitted or not. + self.cancel_in_transaction(); + +- self.client.map(|client| client.packet_received(&buf)); ++ self.client.map(|client| client.packet_received(&buf, app)); + true + } else { + // Cannot receive now, indicate a delay to the controller. +@@ -421,7 +422,7 @@ impl<'a, 'b, C: hil::usb::UsbController<'a>> hil::usb::Client<'a> for ClientCtap + // Cannot process this packet + hil::usb::OutResult::Error + } else { +- if self.send_packet_to_client() { ++ if self.send_packet_to_client(None) { + hil::usb::OutResult::Ok + } else { + hil::usb::OutResult::Delay