WolfSSL
/
wolfBoot
mirror of https://github.com/wolfSSL/wolfBoot.git
1
0
Fork 0
Code Issues Releases Wiki Activity

F-7987: abort the swap when a sector copy fails 

wolfBoot_copy_sector() discarded the return value of every flash
operation it performed and unconditionally returned the number of bytes
processed. Callers therefore treated a partially written sector as a
completed one and advanced the persistent sector flags, which are the
only record used to resume an interrupted swap. A write error while
copying BOOT into UPDATE (the backup step) could leave both the running
image and its backup corrupted with no way to redo the sector.

Check the result of every erase/read/write in wolfBoot_copy_sector() and
return -1 on the first failure. In the interruptible swap loop, the
delta loop and the DISABLE_BACKUP direct copy, stop on a negative return
without advancing the sector flag or confirming the boot partition, so
the swap is retried from the last completed step on the next boot.
pull/851/head
Daniele Lacamera 2026-08-11 12:48:05 +02:00
parent f446a4ad9f
commit bc743ad3d7
2 changed files with 97 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
src/update_flash.c
View File

@ -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 */

22
tools/unit-tests/unit-update-flash.c
View File

@ -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);