[nrf noup] boot/bootutil/loader: image discovery by ih_load_address #461
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nvlsianpu
force-pushed
the
img_disc_by_load_addr
branch
2 times, most recently
from
dda8a3b to
db3fe90
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds a configuration option to use the image header’s load address for slot discovery instead of the reset handler address.
- Introduce
MCUBOOT_USE_CHECK_LOAD_ADDRKconfig option and adjust dependencies forMCUBOOT_VERIFY_IMG_ADDRESS - Refactor
boot_validate_slotandboot_validated_swap_typeto readih_load_addrwhen enabled - Define new
IS_IN_RANGE_*macros for address range checks
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| boot/zephyr/Kconfig | Add new config MCUBOOT_USE_CHECK_LOAD_ADDR and update dependencies |
| boot/bootutil/src/loader.c | Conditional read of ih_load_addr, macro definitions, and slot validation updates |
Comments suppressed due to low confidence (3)
boot/bootutil/src/loader.c:1264
- Mismatched
#ifdef/#elseguards—this#elsedoes not match the preceding#ifdef CONFIG_MCUBOOT_USE_CHECK_LOAD_ADDRcorrectly and will break compilation.
#else /* BOOT_USE_CHECK_LOAD_ADDR */
boot/bootutil/src/loader.c:1522
- Extra closing parenthesis in macro definition leads to a syntax error; remove the surplus
)at the end.
#define IS_IN_RANGE_CPUNET_APP_ADDR(_addr) ((_addr) >= PM_CPUNET_APP_ADDRESS && (_addr) < PM_CPUNET_APP_END_ADDRESS))
boot/bootutil/src/loader.c:1595
- The
IS_IN_RANGE_S_VARIANT_ADDRmacro expects two parameters but is invoked with one; useIS_IN_RANGE_S_ALTERNATE_ADDRorIS_IN_RANGE_S_CURRENT_ADDRinstead.
if (IS_IN_RANGE_S_VARIANT_ADDR(internal_img_addr)) {
boot/zephyr/Kconfig
Outdated
Comment on lines
1200
to
1203
| If y, the bootloader will use the load address form image header | ||
| for checking to which slot image belongs instead of usage of reset | ||
| handler addres reading form the image. |
There was a problem hiding this comment.
[nitpick] Typo in help text: replace “form image header” with “from image header” and correct “addres” to “address”.
Suggested change
| If y, the bootloader will use the load address form image header | |
| for checking to which slot image belongs instead of usage of reset | |
| handler addres reading form the image. | |
| If y, the bootloader will use the load address from image header | |
| for checking to which slot image belongs instead of usage of reset | |
| handler address reading from the image. |
Copilot uses AI. Check for mistakes.
nvlsianpu
force-pushed
the
img_disc_by_load_addr
branch
from
db3fe90 to
2fb0029
Compare
This was referenced Jul 8, 2025
Introduce alternative procedure for detecting to which partition image candidate belongs. This method uses ih_load_address field of the image header instead of reset vector address. This allows to match incoming image to the partition even when it is for instance encrypted, as the image header is always plain-text. This new procedure can be enabled using CONFIG_MCUBOOT_USE_CHECK_LOAD_ADDR=y. Firmware need to be signed with imgtool.py sign --rom-fixed <partition_address> parameter. ref.: NCSIDB-1173 Signed-off-by: Andrzej Puzdrowski <[email protected]>
nvlsianpu
force-pushed
the
img_disc_by_load_addr
branch
from
2fb0029 to
5155061
Compare
|
nordicjm
requested changes
Jul 24, 2025
| @@ -1331,15 +1331,19 @@ boot_validate_slot(struct boot_loader_state *state, int slot, | |||
| if (fap == BOOT_IMG_AREA(state, BOOT_SECONDARY_SLOT)) { | |||
| const struct flash_area *pri_fa = BOOT_IMG_AREA(state, BOOT_PRIMARY_SLOT); | |||
| struct image_header *secondary_hdr = boot_img_hdr(state, slot); | |||
| uint32_t reset_value = 0; | |||
| uint32_t reset_addr = secondary_hdr->ih_hdr_size + sizeof(reset_value); | |||
| uint32_t internal_img_addr = 0; /* either the reset handler addres or the image beginning addres */ | |||
| @@ -1620,8 +1635,9 @@ boot_validated_swap_type(struct boot_loader_state *state, | |||
| const struct flash_area *secondary_fa = | |||
| BOOT_IMG_AREA(state, BOOT_SECONDARY_SLOT); | |||
| struct image_header *hdr = boot_img_hdr(state, BOOT_SECONDARY_SLOT); | |||
| uint32_t reset_addr = 0; | |||
| uint32_t internal_img_addr = 0; /* either the reset handler addres or the image beginning addres */ | |||
There was a problem hiding this comment.
few place it needs correcting
| @@ -1283,10 +1283,17 @@ config USB_DEVICE_PRODUCT | |||
| config MCUBOOT_BOOTUTIL_LIB_OWN_LOG | |||
| bool | |||
|
|
|||
| config MCUBOOT_USE_CHECK_LOAD_ADDR | |||
| bool "use check of load address" | |||
There was a problem hiding this comment.
Suggested change
| bool "use check of load address" | |
| bool "Use check of load address" |
if we're adding to all new images, do we want to default y this? Or I guess not right away so other things e.g. qspi xip can be updated
| @@ -1597,6 +1601,17 @@ static inline void sec_slot_cleanup_if_unusable(void) | |||
| #endif /* defined(CONFIG_MCUBOOT_CLEANUP_UNUSABLE_SECONDARY) &&\ | |||
| defined(PM_S1_ADDRESS) || defined(CONFIG_SOC_NRF5340_CPUAPP) */ | |||
|
|
|||
| #define IS_IN_RANGE_CPUNET_APP_ADDR(_addr) ((_addr) >= PM_CPUNET_APP_ADDRESS && (_addr) < PM_CPUNET_APP_END_ADDRESS) | |||
| #define _IS_IN_RANGE_S_VARIANT_ADDR(_addr, x) ((_addr) >= PM_S##x_ADDRESS && (_addr) <= (PM_S##x_ADDRESS + PM_S##x_SIZE)) | |||
| #if (CONFIG_NCS_IS_VARIANT_IMAGE) | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce alternative procedure for detecting to which partition
image candidate belongs. This method uses ih_load_address field of the
image header instead of reset vector address. This allows to match
incoming image to the partition even when it is for instance encrypted,
as the image header is always plain-text.
This new procedure can be enabled using
CONFIG_MCUBOOT_USE_CHECK_LOAD_ADDR=y. Firmware need to be signed with
imgtool.py sign --rom-fixed <partition_address> parameter.
ref.: NCSIDB-1173