aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRob Austein <sra@hactrn.net>2016-09-27 14:53:06 -0400
committerRob Austein <sra@hactrn.net>2016-09-27 14:53:06 -0400
commit19a31e1906592391b3e60c387b69aaa799e47077 (patch)
treea8ff31ac1a6b3f027c35e6888ff891f0436bd6e1
parenta9c0ad1888b7c7a94d46ded4a39dfb6cb3bbcb90 (diff)
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.
-rw-r--r--ks_flash.c43
1 files 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;