Skip to content

Commit ade2da5

Browse files
de-nordicnvlsianpu
andcommitted
bootutil: Add MCUBOOT_CHECK_HEADER_LOAD_ADDRESS
Adding MCUBOOT_CHECK_HEADER_LOAD_ADDRESS that allows to verify header stored ih_load_addr against target boot slot, to allow MCUboot to reject firmware uploaded for incorrect slot. This option works with encrypted software, as it does not require decrypting image. This option takes precedense over MCUBOOT_VERIFY_IMG_ADDRESS. Note that the change leaves MCUBOOT_VERIFY_IMG_ADDRESS with the bug reported here #2473. Co-authored-by: Andrzej Puzdrowski <[email protected]> Signed-off-by: Dominik Ermel <[email protected]>
1 parent cc98529 commit ade2da5

File tree

1 file changed

+30
-5
lines changed

1 file changed

+30
-5
lines changed

boot/bootutil/src/loader.c

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ static struct image_max_size image_max_sizes[BOOT_IMAGE_NUMBER] = {0};
8181
#define TARGET_STATIC
8282
#endif
8383

84+
#if defined(MCUBOOT_VERIFY_IMG_ADDRESS) && defined(MCUBOOT_CHECK_HEADER_LOAD_ADDRESS)
85+
#warning MCUBOOT_CHECK_HEADER_LOAD_ADDRESS takes precedense over MCUBOOT_VERIFY_IMG_ADDRESS
86+
#endif
87+
88+
/* Valid only for ARM Cortext M */
89+
#define RESET_OFFSET (2 * sizeof(uin32_t))
90+
8491
#if BOOT_MAX_ALIGN > 1024
8592
#define BUF_SZ BOOT_MAX_ALIGN
8693
#else
@@ -1002,24 +1009,42 @@ boot_validate_slot(struct boot_loader_state *state, int slot,
10021009
goto out;
10031010
}
10041011

1005-
#if MCUBOOT_IMAGE_NUMBER > 1 && !defined(MCUBOOT_ENC_IMAGES) && defined(MCUBOOT_VERIFY_IMG_ADDRESS)
1012+
#if defined(MCUBOOT_VERIFY_IMG_ADDRESS && !defined(MCUBOOT_ENC_IMAGES) || \
1013+
defined(MCUBOOT_CHECK_HEADER_LOAD_ADDRESS)
10061014
/* Verify that the image in the secondary slot has a reset address
10071015
* located in the primary slot. This is done to avoid users incorrectly
10081016
* overwriting an application written to the incorrect slot.
10091017
* This feature is only supported by ARM platforms.
10101018
*/
10111019
if (fap == BOOT_IMG_AREA(state, BOOT_SLOT_SECONDARY)) {
1012-
const struct flash_area *pri_fa = BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY);
10131020
struct image_header *secondary_hdr = boot_img_hdr(state, slot);
10141021
uint32_t reset_value = 0;
1015-
uint32_t reset_addr = secondary_hdr->ih_hdr_size + sizeof(reset_value);
1022+
uint32_t internal_img_addr = 0; /* either the reset handler addres or the image beginning addres */
1023+
uint32_t min_addr;
1024+
uint32_t max_addr;
10161025

1017-
if (flash_area_read(fap, reset_addr, &reset_value, sizeof(reset_value)) != 0) {
1026+
min_addr = flash_area_get_off(BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY));
1027+
max_addr = flash_area_get_size(BOOT_IMG_AREA(state, BOOT_SLOT_PRIMARY)) + min_addr;
1028+
1029+
/* MCUBOOT_CHECK_HEADER_LOAD_ADDRESS takes priority over MCUBOOT_VERIFY_IMG_ADDRESS */
1030+
#ifdef MCUBOOT_CHECK_HEADER_LOAD_ADDRESS
1031+
internal_img_addr = secondary_hdr->ih_load_addr;
1032+
#else
1033+
/* This is platform specific code that should not be here */
1034+
const uint32_t offset = secondary_hdr->ih_hdr_size + RESET_OFFSET;
1035+
BOOT_LOG_DBG("Getting image %d internal addr from offset %u",
1036+
BOOT_CURR_IMG(state), offset);
1037+
if (flash_area_read(fap, offset, &internal_img_addr, sizeof(internal_img_addr)) != 0)
1038+
BOOT_LOG_ERR("Failed to read image %d load address", BOOT_CURR_IMG(state));
10181039
fih_rc = FIH_NO_BOOTABLE_IMAGE;
10191040
goto out;
10201041
}
1042+
#endif
10211043

1022-
if (reset_value < pri_fa->fa_off || reset_value> (pri_fa->fa_off + pri_fa->fa_size)) {
1044+
BOOT_LOG_DBG("Image %d expected load address 0x%x", BOOT_CURR_IMG(state), internal_img_addr);
1045+
BOOT_LOG_DBG("Check 0x%x is within [min_addr, max_addr] = [0x%x, 0x%x)",
1046+
internal_img_addr, min_addr, max_addr);
1047+
if (internal_img_addr < min_addr || internal_img_addr >= max_addr) {
10231048
BOOT_LOG_ERR("Reset address of image in secondary slot is not in the primary slot");
10241049
BOOT_LOG_ERR("Erasing image from secondary slot");
10251050

0 commit comments

Comments
 (0)