Skip to content
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

mgmt: mcumgr: grp: img_mgmt: Add optional max image size reduction #66615

Merged
merged 4 commits into from
Jan 30, 2024
Merged
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
3 changes: 3 additions & 0 deletions include/zephyr/mgmt/mcumgr/grp/img_mgmt/img_mgmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ enum img_mgmt_err_code_t {

/** Setting test to active slot is not allowed */
IMG_MGMT_ERR_IMAGE_SETTING_TEST_TO_ACTIVE_DENIED,

/** Current active slot for image cannot be determined */
IMG_MGMT_ERR_ACTIVE_SLOT_NOT_KNOWN,
};

/**
Expand Down
9 changes: 9 additions & 0 deletions modules/Kconfig.mcuboot
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,15 @@ config MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP_WITH_REVERT
to downgrade running application, but note that MCUboot may do that
if application with higher version will not get confirmed.

config MCUBOOT_BOOTLOADER_MODE_FIRMWARE_UPDATER
bool "MCUboot has been configured in firmware updater mode"
select MCUBOOT_IMGTOOL_OVERWRITE_ONLY
help
MCUboot will only boot slot0_partition for the main application but has
an entrance mechanism defined for entering the slot1_partition which is
a dedicated firmware updater application used to update the slot0_partition
application.

endchoice # MCUBOOT_BOOTLOADER_MODE

config MCUBOOT_BOOTLOADER_MODE_HAS_NO_DOWNGRADE
Expand Down
17 changes: 12 additions & 5 deletions subsys/dfu/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ config MCUBOOT_IMG_MANAGER

endchoice

if MCUBOOT_IMG_MANAGER

config MCUBOOT_SHELL
bool "MCUboot shell"
depends on MCUBOOT_IMG_MANAGER
depends on SHELL
help
Enable shell module, which provides information about image slots and
Expand All @@ -43,25 +44,30 @@ config MCUBOOT_SHELL
config MCUBOOT_TRAILER_SWAP_TYPE
bool "use trailer's swap_type field"
default y
depends on MCUBOOT_IMG_MANAGER
help
Enables usage swap type field which is required after
"Fix double swap on interrupted revert" mcuboot patch
(https://github.com/JuulLabs-OSS/mcuboot/pull/485)
Disable this option if need to be compatible with earlier version
of MCUBoot.

config MCUBOOT_UPDATE_FOOTER_SIZE
hex "Estimated update footer size"
default 0x0
help
Estimated size of update image data, which is used to prevent loading of firmware updates
that MCUboot cannot update due to being too large. This should be set from sysbuild, only
used when MCUMGR_GRP_IMG_TOO_LARGE_SYSBUILD is enabled.

config IMG_BLOCK_BUF_SIZE
int "Image writer buffer size"
depends on MCUBOOT_IMG_MANAGER
default 512
help
Size (in Bytes) of buffer for image writer. Must be a multiple of
the access alignment required by used flash driver.

config IMG_ERASE_PROGRESSIVELY
bool "Erase flash progressively when receiving new firmware"
depends on MCUBOOT_IMG_MANAGER
select STREAM_FLASH_ERASE
help
If enabled, flash is erased as necessary when receiving new firmware,
Expand All @@ -71,7 +77,6 @@ config IMG_ERASE_PROGRESSIVELY

config IMG_ENABLE_IMAGE_CHECK
bool "Image check functions"
depends on MCUBOOT_IMG_MANAGER
select FLASH_AREA_CHECK_INTEGRITY
help
If enabled, there will be available the function to check flash
Expand All @@ -80,6 +85,8 @@ config IMG_ENABLE_IMAGE_CHECK
Another use is to ensure that firmware upgrade routines from internet
server to flash slot are performing properly.

endif # MCUBOOT_IMG_MANAGER

module = IMG_MANAGER
module-str = image manager
source "subsys/logging/Kconfig.template.log_config"
Expand Down
32 changes: 32 additions & 0 deletions subsys/mgmt/mcumgr/grp/img_mgmt/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,38 @@ config MCUMGR_GRP_IMG_MUTEX
can be used by applications to reset the image management state (useful if there are
multiple ways that firmware updates can be loaded).

choice MCUMGR_GRP_IMG_TOO_LARGE_CHECK
prompt "Image size check overhead"
default MCUMGR_GRP_IMG_TOO_LARGE_DISABLED
help
MCUboot images should be limited to the maximum size that the bootloader can swap, in
order to know this size, additional information is needed from the MCUboot
configuration, otherwise an image can be uploaded that is too large for the bootloader
to swap, this selects which method to use.

Note: setting this to a non-disabled option will prevent uploading of padded and
confirmed images, if support for that is required then this feature should be left as
disabled.
Comment on lines +165 to +167
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be possible for us to develop MCUboot/bootutil function that would check the image for valid footer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not possible, well, depending on what you mean by valid footer. Only MCUboot can confirm the image properly due to possibly being encrypted, an application could do some minor validation but nothing more than that

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually you can use imgtool to pad, confirm and encrypt image.
But yes, I guess you can not check the footer if the MCUboot only knows the key.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also can't check that here because to check that you need to receive the whole image

Copy link
Collaborator

Choose a reason for hiding this comment

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

The image slot trailer, the one that stores swap bits, etc, is not encrypted. actually.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not encrypted no but only mcuboot can verify that it is a valid image or supported on that device, the application can't know anything more than the basic validity of parts of the image


config MCUMGR_GRP_IMG_TOO_LARGE_DISABLED
bool "Disabled"
help
Will not take MCUboot configuration into account when checking for maximum file size.

config MCUMGR_GRP_IMG_TOO_LARGE_SYSBUILD
bool "Via Sysbuild from MCUboot configuration"
depends on ROM_END_OFFSET > 0 || MCUBOOT_UPDATE_FOOTER_SIZE > 0
help
Will use the image overhead size calculated during Sysbuild image configuration.

config MCUMGR_GRP_IMG_TOO_LARGE_BOOTLOADER_INFO
bool "Via retention bootloader info"
depends on RETENTION_BOOTLOADER_INFO_OUTPUT_FUNCTION
help
Will fetch the maximum image size from the bootloader info retention subsystem module.
Comment on lines +180 to +184
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be able to, also, provide this info at compile time. At all we are able to calculate space needed for footer in MCUboot, when we know size of slot and write block size.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is provided at compile time when using the sysbuild option. If not using sysbuild then you cannot get that information


endchoice

module = MCUMGR_GRP_IMG
module-str = mcumgr_grp_img
source "subsys/logging/Kconfig.template.log_config"
Expand Down
84 changes: 84 additions & 0 deletions subsys/mgmt/mcumgr/grp/img_mgmt/src/zephyr_img_mgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

#include <mgmt/mcumgr/grp/img_mgmt/img_mgmt_priv.h>

#if defined(CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_BOOTLOADER_INFO)
#include <zephyr/retention/retention.h>
#include <zephyr/retention/blinfo.h>
#endif

LOG_MODULE_DECLARE(mcumgr_img_grp, CONFIG_MCUMGR_GRP_IMG_LOG_LEVEL);

#define SLOT0_PARTITION slot0_partition
Expand Down Expand Up @@ -564,6 +569,18 @@ int img_mgmt_upload_inspect(const struct img_mgmt_upload_req *req,
if (req->off == 0) {
/* First upload chunk. */
const struct flash_area *fa;
#if defined(CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_SYSBUILD) && \
(defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_WITHOUT_SCRATCH) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_SCRATCH) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_OVERWRITE_ONLY) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP_WITH_REVERT)) && \
CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE > 0
const struct flash_area *fa_current;
int current_img_area;
#elif defined(CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_BOOTLOADER_INFO)
int max_image_size;
#endif

