kmwebnet/OpenSK
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid app re-entry by passing App argument to relevant calls (#494)

Browse Source 
* 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>
This commit is contained in:
Liam Murphy
2022-06-16 23:44:33 +10:00
committed by GitHub
parent 92e1d51442
commit 7e0c0938bb
1 changed files with 329 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

329
patches/tock/10-avoid-app-reentry.patch Normal file
View File

@@ -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<Side>,
+ pub callback: Option<Callback>,
+ pub buffer: Option<AppSlice<Shared, u8>>,
+ // 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<Side>,
- callback: Option<Callback>,
- buffer: Option<AppSlice<Shared, u8>>,
- // 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<App>) -> 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