Merge branch 'develop' into firmware_protection

src/env/tock/mod.rs

@@ -1,6 +1,6 @@
-use self::storage::{SyscallStorage, SyscallUpgradeStorage};
+pub use self::storage::{TockStorage, TockUpgradeStorage};
 use crate::api::firmware_protection::FirmwareProtection;
-use crate::ctap::hid::{ChannelID, CtapHid, KeepaliveStatus, ProcessedPacket};
+use crate::ctap::hid::{ChannelID, CtapHid, CtapHidCommand, KeepaliveStatus, ProcessedPacket};
 use crate::ctap::status_code::Ctap2StatusCode;
 use crate::env::{Env, UserPresence};
 use core::cell::Cell;
@@ -15,46 +15,45 @@ use libtock_drivers::console::Console;
 use libtock_drivers::result::{FlexUnwrap, TockError};
 use libtock_drivers::timer::Duration;
 use libtock_drivers::{crp, led, timer, usb_ctap_hid};
-use persistent_store::StorageResult;
+use persistent_store::{StorageResult, Store};
 
 mod storage;
 
 pub struct TockEnv {
     rng: TockRng256,
-    storage: bool,
-    upgrade_storage: bool,
+    store: Store<TockStorage>,
+    upgrade_storage: Option<TockUpgradeStorage>,
 }
 
 impl TockEnv {
     /// Returns the unique instance of the Tock environment.
     ///
-    /// This function returns `Some` the first time it is called. Afterwards, it repeatedly returns
-    /// `None`.
-    pub fn new() -> Option<Self> {
-        // Make sure the environment was not already taken.
-        static TAKEN: AtomicBool = AtomicBool::new(false);
-        if TAKEN.fetch_or(true, Ordering::SeqCst) {
-            return None;
-        }
-        Some(TockEnv {
+    /// # Panics
+    ///
+    /// - If called a second time.
+    pub fn new() -> Self {
+        // We rely on `take_storage` to ensure that this function is called only once.
+        let storage = take_storage().unwrap();
+        let store = Store::new(storage).ok().unwrap();
+        let upgrade_storage = TockUpgradeStorage::new().ok();
+        TockEnv {
             rng: TockRng256 {},
-            storage: false,
-            upgrade_storage: false,
-        })
+            store,
+            upgrade_storage,
+        }
     }
 }
 
-/// Creates a new storage instance.
+/// Returns the unique storage instance.
 ///
-/// # Safety
+/// # Panics
 ///
-/// It is probably technically memory-safe to hame multiple storage instances at the same time, but
-/// for extra precaution we mark the function as unsafe. To ensure correct usage, this function
-/// should only be called if the previous storage instance was dropped.
-// This function is exposed for example binaries testing the hardware. This could probably be
-// cleaned up by having the persistent store return its storage.
-pub unsafe fn steal_storage() -> StorageResult<SyscallStorage> {
-    SyscallStorage::new()
+/// - If called a second time.
+pub fn take_storage() -> StorageResult<TockStorage> {
+    // Make sure the storage was not already taken.
+    static TAKEN: AtomicBool = AtomicBool::new(false);
+    assert!(!TAKEN.fetch_or(true, Ordering::SeqCst));
+    TockStorage::new()
 }
 
 impl UserPresence for TockEnv {
@@ -79,8 +78,8 @@ impl FirmwareProtection for TockEnv {
 impl Env for TockEnv {
     type Rng = TockRng256;
     type UserPresence = Self;
-    type Storage = SyscallStorage;
-    type UpgradeStorage = SyscallUpgradeStorage;
+    type Storage = TockStorage;
+    type UpgradeStorage = TockUpgradeStorage;
     type FirmwareProtection = Self;
 
     fn rng(&mut self) -> &mut Self::Rng {
@@ -91,14 +90,12 @@ impl Env for TockEnv {
         self
     }
 
-    fn storage(&mut self) -> StorageResult<Self::Storage> {
-        assert_once(&mut self.storage);
-        unsafe { steal_storage() }
+    fn store(&mut self) -> &mut Store<Self::Storage> {
+        &mut self.store
     }
 
-    fn upgrade_storage(&mut self) -> StorageResult<Self::UpgradeStorage> {
-        assert_once(&mut self.upgrade_storage);
-        SyscallUpgradeStorage::new()
+    fn upgrade_storage(&mut self) -> Option<&mut Self::UpgradeStorage> {
+        self.upgrade_storage.as_mut()
     }
 
     fn firmware_protection(&mut self) -> &mut Self::FirmwareProtection {
@@ -106,12 +103,6 @@ impl Env for TockEnv {
     }
 }
 
-/// Asserts a boolean is false and sets it to true.
-fn assert_once(b: &mut bool) {
-    assert!(!*b);
-    *b = true;
-}
-
 // Returns whether the keepalive was sent, or false if cancelled.
 fn send_keepalive_up_needed(
     cid: ChannelID,
@@ -146,7 +137,7 @@ fn send_keepalive_up_needed(
                 }
                 match processed_packet {
                     ProcessedPacket::InitPacket { cmd, .. } => {
-                        if cmd == CtapHid::COMMAND_CANCEL {
+                        if cmd == CtapHidCommand::Cancel as u8 {
                             // We ignore the payload, we can't answer with an error code anyway.
                             #[cfg(feature = "debug_ctap")]
                             writeln!(Console::new(), "User presence check cancelled").unwrap();