Move tracking of HWM to RAM

- rename nvram_data to baking_hwm_data - Move out updating hwm_data out of UPDATE_NVRAM macro - update single variable at a time in nvram - enable/disable nvram update in macro based on hwm setting.
trilitech · Apr 8, 2024 · 27f9ae8 · 27f9ae8
1 parent 33ef9d2
commit 27f9ae8
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 61 deletions.
diff --git a/src/apdu_reset.c b/src/apdu_reset.c
@@ -36,14 +36,15 @@
  * @return true
  */
 static bool ok(void) {
-    UPDATE_NVRAM(ram, {
-        ram->hwm.main.highest_level = G.reset_level;
-        ram->hwm.main.highest_round = 0;
-        ram->hwm.main.had_attestation = false;
-        ram->hwm.test.highest_level = G.reset_level;
-        ram->hwm.test.highest_round = 0;
-        ram->hwm.test.had_attestation = false;
-    });
+    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
+    hwm_data->hwm.main.highest_level = G.reset_level;
+    hwm_data->hwm.main.highest_round = 0;
+    hwm_data->hwm.main.had_attestation = false;
+    hwm_data->hwm.test.highest_level = G.reset_level;
+    hwm_data->hwm.test.highest_round = 0;
+    hwm_data->hwm.test.had_attestation = false;
+
+    UPDATE_NVRAM;
 
     // Send back the response, do not restart the event loop
     io_send_sw(SW_OK);

diff --git a/src/apdu_setup.c b/src/apdu_setup.c
@@ -42,18 +42,19 @@
  * @return true
  */
 static bool ok(void) {
-    UPDATE_NVRAM(ram, {
-        copy_bip32_path_with_curve(&ram->baking_key, &global.path_with_curve);
-        ram->main_chain_id = G.main_chain_id;
-        ram->hwm.main.highest_level = G.hwm.main;
-        ram->hwm.main.highest_round = 0;
-        ram->hwm.main.had_attestation = false;
-        ram->hwm.main.had_preattestation = false;
-        ram->hwm.test.highest_level = G.hwm.test;
-        ram->hwm.test.highest_round = 0;
-        ram->hwm.test.had_attestation = false;
-        ram->hwm.test.had_preattestation = false;
-    });
+    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
+    copy_bip32_path_with_curve(&(hwm_data->baking_key), &global.path_with_curve);
+    hwm_data->main_chain_id = G.main_chain_id;
+    hwm_data->hwm.main.highest_level = G.hwm.main;
+    hwm_data->hwm.main.highest_round = 0;
+    hwm_data->hwm.main.had_attestation = false;
+    hwm_data->hwm.main.had_preattestation = false;
+    hwm_data->hwm.test.highest_level = G.hwm.test;
+    hwm_data->hwm.test.highest_round = 0;
+    hwm_data->hwm.test.had_attestation = false;
+    hwm_data->hwm.test.had_preattestation = false;
+
+    UPDATE_NVRAM;
 
     provide_pubkey(&global.path_with_curve);
 
@@ -89,7 +90,9 @@ int handle_setup(buffer_t *cdata, derivation_type_t derivation_type) {
 }
 
 int handle_deauthorize(void) {
-    UPDATE_NVRAM(ram, { memset(&ram->baking_key, 0, sizeof(ram->baking_key)); });
+    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
+    memset(&(hwm_data->baking_key), 0, sizeof(hwm_data->baking_key));
+    UPDATE_NVRAM_VAR(baking_key);
 #ifdef HAVE_BAGL
     // Ignore calculation errors
     calculate_idle_screen_authorized_key();

diff --git a/src/baking_auth.c b/src/baking_auth.c
@@ -44,20 +44,21 @@ tz_exc write_high_water_mark(parsed_baking_data_t const *const in) {
 
     TZ_ASSERT(is_valid_level(in->level), EXC_WRONG_VALUES);
 
-    UPDATE_NVRAM(ram, {
-        // If the chain matches the main chain *or* the main chain is not set, then use 'main' HWM.
-        high_watermark_t volatile *const dest = select_hwm_by_chain(in->chain_id, ram);
-        TZ_ASSERT_NOT_NULL(dest);
-
-        if ((in->level > dest->highest_level) || (in->round > dest->highest_round)) {
-            dest->had_attestation = false;
-            dest->had_preattestation = false;
-        };
-        dest->highest_level = CUSTOM_MAX(in->level, dest->highest_level);
-        dest->highest_round = in->round;
-        dest->had_attestation |= in->type == BAKING_TYPE_ATTESTATION;
-        dest->had_preattestation |= in->type == BAKING_TYPE_PREATTESTATION;
-    });
+    baking_hwm_data *hwm_data = &global.apdu.baking_auth.new_data;
+    // If the chain matches the main chain *or* the main chain is not set, then use 'main' HWM.
+    high_watermark_t *dest = select_hwm_by_chain(in->chain_id, hwm_data);
+    TZ_ASSERT_NOT_NULL(dest);
+
+    if ((in->level > dest->highest_level) || (in->round > dest->highest_round)) {
+        dest->had_attestation = false;
+        dest->had_preattestation = false;
+    };
+    dest->highest_level = CUSTOM_MAX(in->level, dest->highest_level);
+    dest->highest_round = in->round;
+    dest->had_attestation |= in->type == BAKING_TYPE_ATTESTATION;
+    dest->had_preattestation |= in->type == BAKING_TYPE_PREATTESTATION;
+
+    UPDATE_NVRAM_VAR(hwm);
 
 end:
     return exc;
@@ -73,10 +74,10 @@ tz_exc authorize_baking(derivation_type_t const derivation_type,
               EXC_WRONG_LENGTH);
 
     if (bip32_path->length != 0u) {
-        UPDATE_NVRAM(ram, {
-            ram->baking_key.derivation_type = derivation_type;
-            copy_bip32_path(&ram->baking_key.bip32_path, bip32_path);
-        });
+        baking_hwm_data *out_data = &global.apdu.baking_auth.new_data;
+        out_data->baking_key.derivation_type = derivation_type;
+        copy_bip32_path(&out_data->baking_key.bip32_path, bip32_path);
+        UPDATE_NVRAM_VAR(baking_key);
     }
 
 end:

diff --git a/src/globals.c b/src/globals.c
@@ -47,15 +47,16 @@ void init_globals(void) {
 }
 
 void toggle_hwm(void) {
-    UPDATE_NVRAM(ram, { ram->hwm_disabled = !ram->hwm_disabled; });
+    baking_hwm_data *out = &global.apdu.baking_auth.new_data;
+    out->hwm_disabled = !out->hwm_disabled;
+    UPDATE_NVRAM_VAR(hwm_disabled);  // Update the NVRAM data.
 }
 
 // DO NOT TRY TO INIT THIS. This can only be written via an system call.
 // The "N_" is *significant*. It tells the linker to put this in NVRAM.
-nvram_data const N_data_real;
+baking_hwm_data const N_data_real;
 
-high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id,
-                                               nvram_data volatile *const ram) {
+high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id, baking_hwm_data *const ram) {
     if (ram == NULL) {
         return NULL;
     }

diff --git a/src/globals.h b/src/globals.h
@@ -149,15 +149,15 @@ typedef struct {
 
         /// state used to store baking authorizing data
         struct {
-            nvram_data new_data;  ///< Staging area for setting N_data
+            baking_hwm_data new_data;  ///< baking HWM data in RAM
         } baking_auth;
     } apdu;
 } globals_t;
 
 extern globals_t global;
 
-extern nvram_data const N_data_real;
-#define N_data (*(volatile nvram_data *) PIC(&N_data_real))
+extern baking_hwm_data const N_data_real;
+#define N_data (*(volatile baking_hwm_data *) PIC(&N_data_real))
 
 /**
  * @brief Selects a HWM for a given chain id depending on the ram
@@ -170,21 +170,28 @@ extern nvram_data const N_data_real;
  * @param ram: ram
  * @return high_watermark_t*: selected HWM
  */
-high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id,
-                                               nvram_data volatile *const ram);
+high_watermark_t *select_hwm_by_chain(chain_id_t const chain_id,
+                                      baking_hwm_data volatile *const ram);
 
 /**
- * @brief Properly updates NVRAM data to prevent any clobbering of data
+ * @brief Properly updates a single variable in NVRAM.
  *
- * @param out_name: defines the name of a pointer to the nvram_data struct
- * @param body: defines the code to apply updates
+ * @param variable: defines the name of the variable to be updated in NVRAM
  */
-#define UPDATE_NVRAM(out_name, body)                                                    \
-    ({                                                                                  \
-        nvram_data *const out_name = &global.apdu.baking_auth.new_data;                 \
-        memcpy(&global.apdu.baking_auth.new_data,                                       \
-               (nvram_data const *const) &N_data,                                       \
-               sizeof(global.apdu.baking_auth.new_data));                               \
-        body;                                                                           \
-        nvm_write((void *) &N_data, &global.apdu.baking_auth.new_data, sizeof(N_data)); \
-    })
+#define UPDATE_NVRAM_VAR(variable)                                    \
+    if (!N_data_real.hwm_disabled) {                                  \
+        nvm_write((void *) &(N_data.variable),                        \
+                  &global.apdu.baking_auth.new_data.variable,         \
+                  sizeof(global.apdu.baking_auth.new_data.variable)); \
+    }
+
+/**
+ * @brief Properly updates an entire NVRAM struct to prevent any clobbering of data
+ *
+ */
+#define UPDATE_NVRAM                                         \
+    if (!N_data_real.hwm_disabled) {                         \
+        nvm_write((void *) &(N_data),                        \
+                  &global.apdu.baking_auth.new_data,         \
+                  sizeof(global.apdu.baking_auth.new_data)); \
+    }
diff --git a/src/types.h b/src/types.h
@@ -202,7 +202,7 @@ typedef struct {
     bool hwm_disabled;                   /**< Set HWM setting on/off,
                                               e.g. if you are using signer assisted HWM,
                                               no need to track HWM using Ledger.*/
-} nvram_data;
+} baking_hwm_data;
 
 #define SIGN_HASH_SIZE 32u  // TODO: Rename or use a different constant.