Skip to content

Commit 8975d5c

Browse files
taltenbachnordicjm
authored andcommitted
boot: bootutil: Fix underflow in swap-scratch when trailer is large
When using swap-scratch, the last sector of a slot able to contain firmware data might also contain part of the trailer (or the whole trailer, if the latter is small enough). When the trailer is large, a single sector it might not fit in a single sector and that last firmware sector might therefore not be the last sector of the slot. When that happens, and unless the trailer starts exactly at the beginning of a sector, an underflow could occur when computing the number of bytes that must be copied from the last firmware sector. Indeed, when the trailer is large, its size can be larger than that sector and, depending on the size of the sratch area, 'copy_sz' can at worst equal to the size of this sector. If this underflow occurs, 'copy_sz' would end up containing a very large value, that would probably cause the upgrade to fail and could lead to a corruption of a large part of the flash memory if no bound check is performed in the flash driver. Signed-off-by: Thomas Altenbach <[email protected]>
1 parent 7fd7611 commit 8975d5c

File tree

1 file changed

+13
-14
lines changed

1 file changed

+13
-14
lines changed

boot/bootutil/src/swap_scratch.c

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,17 @@ boot_swap_sectors(int idx, uint32_t sz, struct boot_loader_state *state,
563563
uint8_t image_index;
564564
int rc;
565565

566+
image_index = BOOT_CURR_IMG(state);
567+
568+
rc = flash_area_open(FLASH_AREA_IMAGE_PRIMARY(image_index), &fap_primary_slot);
569+
assert (rc == 0);
570+
571+
rc = flash_area_open(FLASH_AREA_IMAGE_SECONDARY(image_index), &fap_secondary_slot);
572+
assert (rc == 0);
573+
574+
rc = flash_area_open(FLASH_AREA_IMAGE_SCRATCH, &fap_scratch);
575+
assert (rc == 0);
576+
566577
/* Calculate offset from start of image area. */
567578
img_off = boot_img_sector_off(state, BOOT_PRIMARY_SLOT, idx);
568579

@@ -594,26 +605,14 @@ boot_swap_sectors(int idx, uint32_t sz, struct boot_loader_state *state,
594605
}
595606
}
596607

608+
/* Check if the currently swapped sector(s) contain the trailer or part of it */
597609
if ((img_off + sz) >
598610
boot_img_sector_off(state, BOOT_PRIMARY_SLOT, last_sector)) {
599-
copy_sz -= trailer_sz;
611+
copy_sz = flash_area_get_size(fap_primary_slot) - img_off - trailer_sz;
600612
}
601613

602614
bs->use_scratch = (bs->idx == BOOT_STATUS_IDX_0 && copy_sz != sz);
603615

604-
image_index = BOOT_CURR_IMG(state);
605-
606-
rc = flash_area_open(FLASH_AREA_IMAGE_PRIMARY(image_index),
607-
&fap_primary_slot);
608-
assert (rc == 0);
609-
610-
rc = flash_area_open(FLASH_AREA_IMAGE_SECONDARY(image_index),
611-
&fap_secondary_slot);
612-
assert (rc == 0);
613-
614-
rc = flash_area_open(FLASH_AREA_IMAGE_SCRATCH, &fap_scratch);
615-
assert (rc == 0);
616-
617616
if (bs->state == BOOT_STATUS_STATE_0) {
618617
BOOT_LOG_DBG("erasing scratch area");
619618
rc = boot_erase_region(fap_scratch, 0, flash_area_get_size(fap_scratch));

0 commit comments

Comments
 (0)