[Cryptech-Commits] [sw/libhal] 03/04: Write updated PIN block before updating index.

[git at cryptech.is]

This is an automated email from the git hooks/post-receive script.

sra at hactrn.net pushed a commit to branch ksng
in repository sw/libhal.

commit 19a31e1906592391b3e60c387b69aaa799e47077
Author: Rob Austein <sra at hactrn.net>
AuthorDate: Tue Sep 27 14:53:06 2016 -0400

    Write updated PIN block before updating index.
    
    Order of operations is tricky when updating flash blocks, because the
    process is not atomic and we want to leave the index in a consistent
    state if something fails.
---
 ks_flash.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/ks_flash.c b/ks_flash.c
index 895f7df..e8124a5 100644
--- a/ks_flash.c
+++ b/ks_flash.c
@@ -986,6 +986,14 @@ static hal_error_t fetch_pin_block(unsigned *b, flash_block_t **block)
  * Update the PIN block.  This block should always be present, but we
  * have to dance a bit to make sure we write the new PIN block before
  * destroying the old one.
+ *
+ * In theory we could simplify and speed this up a bit by taking
+ * advantage of knowing that the PIN block is always db.ksi.index[0]
+ * (because of the all-zeros UUID).  Maybe later.
+ *
+ * More generally, most of what happens here is part of updating any
+ * block, not just a PIN block, so we'll probably want to refactor
+ * once we get to the point where we need to update key blocks too.
  */
 
 static hal_error_t update_pin_block(const unsigned b1,
@@ -994,12 +1002,12 @@ static hal_error_t update_pin_block(const unsigned b1,
 {
   assert(block != NULL && new_data != NULL && block_get_type(block) == FLASH_PINBLK);
 
-  hal_error_t err;
-  unsigned b2;
+  if (db.ksi.used == db.ksi.size)
+    return HAL_ERROR_NO_KEY_SLOTS_AVAILABLE;
 
   block->header.block_type = FLASH_PINOLD;
 
-  err = block_write(b1, block);
+  hal_error_t err = block_write(b1, block);
 
   cache_release(block);
 
@@ -1007,28 +1015,31 @@ static hal_error_t update_pin_block(const unsigned b1,
     return err;
 
   /*
-   * We could simplify and speed this up a bit by taking advantage of
-   * knowing that the PIN block is always db.ksi->index[0] (because of
-   * the all-zeros UUID).  Maybe later.
+   * At this point we're committed to an update, because the old flash
+   * block is now a tombstone and can't be reverted in place without
+   * risking data loss.  So the rest of this dance is to make sure
+   * that we don't destroy the tombstone unless we succeeed in writing
+   * the new block, so that we can attempt recovery on reboot.
    */
 
-  if ((err = hal_ks_index_replace(&db.ksi, &pin_uuid, &b2)) != HAL_OK)
-    return err;
+  unsigned b2 = db.ksi.index[db.ksi.used];
 
-  block->pin = *new_data;
+  cache_mark_used(block, b2);
 
-  if (err == HAL_OK)
-    cache_mark_used(block, b2);
+  block->pin = *new_data;
 
-  if (err == HAL_OK)
-    err = block_erase_maybe(b2);
+  if ((err = block_erase_maybe(b2))  != HAL_OK ||
+      (err = block_write(b2, block)) != HAL_OK)
+    return err;
 
-  if (err == HAL_OK)
-    err = block_write(b2, block);
+  unsigned b3;
 
-  if (err != HAL_OK)
+  if ((err = hal_ks_index_replace(&db.ksi, &pin_uuid, &b3)) != HAL_OK)
     return err;
 
+  if (b2 != b3)
+    return HAL_ERROR_IMPOSSIBLE;
+
   if ((err = block_zero(b1)) != HAL_OK)
     return err;