Skip to content

Conversation

@nvlsianpu
Copy link
Contributor

@nvlsianpu nvlsianpu commented Jul 2, 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

manifest-pr-skip

@nvlsianpu nvlsianpu marked this pull request as draft July 2, 2025 15:49
@nvlsianpu nvlsianpu force-pushed the img_disc_by_load_addr branch 2 times, most recently from dda8a3b to db3fe90 Compare July 3, 2025 14:17
@nvlsianpu nvlsianpu requested a review from Copilot July 4, 2025 15:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_ADDR Kconfig option and adjust dependencies for MCUBOOT_VERIFY_IMG_ADDRESS
  • Refactor boot_validate_slot and boot_validated_swap_type to read ih_load_addr when 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/#else guards—this #else does not match the preceding #ifdef CONFIG_MCUBOOT_USE_CHECK_LOAD_ADDR correctly 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_ADDR macro expects two parameters but is invoked with one; use IS_IN_RANGE_S_ALTERNATE_ADDR or IS_IN_RANGE_S_CURRENT_ADDR instead.
            if (IS_IN_RANGE_S_VARIANT_ADDR(internal_img_addr)) {

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.
Copy link

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.
@sonarqubecloud
Copy link

@nvlsianpu nvlsianpu marked this pull request as ready for review July 24, 2025 11:57
Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add nrf-squash! [nrf noup] treewide: Add support for sysbuild assigned images to commit message

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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*address

bool

config MCUBOOT_USE_CHECK_LOAD_ADDR
bool "use check of load address"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


#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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ifdef

@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch from 5155061 to aa627f8 Compare September 10, 2025 18:20
@de-nordic de-nordic changed the title [nrf noup] boot/bootutil/loader: image discovery by ih_load_address [wip] [nrf noup] boot/bootutil/loader: image discovery by ih_load_address Sep 10, 2025
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch from aa627f8 to 900c495 Compare September 11, 2025 08:17
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch 5 times, most recently from 566d3f3 to e1d9d3f Compare September 12, 2025 14:53
nvlsianpu and others added 6 commits September 16, 2025 16:38
Application need special support in the bootloader
in order to resume for suspend to RAM.

MCUboot is immediate actor which redirects execution to the application
(application reset vector) when wake-up from S2RAM is detected.
Detection is based on HW (NRF_RESETINFO) and hardened using additional
check over independent source of truth (variable with magic value).

Thanks to above the application is resuming using its routines - instead
of mocking that by routines compiled in by the MCUboot.

Implementation is able to support only MCUboot modes with a swap.
Direct-XIP is not handled as it require a way to run-time recognization of
active application slot.

Signed-off-by: Karol Lasończyk <[email protected]>
Signed-off-by: Tomasz Chyrowicz <[email protected]>
Signed-off-by: Andrzej Puzdrowski <[email protected]>
Added configuration which pre-configures MCUboot so It is able
to support operation of resuming the App from S2RAM by the application
itself.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Previously reopening of PR did not reopen manifest PR.
This commit will enable reopening of manifest PR in such case.

Signed-off-by: Kari Hamalainen <[email protected]>
This reverts commit d0796dc.

Signed-off-by: Jukka Rissanen <[email protected]>
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch from 89a583a to 2d42059 Compare October 1, 2025 08:06
tomchy and others added 3 commits October 1, 2025 09:58
Use the dedicated type to specify the slot number.

Signed-off-by: Tomasz Chyrowicz <[email protected]>
(cherry picked from commit d5b0dcb9aaee397fc105ae2228e8030038c3d871)
nrf-squash! [nrf noup] bootloader: Add bootloader requests

Change bootloader request module, so it uses the new, dedicated type to
point to the specific slot.

Ref: NCSDK-35199

Signed-off-by: Tomasz Chyrowicz <[email protected]>
fixes missing array bug.

Signed-off-by: Mateusz Michalek <[email protected]>
(cherry picked from commit 5e1be19fbc2afde8f7a6eb2ad8e3b31e5c3921cd)
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch 6 times, most recently from bd80231 to e4fd441 Compare October 3, 2025 18:08
tomchy added 3 commits October 6, 2025 08:55
Fix uninitialized variable warning as well as compile time issue, when
the slotted dependencies are enabled.

Signed-off-by: Tomasz Chyrowicz <[email protected]>
(cherry picked from commit 9ca088179e611ebd0054b0ef503039132c7a0ad1)
nrf-squash! [nrf noup] bootloader: Add bootloader requests

Fix a warning for uninitialized variable.

Ref: NCSDK-35733

Signed-off-by: Tomasz Chyrowicz <[email protected]>
If a device uses RESETINFO, than there are some bits set in the
resetinfo, even for reboots that should allow to interpret and
enter the device recovery.
That way it is possible to recover a device through serial recovery if
the main application resets due to i.e. watchdog.

Signed-off-by: Tomasz Chyrowicz <[email protected]>
(cherry picked from commit cc9852984f799d8d992594f966c472173925731f)
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch from e4fd441 to 3b21cec Compare October 7, 2025 07:29
nvlsianpu and others added 3 commits October 7, 2025 10:38
Added direct-xip support:
* new API for marking active slot to be used by boot_go() routines.
* jump vector assignment which is basing on above designation.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
Added call which designate active slot so MCUBoot can jump to
proper slot when CPU is resuming from S2RAM.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
nrf-squash! [nrf noup] bootutil: Add support for KMU stored ED25519 signature key

Will instead use the immutable bootloader key slot IDs if b0 is not
enabled, adds a Kconfig which can be used to fall back to the
previous slot IDs for previously deployed bootloaders

Signed-off-by: Jamie McCrae <[email protected]>
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch 3 times, most recently from 6460f21 to 4752c56 Compare October 7, 2025 14:22
nrf-squash! [nrf noup] boot/zephyr: nRF54h20 resume from S2RAM (hardened)

CONFIG_ARM_SOC_START_HOOK=y allow to rework the
resume from S2RAM code to work without PM_S2RAM mocking.
It allows to implement only what really needed from
the MCUboot perspective.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
nrf-squash! [nrf noup] boot/zephyr/socs: nrf54h20 prj.conf for S2RAM

Updated in order to use optimized configuration.

Signed-off-by: Andrzej Puzdrowski <[email protected]>
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch 2 times, most recently from 0f013f2 to d91e83e Compare October 10, 2025 19:20
nrf-squash! [nrf noup] treewide: Add support for sysbuild assigned images

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_CHECK_HEADER_LOAD_ADDRES =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]>
@de-nordic de-nordic force-pushed the img_disc_by_load_addr branch from d91e83e to 6435279 Compare October 10, 2025 19:27
@de-nordic de-nordic changed the title [wip] [nrf noup] boot/bootutil/loader: image discovery by ih_load_address [nrf noup] boot/bootutil/loader: image discovery by ih_load_address Oct 10, 2025
@sonarqubecloud
Copy link