if (req->img_data.len < sizeof(struct image_header)) {
/* Image header is the first thing in the image */
Expand All @@ -576,6 +593,7 @@ int img_mgmt_upload_inspect(const struct img_mgmt_upload_req *req,
IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(action, img_mgmt_err_str_hdr_malformed);
return IMG_MGMT_ERR_INVALID_LENGTH;
}

action->size = req->size;

hdr = (struct image_header *)req->img_data.value;
Expand Down Expand Up @@ -626,6 +644,72 @@ int img_mgmt_upload_inspect(const struct img_mgmt_upload_req *req,
return IMG_MGMT_ERR_INVALID_IMAGE_TOO_LARGE;
}

#if defined(CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_SYSBUILD) && \
(defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_WITHOUT_SCRATCH) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_SWAP_SCRATCH) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_OVERWRITE_ONLY) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP) || \
defined(CONFIG_MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP_WITH_REVERT)) && \
CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE > 0
/* Check if slot1 is larger than slot0 by the update size, if so then the size
* check can be skipped because the devicetree partitions are okay
*/
current_img_area = img_mgmt_flash_area_id(req->image);

if (current_img_area < 0) {
/* Current slot cannot be determined */
LOG_ERR("Failed to determine active slot for image %d: %d", req->image,
current_img_area);
return IMG_MGMT_ERR_ACTIVE_SLOT_NOT_KNOWN;
}

