diff --git a/src/update_flash.c b/src/update_flash.c index fffa5c0d..0a720560 100644 --- a/src/update_flash.c +++ b/src/update_flash.c @@ -312,22 +312,35 @@ static int RAMFUNCTION wolfBoot_copy_sector(struct wolfBoot_image *src, #define BUFFER_DECLARED static uint8_t buffer[FLASHBUFFER_SIZE] XALIGNED(4); #endif - wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE); + if (wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE) < 0) { + ret = -1; + goto out; + } while (pos < WOLFBOOT_SECTOR_SIZE) { if (src_sector_offset + pos < (src->fw_size + IMAGE_HEADER_SIZE + FLASHBUFFER_SIZE)) { /* bypass decryption, copy encrypted data into swap if its external */ if (dst->part == PART_SWAP && SWAP_EXT) { - ext_flash_read((uintptr_t)(src->hdr) + src_sector_offset + pos, - (void *)buffer, FLASHBUFFER_SIZE); + if (ext_flash_read((uintptr_t)(src->hdr) + src_sector_offset + + pos, + (void *)buffer, FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } else { - ext_flash_check_read((uintptr_t)(src->hdr) + src_sector_offset + - pos, - (void *)buffer, FLASHBUFFER_SIZE); + if (ext_flash_check_read((uintptr_t)(src->hdr) + + src_sector_offset + pos, + (void *)buffer, FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } - wb_flash_write(dst, dst_sector_offset + pos, buffer, - FLASHBUFFER_SIZE); + if (wb_flash_write(dst, dst_sector_offset + pos, buffer, + FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } pos += FLASHBUFFER_SIZE; } @@ -335,19 +348,24 @@ static int RAMFUNCTION wolfBoot_copy_sector(struct wolfBoot_image *src, goto out; } #endif - wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE); + if (wb_flash_erase(dst, dst_sector_offset, WOLFBOOT_SECTOR_SIZE) < 0) { + ret = -1; + goto out; + } while (pos < WOLFBOOT_SECTOR_SIZE) { if (src_sector_offset + pos < (src->fw_size + IMAGE_HEADER_SIZE + FLASHBUFFER_SIZE)) { uint8_t *orig = (uint8_t*)(src->hdr + src_sector_offset + pos); - wb_flash_write(dst, dst_sector_offset + pos, orig, FLASHBUFFER_SIZE); + if (wb_flash_write(dst, dst_sector_offset + pos, orig, + FLASHBUFFER_SIZE) < 0) { + ret = -1; + goto out; + } } pos += FLASHBUFFER_SIZE; } ret = pos; -#if defined(EXT_FLASH) || defined(EXT_ENCRYPTED) out: -#endif #ifdef EXT_ENCRYPTED wolfBoot_zeroize(key, sizeof(key)); wolfBoot_zeroize(nonce, sizeof(nonce)); @@ -605,6 +623,7 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, { int sector = 0; int ret; + int copy_ret; uint8_t flag; uint8_t delta_blk[DELTA_BLOCK_SIZE]; uint32_t *img_offset; @@ -777,7 +796,11 @@ static int wolfBoot_delta_update(struct wolfBoot_image *boot, } } if (flag == SECT_FLAG_SWAPPING) { - wolfBoot_copy_sector(swap, boot, sector); + copy_ret = wolfBoot_copy_sector(swap, boot, sector); + if (copy_ret < 0) { + ret = -1; + goto out; + } flag = SECT_FLAG_UPDATED; if (((sector + 1) * WOLFBOOT_SECTOR_SIZE) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); @@ -919,6 +942,7 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) int bootStateRet = -1; uint8_t bootState = 0; #endif + int copy_ret = 0; #if defined(DISABLE_BACKUP) && defined(EXT_ENCRYPTED) uint8_t key[ENCRYPT_KEY_SIZE]; uint8_t nonce[ENCRYPT_NONCE_SIZE]; @@ -1125,7 +1149,9 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) switch (flag) { case SECT_FLAG_NEW: flag = SECT_FLAG_SWAPPING; - wolfBoot_copy_sector(&update, &swap, sector); + copy_ret = wolfBoot_copy_sector(&update, &swap, sector); + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); /* FALL THROUGH */ @@ -1145,11 +1171,13 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) */ int prev_iv = wolfBoot_enable_fallback_iv(1); #endif - wolfBoot_copy_sector(&boot, &update, sector); + copy_ret = wolfBoot_copy_sector(&boot, &update, sector); #ifdef EXT_ENCRYPTED wolfBoot_enable_fallback_iv(prev_iv); #endif } + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); /* FALL THROUGH */ @@ -1158,7 +1186,9 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) if (size > sector_size) size = sector_size; flag = SECT_FLAG_UPDATED; - wolfBoot_copy_sector(&swap, &boot, sector); + copy_ret = wolfBoot_copy_sector(&swap, &boot, sector); + if (copy_ret < 0) + break; if (((sector + 1) * sector_size) < WOLFBOOT_PARTITION_SIZE) wolfBoot_set_update_sector_flag(sector, flag); break; @@ -1167,6 +1197,20 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) default: break; } + if (copy_ret < 0) { + /* A flash operation failed: do not advance any further, the + * sector flags still describe the last completed step so the + * swap can be resumed from there. */ + wolfBoot_printf("Sector %d copy failed, aborting swap\n", sector); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); +#ifdef EXT_ENCRYPTED + wolfBoot_enable_fallback_iv(0); +#endif + return -1; + } sector++; /* headers that can be in different positions depending on when the @@ -1291,7 +1335,21 @@ static int RAMFUNCTION wolfBoot_update(int fallback_allowed) /* Directly copy the content of the UPDATE partition into the BOOT * partition. */ while ((sector * sector_size) < total_size) { - wolfBoot_copy_sector(&update, &boot, sector); + copy_ret = wolfBoot_copy_sector(&update, &boot, sector); + if (copy_ret < 0) { + /* Never confirm a boot image that was not fully written. */ + wolfBoot_printf("Sector %d copy failed, aborting swap\n", sector); +#ifdef EXT_FLASH + ext_flash_lock(); +#endif + hal_flash_lock(); +#ifdef EXT_ENCRYPTED + wolfBoot_zeroize(key, sizeof(key)); + wolfBoot_zeroize(nonce, sizeof(nonce)); + wolfBoot_enable_fallback_iv(0); +#endif + return -1; + } sector++; } /* erase remainder of partition */ diff --git a/tools/unit-tests/unit-update-flash.c b/tools/unit-tests/unit-update-flash.c index 245b15e0..cee449cc 100644 --- a/tools/unit-tests/unit-update-flash.c +++ b/tools/unit-tests/unit-update-flash.c @@ -787,6 +787,27 @@ START_TEST (test_forward_update_samesize) { } END_TEST +/* A failing flash write must abort the swap instead of marking the sector as + * updated: the sector flags are the only record used to resume an + * interrupted swap. */ +START_TEST (test_update_aborts_on_sector_copy_failure) { + uint8_t flag = SECT_FLAG_NEW; + reset_mock_stats(); + prepare_flash(); + add_payload(PART_BOOT, 1, TEST_SIZE_SMALL); + add_payload(PART_UPDATE, 2, TEST_SIZE_SMALL); + wolfBoot_update_trigger(); + /* BOOT is the only internal partition here, so the first write to + * internal flash is the copy of sector 0 from SWAP into BOOT. */ + hal_flash_write_fail = 1; + ck_assert_int_lt(wolfBoot_update(0), 0); + ck_assert_int_eq(hal_flash_write_fail, 0); + wolfBoot_get_update_sector_flag(0, &flag); + ck_assert_int_ne(flag, SECT_FLAG_UPDATED); + cleanup_flash(); +} +END_TEST + START_TEST (test_forward_update_tolarger) { reset_mock_stats(); prepare_flash(); @@ -1602,6 +1623,7 @@ Suite *wolfboot_suite(void) #endif tcase_add_test(sunnyday_noupdate, test_sunnyday_noupdate); tcase_add_test(forward_update_samesize, test_forward_update_samesize); + tcase_add_test(forward_update_samesize, test_update_aborts_on_sector_copy_failure); tcase_add_test(forward_update_tolarger, test_forward_update_tolarger); tcase_add_test(forward_update_tosmaller, test_forward_update_tosmaller); tcase_add_test(forward_update_sameversion_denied, test_forward_update_sameversion_denied);