Skip to content

loader: Allow to specify slot number in version comparison #2343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions boot/bootutil/include/bootutil/image.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ extern "C" {
*/
#define IMAGE_TLV_ANY 0xffff /* Used to iterate over all TLV */

#define VERSION_DEP_SLOT_ACTIVE 0x00 /* Check dependency against active slot. */
#define VERSION_DEP_SLOT_PRIMARY 0x01 /* Check dependency against primary slot. */
#define VERSION_DEP_SLOT_SECONDARY 0x02 /* Check dependency against secondary slot. */

STRUCT_PACKED image_version {
uint8_t iv_major;
uint8_t iv_minor;
Expand All @@ -153,7 +157,11 @@ STRUCT_PACKED image_version {

struct image_dependency {
uint8_t image_id; /* Image index (from 0) */
#ifdef MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
uint8_t slot; /* Image slot */
#else
uint8_t _pad1;
#endif /* MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER */
uint16_t _pad2;
struct image_version image_min_version; /* Indicates at minimum which
* version of firmware must be
Expand Down
167 changes: 166 additions & 1 deletion boot/bootutil/src/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,24 @@ boot_verify_slot_dependency(struct boot_loader_state *state,
uint8_t swap_type = state->swap_type[dep->image_id];
dep_slot = BOOT_IS_UPGRADE(swap_type) ? BOOT_SECONDARY_SLOT
: BOOT_PRIMARY_SLOT;
#elif defined(MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER)
switch(dep->slot) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

boot_verify_slot_dependencies does range check for image number, on the dependency TLV it read, it should do the same with slot number.

case VERSION_DEP_SLOT_ACTIVE:
dep_slot = state->slot_usage[dep->image_id].active_slot;
break;
case VERSION_DEP_SLOT_PRIMARY:
dep_slot = BOOT_PRIMARY_SLOT;
break;
case VERSION_DEP_SLOT_SECONDARY:
dep_slot = BOOT_SECONDARY_SLOT;
break;
default:
return -1;
}

if (!state->slot_usage[dep->image_id].slot_available[dep_slot]) {
return -1;
}
#else
dep_slot = state->slot_usage[dep->image_id].active_slot;
#endif
Expand Down Expand Up @@ -354,7 +372,27 @@ boot_verify_slot_dependency(struct boot_loader_state *state,
}
#endif

return rc;
#ifdef MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
if (rc == 0) {
switch(dep->slot) {
case VERSION_DEP_SLOT_PRIMARY:
state->slot_usage[dep->image_id].slot_available[BOOT_PRIMARY_SLOT] = true;
state->slot_usage[dep->image_id].slot_available[BOOT_SECONDARY_SLOT] = false;
state->slot_usage[dep->image_id].active_slot = BOOT_PRIMARY_SLOT;
break;
case VERSION_DEP_SLOT_SECONDARY:
state->slot_usage[dep->image_id].slot_available[BOOT_PRIMARY_SLOT] = false;
state->slot_usage[dep->image_id].slot_available[BOOT_SECONDARY_SLOT] = true;
state->slot_usage[dep->image_id].active_slot = BOOT_SECONDARY_SLOT;
break;
case VERSION_DEP_SLOT_ACTIVE:
default:
break;
}
}
#endif /* MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER */

return rc;
}

#if !defined(MCUBOOT_DIRECT_XIP) && !defined(MCUBOOT_RAM_LOAD)
Expand Down Expand Up @@ -499,6 +537,19 @@ boot_verify_slot_dependencies(struct boot_loader_state *state, uint32_t slot)
goto done;
}

#ifdef MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
/* Validate against possible dependency slot values. */
switch(dep->slot) {
case VERSION_DEP_SLOT_ACTIVE:
case VERSION_DEP_SLOT_PRIMARY:
case VERSION_DEP_SLOT_SECONDARY:
break;
default:
rc = BOOT_EBADARGS;
goto done;
}
#endif /* MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER */

/* Verify dependency and modify the swap type if not satisfied. */
rc = boot_verify_slot_dependency(state, &dep);
if (rc != 0) {
Expand Down Expand Up @@ -2673,6 +2724,119 @@ boot_select_or_erase(struct boot_loader_state *state)
}
#endif /* MCUBOOT_DIRECT_XIP && MCUBOOT_DIRECT_XIP_REVERT */

#ifdef MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
/**
* Tries to load a slot for all the images with validation.
*
* @param state Boot loader status information.
*
* @return 0 on success; nonzero on failure.
*/
fih_ret
boot_load_and_validate_images(struct boot_loader_state *state)
{
uint32_t active_slot;
int rc;
fih_ret fih_rc;
uint32_t slot;

/* Go over all the images and all slots and validate them */
IMAGES_ITER(BOOT_CURR_IMG(state)) {
for (slot = 0; slot < BOOT_NUM_SLOTS; slot++) {
#if BOOT_IMAGE_NUMBER > 1
if (state->img_mask[BOOT_CURR_IMG(state)]) {
continue;
}
#endif

/* Save the number of the active slot. */
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = slot;

#ifdef MCUBOOT_DIRECT_XIP
rc = boot_rom_address_check(state);
if (rc != 0) {
/* The image is placed in an unsuitable slot. */
state->slot_usage[BOOT_CURR_IMG(state)].slot_available[slot] = false;
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}

#ifdef MCUBOOT_DIRECT_XIP_REVERT
rc = boot_select_or_erase(state);
if (rc != 0) {
/* The selected image slot has been erased. */
state->slot_usage[BOOT_CURR_IMG(state)].slot_available[slot] = false;
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}
#endif /* MCUBOOT_DIRECT_XIP_REVERT */
#endif /* MCUBOOT_DIRECT_XIP */

#ifdef MCUBOOT_RAM_LOAD
/* Image is first loaded to RAM and authenticated there in order to
* prevent TOCTOU attack during image copy. This could be applied
* when loading images from external (untrusted) flash to internal
* (trusted) RAM and image is authenticated before copying.
*/
rc = boot_load_image_to_sram(state);
if (rc != 0 ) {
/* Image cannot be ramloaded. */
boot_remove_image_from_flash(state, slot);
state->slot_usage[BOOT_CURR_IMG(state)].slot_available[slot] = false;
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}
#endif /* MCUBOOT_RAM_LOAD */

FIH_CALL(boot_validate_slot, fih_rc, state, slot, NULL, 0);
if (FIH_NOT_EQ(fih_rc, FIH_SUCCESS)) {
/* Image is invalid. */
#ifdef MCUBOOT_RAM_LOAD
boot_remove_image_from_sram(state);
#endif /* MCUBOOT_RAM_LOAD */
state->slot_usage[BOOT_CURR_IMG(state)].slot_available[slot] = false;
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
continue;
}

/* Valid image loaded from a slot, go to the next slot. */
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = NO_ACTIVE_SLOT;
}
}

/* Go over all the images and all slots and validate them */
IMAGES_ITER(BOOT_CURR_IMG(state)) {
/* All slots tried until a valid image found. Breaking from this loop
* means that a valid image found or already loaded. If no slot is
* found the function returns with error code. */
while (true) {
/* Go over all the slots and try to load one */
active_slot = state->slot_usage[BOOT_CURR_IMG(state)].active_slot;
if (active_slot != NO_ACTIVE_SLOT){
/* A slot is already active, go to next image. */
break;
}

active_slot = find_slot_with_highest_version(state);
if (active_slot == NO_ACTIVE_SLOT) {
BOOT_LOG_INF("No slot to load for image %d",
BOOT_CURR_IMG(state));
FIH_RET(FIH_FAILURE);
}

/* Save the number of the active slot. */
state->slot_usage[BOOT_CURR_IMG(state)].active_slot = active_slot;

/* Valid image loaded from a slot, go to the next image. */
break;
}
}

FIH_RET(FIH_SUCCESS);
}

#else /* MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER */

/**
* Tries to load a slot for all the images with validation.
*
Expand Down Expand Up @@ -2770,6 +2934,7 @@ boot_load_and_validate_images(struct boot_loader_state *state)

FIH_RET(FIH_SUCCESS);
}
#endif /* MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER */

/**
* Updates the security counter for the current image.
Expand Down
9 changes: 9 additions & 0 deletions boot/zephyr/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,15 @@ config BOOT_VERSION_CMP_USE_BUILD_NUMBER
minor and revision. Enable this option to take into account the build
number as well.

config BOOT_VERSION_CMP_USE_SLOT_NUMBER
bool "Use slot number while comparing image version"
depends on (UPDATEABLE_IMAGE_NUMBER > 1) || BOOT_DIRECT_XIP || \
BOOT_RAM_LOAD || MCUBOOT_DOWNGRADE_PREVENTION
help
By default, the image slot comparison relies only on active slot.
Enable this option to take into account the specified slot number
instead.

choice BOOT_DOWNGRADE_PREVENTION_CHOICE
prompt "Downgrade prevention"
optional
Expand Down
4 changes: 4 additions & 0 deletions boot/zephyr/include/mcuboot_config/mcuboot_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@
#define MCUBOOT_VERSION_CMP_USE_BUILD_NUMBER
#endif

#ifdef CONFIG_BOOT_VERSION_CMP_USE_SLOT_NUMBER
#define MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER
#endif
Comment on lines +123 to +125
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some more verbose description on MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER please. I know that this, in this case, is translation from Kconfig, but it affects generic code and we are, at this point, very bad at documenting all the crap we add to the mcuboot_config.h files.


#ifdef CONFIG_BOOT_SWAP_SAVE_ENCTLV
#define MCUBOOT_SWAP_SAVE_ENCTLV 1
#endif
Expand Down
17 changes: 17 additions & 0 deletions docs/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,23 @@ process is presented below.
+ Boot into image in the primary slot of the 0th image position\
(other image in the boot chain is started by another image).

By enabling the `MCUBOOT_VERSION_CMP_USE_SLOT_NUMBER` configuration option,
the dependency check may be extended to match for a spacified slot of a specific
image. This functionality is useful in a multi-core system when Direct XIP mode
is used.
In this case, the main image can be started from one of the two (primary or
secondary) slots.
If there is a fixed connection between the slots of two different images,
e.g. if the main image always chainloads a companion image from the same slot,
the check must take this into account and only consider a matching slot when
resolving dependencies.

There are three values that can be passed when specifying dependencies:

1. ``active``: the dependency should be checked against either primary or secondary slot.
2. ``primary``: the dependency should be checked only against primary slot.
3. ``secondary``: the dependency should be checked only against secondary slot.

### [Multiple image boot for RAM loading and direct-xip](#multiple-image-boot-for-ram-loading-and-direct-xip)

The operation of the bootloader is different when the ram-load or the
Expand Down
13 changes: 12 additions & 1 deletion docs/imgtool.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ primary slot and adds a header and trailer that the bootloader is expecting:
the `auto` keyword to automatically generate
it from the image version.
-d, --dependencies TEXT Add dependence on another image, format:
"(<image_ID>,<image_version>), ... "
"(<image_ID>,[<slot:active|primary|secondary>],
<image_version>), ... "
--pad-sig Add 0-2 bytes of padding to ECDSA signature
(for mcuboot <1.5)
-H, --header-size INTEGER [required]
Expand Down Expand Up @@ -182,6 +183,16 @@ which the current image depends on. The `image_version` is the minimum version
of that image to satisfy compliance. For example `-d "(1, 1.2.3+0)"` means this
image depends on Image 1 which version has to be at least 1.2.3+0.

In addition, a dependency can specify the slot as follows:
`-d "(image_id, slot, image_version)"`. The `image_id` is the number of the
image on which the current image depends.
The slot specifies which slots of the image are to be taken into account
(`active`: primary or secondary, `primary`: only primary `secondary`: only
secondary slot). The `image_version` is the minimum version of that image to
fulfill the requirements.
For example `-d "(1, primary, 1.2.3+0)"` means that this image depends on the
primary slot of the Image 1, whose version must be at least 1.2.3+0.

The `--public-key-format` argument can be used to distinguish where the public
key is stored for image authentication. The `hash` option is used by default, in
which case only the hash of the public key is added to the TLV area (the full
Expand Down
3 changes: 2 additions & 1 deletion scripts/imgtool/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,9 @@ def create(self, key, public_key_format, enckey, dependencies=None,
if dependencies is not None:
for i in range(dependencies_num):
payload = struct.pack(
e + 'B3x' + 'BBHI',
e + 'BB2x' + 'BBHI',
int(dependencies[DEP_IMAGES_KEY][i]),
dependencies[DEP_VERSIONS_KEY][i].slot,
dependencies[DEP_VERSIONS_KEY][i].major,
dependencies[DEP_VERSIONS_KEY][i].minor,
dependencies[DEP_VERSIONS_KEY][i].revision,
Expand Down
32 changes: 29 additions & 3 deletions scripts/imgtool/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import lzma
import hashlib
import base64
from collections import namedtuple
from imgtool import image, imgtool_version
from imgtool.version import decode_version
from imgtool.dumpinfo import dump_imginfo
Expand All @@ -45,6 +46,14 @@
sys.exit("Python %s.%s or newer is required by imgtool."
% MIN_PYTHON_VERSION)

SlottedSemiSemVersion = namedtuple('SemiSemVersion', ['major', 'minor', 'revision',
'build', 'slot'])

DEPENDENCY_SLOT_VALUES = {
'active': 0x00,
'primary': 0x01,
'secondary': 0x02
}

def gen_rsa2048(keyfile, passwd):
keys.RSA.generate().export_private(path=keyfile, passwd=passwd)
Expand Down Expand Up @@ -301,16 +310,33 @@ def get_dependencies(ctx, param, value):
if len(images) == 0:
raise click.BadParameter(
"Image dependency format is invalid: {}".format(value))
raw_versions = re.findall(r",\s*([0-9.+]+)\)", value)
raw_versions = re.findall(r",\s*((active|primary|secondary)\s*,)?\s*([0-9.+]+)\)", value)
if len(images) != len(raw_versions):
raise click.BadParameter(
'''There's a mismatch between the number of dependency images
and versions in: {}'''.format(value))
for raw_version in raw_versions:
try:
versions.append(decode_version(raw_version))
decoded_version = decode_version(raw_version[2])
if len(raw_version[1]) > 0:
slotted_version = SlottedSemiSemVersion(
decoded_version.major,
decoded_version.minor,
decoded_version.revision,
decoded_version.build,
DEPENDENCY_SLOT_VALUES[raw_version[1]]
)
else:
slotted_version = SlottedSemiSemVersion(
decoded_version.major,
decoded_version.minor,
decoded_version.revision,
decoded_version.build,
0
)
except ValueError as e:
raise click.BadParameter("{}".format(e))
versions.append(slotted_version)
dependencies = dict()
dependencies[image.DEP_IMAGES_KEY] = images
dependencies[image.DEP_VERSIONS_KEY] = versions
Expand Down Expand Up @@ -405,7 +431,7 @@ def convert(self, value, param, ctx):
'(for mcuboot <1.5)')
@click.option('-d', '--dependencies', callback=get_dependencies,
required=False, help='''Add dependence on another image, format:
"(<image_ID>,<image_version>), ... "''')
"(<image_ID>,[<slot:active|primary|secondary>],<image_version>), ... "''')
@click.option('-s', '--security-counter', callback=validate_security_counter,
help='Specify the value of security counter. Use the `auto` '
'keyword to automatically generate it from the image version.')
Expand Down
Loading