*/
#ifdef PM_CPUNET_APP_ADDRESS
if (BOOT_CURR_IMG(state) == CONFIG_MCUBOOT_NETWORK_CORE_IMAGE_NUMBER) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

check_addresses = true;
} else
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

}
#endif /* CHECK_MCUBOOT_IMAGE */

#if CONFIG_MCUBOOT_APPLICATION_IMAGE_NUMBER != -1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#if CONFIG_MCUBOOT_APPLICATION_IMAGE_NUMBER != -1

there is always an app update image

#endif
check_addresses = true;
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#endif

#else
min_addr = pri_fa->fa_off;
max_addr = pri_fa->fa_off + pri_fa->fa_size;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

Comment on lines +1281 to +1283
#if CHECK_MCUBOOT_IMAGE == 1
min_addr = MIN(min_addr, NCS_VARIANT_SLOT_MIN_ADDR);
max_addr = MAX(max_addr, NCS_VARIANT_SLOT_MAX_ADDR);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to preset an unintended side effect, an image that spans across both the s0/s1 and the application partitions would pass this validation

BOOT_LOG_ERR("Cleaned-up secondary slot of image %d", BOOT_CURR_IMG(state));
return BOOT_SWAP_TYPE_FAIL;
} else if (reset_addr < primary_fa->fa_off || reset_addr > (primary_fa->fa_off + primary_fa->fa_size)) {
} else if (!IS_IN_RANGE_IMAGE_ADDR(internal_img_addr, primary_fa)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the primary and secondary slots are located in e.g. SPI or QSPI? This is how QSPI XIP split image support works and an upcoming extra DFU slot system will work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.