rc = flash_area_open(current_img_area, &fa_current);
if (rc) {
IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(action,
img_mgmt_err_str_flash_open_failed);
LOG_ERR("Failed to open flash area ID %u: %d", current_img_area, rc);
flash_area_close(fa);
return IMG_MGMT_ERR_FLASH_OPEN_FAILED;
}

flash_area_close(fa_current);

LOG_DBG("Primary size: %d, secondary size: %d, overhead: %d, max update size: %d",
fa_current->fa_size, fa->fa_size, CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE,
(fa->fa_size + CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE));

if (fa_current->fa_size >= (fa->fa_size + CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE)) {
/* Upgrade slot is of sufficient size, nothing to check */
LOG_INF("Upgrade slots already sized appropriately, "
"CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_SYSBUILD is not needed");
goto skip_size_check;
}

if (req->size > (fa->fa_size - CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE)) {
IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(action,
img_mgmt_err_str_image_too_large);
flash_area_close(fa);
LOG_ERR("Upload too large for slot (with end offset): %u > %u", req->size,
(fa->fa_size - CONFIG_MCUBOOT_UPDATE_FOOTER_SIZE));
return IMG_MGMT_ERR_INVALID_IMAGE_TOO_LARGE;
}

skip_size_check:
#elif defined(CONFIG_MCUMGR_GRP_IMG_TOO_LARGE_BOOTLOADER_INFO)
rc = blinfo_lookup(BLINFO_MAX_APPLICATION_SIZE, (char *)&max_image_size,
sizeof(max_image_size));

if (rc == sizeof(max_image_size) && max_image_size > 0 &&
req->size > max_image_size) {
IMG_MGMT_UPLOAD_ACTION_SET_RC_RSN(action,
img_mgmt_err_str_image_too_large);
flash_area_close(fa);
LOG_ERR("Upload too large for slot (with max image size): %u > %u",
req->size, max_image_size);
return IMG_MGMT_ERR_INVALID_IMAGE_TOO_LARGE;
}
#endif

#if defined(CONFIG_MCUMGR_GRP_IMG_REJECT_DIRECT_XIP_MISMATCHED_SLOT)
if (hdr->ih_flags & IMAGE_F_ROM_FIXED) {
if (fa->fa_off != hdr->ih_load_addr) {
Expand Down
2 changes: 2 additions & 0 deletions subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ os_mgmt_mcumgr_params(struct smp_streamer *ctxt)
#define BOOTLOADER_MODE MCUBOOT_MODE_DIRECT_XIP
#elif IS_ENABLED(CONFIG_MCUBOOT_BOOTLOADER_MODE_DIRECT_XIP_WITH_REVERT)
#define BOOTLOADER_MODE MCUBOOT_MODE_DIRECT_XIP_WITH_REVERT
#elif IS_ENABLED(CONFIG_MCUBOOT_BOOTLOADER_MODE_FIRMWARE_UPDATER)
#define BOOTLOADER_MODE MCUBOOT_MODE_FIRMWARE_LOADER
#else
#define BOOTLOADER_MODE -1
#endif
Expand Down
2 changes: 1 addition & 1 deletion west.yml
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ manifest:
groups:
- crypto
- name: mcuboot
revision: f09e205b1e4a8d2bc3f50dffa7960d6ccd14df59
revision: a4eda30f5b0cfd0cf15512be9dcd559239dbfc91
path: bootloader/mcuboot
- name: mipi-sys-t
path: modules/debug/mipi-sys-t
Expand Down
Loading