Don't reflash storage after each PIN entry

Instead of reflashing the whole storage, we use a designated area
in the second storage block, where we mark each PIN failure by a
single zero bit. This is because one can set bits in flash to zero but
not to one.  If the PIN was entered successfully the whole word is
set to zero and the next word stores the new PIN failure counter.

firmware/protect.c

@@ -142,14 +142,12 @@ const char *requestPin(PinMatrixRequestType type, const char *text)
 
 bool protectPin(bool use_cached)
 {
-	if (!storage.has_pin || strlen(storage.pin) == 0 || (use_cached && session_isPinCached())) {
+	if (!storage.has_pin || storage.pin[0] == 0 || (use_cached && session_isPinCached())) {
 		return true;
 	}
-	uint32_t fails = storage_getPinFails();
+	uint32_t *fails = storage_getPinFailsPtr();
-	if (fails) {
+	uint32_t wait = ~*fails;
-		uint32_t wait;
+	while (wait > 0) {
-		wait = (fails < 32) ? (1u << fails) : 0xFFFFFFFF;
-		while (--wait > 0) {
 		// convert wait to secstr string
 		char secstrbuf[20];
 		strlcpy(secstrbuf, "________0 seconds", sizeof(secstrbuf));
@@ -166,7 +164,7 @@ bool protectPin(bool use_cached)
 		layoutDialog(DIALOG_ICON_INFO, NULL, NULL, NULL, "Wrong PIN entered", NULL, "Please wait", secstr, "to continue ...", NULL);
 		// wait one second
 		usbDelay(840000);
-		}
+		wait--;
 	}
 	const char *pin;
 	pin = requestPin(PinMatrixRequestType_PinMatrixRequestType_Current, "Please enter current PIN:");
@@ -174,11 +172,9 @@ bool protectPin(bool use_cached)
 		fsm_sendFailure(FailureType_Failure_PinCancelled, "PIN Cancelled");
 		return false;
 	}
-	storage_increasePinFails();
+	if (storage_increasePinFails(fails) && storage_isPinCorrect(pin)) {
-	bool increase_failed = (fails >= storage_getPinFails());
-	if (storage_isPinCorrect(pin) && !increase_failed) {
 		session_cachePin();
-		storage_resetPinFails();
+		storage_resetPinFails(fails);
 		return true;
 	} else {
 		fsm_sendFailure(FailureType_Failure_PinInvalid, "Invalid PIN");

firmware/storage.c

@@ -40,20 +40,11 @@
 #include "protect.h"
 #include "layout2.h"
 
-_Static_assert(sizeof(Storage) <= FLASH_STORAGE_LEN, "Storage struct is too large for TREZOR flash");
 Storage storage;
 
 uint8_t storage_uuid[12];
 char    storage_uuid_str[25];
 
-static bool sessionSeedCached;
-static uint8_t sessionSeed[64];
-
-static bool sessionPinCached;
-
-static bool sessionPassphraseCached;
-static char sessionPassphrase[51];
-
 /*
  storage layout:
 
@@ -62,28 +53,67 @@ static char sessionPassphrase[51];
  0x0000 |   4 bytes   |  magic = 'stor'
  0x0004 |  12 bytes   |  uuid
  0x0010 |  ?          |  Storage structure
+ 0x4000 |   4 kbytes  |  area for pin failures
+ 0x5000 |  12 kbytes  |  reserved
+
+The area for pin failures looks like this:
+0 ... 0 pinfail 0xffffffff .. 0xffffffff
+The pinfail is a binary number of the form 1...10...0,
+the number of zeros is the number of pin failures.
+This layout is used because we can only clear bits without 
+erasing the flash.
+
  */
 
-#define STORAGE_VERSION 5
+#define FLASH_STORAGE_PINAREA     (FLASH_META_START + 0x4000)
+#define FLASH_STORAGE_PINAREA_LEN (0x1000)
+#define FLASH_STORAGE_REALLEN (4 + sizeof(storage_uuid) + sizeof(Storage))
+_Static_assert(FLASH_STORAGE_START + FLASH_STORAGE_REALLEN <= FLASH_STORAGE_PINAREA, "Storage struct is too large for TREZOR flash");
+
+static bool sessionSeedCached;
+
+static uint8_t sessionSeed[64];
+
+static bool sessionPinCached;
+
+static bool sessionPassphraseCached;
+static char sessionPassphrase[51];
+
+#define STORAGE_VERSION 6
+
+void storage_check_flash_errors(void)
+{
+	// flash operation failed
+	if (FLASH_SR & (FLASH_SR_PGAERR | FLASH_SR_PGPERR | FLASH_SR_PGSERR | FLASH_SR_WRPERR)) {
+		layoutDialog(DIALOG_ICON_ERROR, NULL, NULL, NULL, "Storage failure", "detected.", NULL, "Please unplug", "the device.", NULL);
+		for (;;) { }
+	}
+}
 
 void storage_from_flash(uint32_t version)
 {
-	switch (version) {
+	// version 1: since 1.0.0
-		case 1: // copy (since 1.0.0)
+	// version 2: since 1.2.1
+	// version 3: since 1.3.1
+	// version 4: since 1.3.2
+	// version 5: since 1.3.3
+	// version 6: since 1.3.6
 	memcpy(&storage, (void *)(FLASH_STORAGE_START + 4 + sizeof(storage_uuid)), sizeof(Storage));
-			break;
+	if (version <= 5) {
-		case 2: // copy (since 1.2.1)
+		// convert PIN failure counter from version 5 format
-			memcpy(&storage, (void *)(FLASH_STORAGE_START + 4 + sizeof(storage_uuid)), sizeof(Storage));
+		uint32_t pinctr = storage.has_pin_failed_attempts
-			break;
+			? storage.pin_failed_attempts : 0;
-		case 3: // copy (since 1.3.1)
+		if (pinctr > 31)
-			memcpy(&storage, (void *)(FLASH_STORAGE_START + 4 + sizeof(storage_uuid)), sizeof(Storage));
+			pinctr = 31;
-			break;
+		flash_clear_status_flags();
-		case 4: // copy (since 1.3.2)
+		flash_unlock();
-			memcpy(&storage, (void *)(FLASH_STORAGE_START + 4 + sizeof(storage_uuid)), sizeof(Storage));
+		// erase extra storage sector
-			break;
+		flash_erase_sector(FLASH_META_SECTOR_LAST, FLASH_CR_PROGRAM_X32);
-		case 5: // copy (since 1.3.3)
+		flash_program_word(FLASH_STORAGE_PINAREA, 0xffffffff << pinctr);
-			memcpy(&storage, (void *)(FLASH_STORAGE_START + 4 + sizeof(storage_uuid)), sizeof(Storage));
+		flash_lock();
-			break;
+		storage_check_flash_errors();
+		storage.has_pin_failed_attempts = false;
+		storage.pin_failed_attempts = 0;
 	}
 	storage.version = STORAGE_VERSION;
 }
@@ -136,35 +166,29 @@ void session_clear(bool clear_pin)
 	}
 }
 
-static uint8_t meta_backup[FLASH_META_LEN];
-
 void storage_commit(void)
 {
 	int i;
-	uint32_t *w;
+	uint32_t meta_backup[(FLASH_STORAGE_PINAREA - FLASH_META_START) / 4];
+
 	// backup meta
-	memcpy(meta_backup, (void *)FLASH_META_START, FLASH_META_LEN);
+	memcpy((uint8_t*)meta_backup, (uint8_t*)FLASH_META_START, FLASH_META_DESC_LEN);
+	// modify storage
+	memcpy((uint8_t*)meta_backup + FLASH_META_DESC_LEN, "stor", 4);
+	memcpy((uint8_t*)meta_backup + FLASH_META_DESC_LEN + 4, storage_uuid, sizeof(storage_uuid));
+	memcpy((uint8_t*)meta_backup + FLASH_META_DESC_LEN + 4 + sizeof(storage_uuid), &storage, sizeof(Storage));
+	memset((uint8_t*)meta_backup + FLASH_META_DESC_LEN + FLASH_STORAGE_REALLEN, 0, sizeof(meta_backup) - FLASH_META_DESC_LEN - FLASH_STORAGE_REALLEN);
+
 	flash_clear_status_flags();
 	flash_unlock();
 	// erase storage
-	for (i = FLASH_META_SECTOR_FIRST; i <= FLASH_META_SECTOR_LAST; i++) {
+	flash_erase_sector(FLASH_META_SECTOR_FIRST, FLASH_CR_PROGRAM_X32);
-		flash_erase_sector(i, FLASH_CR_PROGRAM_X32);
-	}
-	// modify storage
-	memcpy(meta_backup + FLASH_META_DESC_LEN, "stor", 4);
-	memcpy(meta_backup + FLASH_META_DESC_LEN + 4, storage_uuid, sizeof(storage_uuid));
-	memcpy(meta_backup + FLASH_META_DESC_LEN + 4 + sizeof(storage_uuid), &storage, sizeof(Storage));
 	// copy it back
-	for (i = 0; i < FLASH_META_LEN / 4; i++) {
+	for (i = 0; i < (signed) (sizeof(meta_backup) / 4); i++) {
-		w = (uint32_t *)(meta_backup + i * 4);
+		flash_program_word(FLASH_META_START + i * 4, meta_backup[i]);
-		flash_program_word(FLASH_META_START + i * 4, *w);
 	}
 	flash_lock();
-	// flash operation failed
+	storage_check_flash_errors();
-	if (FLASH_SR & (FLASH_SR_PGAERR | FLASH_SR_PGPERR | FLASH_SR_PGSERR | FLASH_SR_WRPERR)) {
-		layoutDialog(DIALOG_ICON_ERROR, NULL, NULL, NULL, "Storage failure", "detected.", NULL, "Please unplug", "the device.", NULL);
-		for (;;) { }
-	}
 }
 
 void storage_loadDevice(LoadDevice *msg)
@@ -344,7 +368,7 @@ bool storage_hasPin(void)
 
 void storage_setPin(const char *pin)
 {
-	if (pin && strlen(pin) > 0) {
+	if (pin && pin[0]) {
 		storage.has_pin = true;
 		strlcpy(storage.pin, pin, sizeof(storage.pin));
 	} else {
@@ -376,28 +400,44 @@ bool session_isPinCached(void)
 	return sessionPinCached;
 }
 
-void storage_resetPinFails(void)
+void storage_resetPinFails(uint32_t *pinfailsptr)
 {
-	storage.has_pin_failed_attempts = true;
+	flash_clear_status_flags();
-	storage.pin_failed_attempts = 0;
+	flash_unlock();
-	storage_commit();
+	if ((uint32_t) (pinfailsptr + 1) - FLASH_STORAGE_PINAREA
-}
+		>= FLASH_STORAGE_PINAREA_LEN) {
-
+		// erase extra storage sector
-void storage_increasePinFails(void)
+		flash_erase_sector(FLASH_META_SECTOR_LAST, FLASH_CR_PROGRAM_X32);
-{
-	if (!storage.has_pin_failed_attempts) {
-		storage.has_pin_failed_attempts = true;
-		storage.pin_failed_attempts = 1;
 	} else {
-		storage.pin_failed_attempts++;
+		flash_program_word((uint32_t) pinfailsptr, 0);
 	}
-	storage_commit();
+	flash_lock();
+	storage_check_flash_errors();
 }
 
-uint32_t storage_getPinFails(void)
+bool storage_increasePinFails(uint32_t *pinfailsptr)
 {
-	storage_from_flash(STORAGE_VERSION); // reload from flash
+	uint32_t newctr = *pinfailsptr << 1;
-	return storage.has_pin_failed_attempts ? storage.pin_failed_attempts : 0;
+	// counter already at maximum, we do not increase it any more
+	// return success so that a good pin is accepted
+	if (!newctr)
+		return true;
+
+	flash_clear_status_flags();
+	flash_unlock();
+	flash_program_word((uint32_t) pinfailsptr, newctr);
+	flash_lock();
+	storage_check_flash_errors();
+
+	return *pinfailsptr == newctr;
+}
+
+uint32_t *storage_getPinFailsPtr(void)
+{
+	uint32_t *pinfailsptr = (uint32_t *) FLASH_STORAGE_PINAREA;
+	while (*pinfailsptr == 0)
+		pinfailsptr++;
+	return pinfailsptr;
 }
 
 bool storage_isInitialized(void)

firmware/storage.h

@@ -56,9 +56,9 @@ bool storage_hasPin(void);
 void storage_setPin(const char *pin);
 void session_cachePin(void);
 bool session_isPinCached(void);
-void storage_resetPinFails(void);
+void storage_resetPinFails(uint32_t *pinfailptr);
-void storage_increasePinFails(void);
+bool storage_increasePinFails(uint32_t *pinfailptr);
-uint32_t storage_getPinFails(void);
+uint32_t *storage_getPinFailsPtr(void);
 
 bool storage_isInitialized(void);