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

Conversation

nordicjm
Copy link
Collaborator

Adds an optional feature that can be used to reduce the maximum allowed image upload file size whereby an image could be uploaded that would be too large to swap even if it could fit the partition

Fixes #46194

Comment on lines +165 to +167
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.
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

Comment on lines +180 to +184
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.
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

de-nordic
de-nordic previously approved these changes Jan 26, 2024
@nordicjm nordicjm added the DNM This PR should not be merged (Do Not Merge) label Jan 29, 2024
@zephyrbot zephyrbot added area: DFU Device Firmware Upgrade area: MCUBoot labels Jan 29, 2024
@zephyrbot zephyrbot requested a review from d3zd3z January 29, 2024 11:21
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jan 29, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
mcuboot zephyrproject-rtos/mcuboot@f09e205 (main) zephyrproject-rtos/mcuboot@a4eda30 (upstream-sync) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@nordicjm nordicjm requested a review from de-nordic January 29, 2024 11:22
@nordicjm nordicjm force-pushed the rejecttoolarge branch 3 times, most recently from 23ba39f to 6071453 Compare January 29, 2024 11:51
Adds firmware uploader to the output of bootloader mode for
MCUboot

Signed-off-by: Jamie McCrae <[email protected]>
Adds support for an overhead size which MCUboot can set when using
sysbuild, this can be used to check the provided size of an
application being uploaded to ensure it will fit and swap without
being rejected

Signed-off-by: Jamie McCrae <[email protected]>
Update Zephyr fork of MCUboot to revision:
  a4eda30f5b0cfd0cf15512be9dcd559239dbfc91

Brings following Zephyr relevant fixes:
 - a4eda30f zephyr: Add estimated size of update trailer to
   sysbuild
 - 205d7e5b boot_serial: Adapt to zcbor 0.8.x

Signed-off-by: Jamie McCrae <[email protected]>
Adds an optional feature that can be used to reduce the maximum
allowed image upload file size whereby an image could be uploaded
that would be too large to swap even if it could fit the partition

Signed-off-by: Jamie McCrae <[email protected]>
@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Jan 30, 2024
@carlescufi carlescufi merged commit b26a4bf into zephyrproject-rtos:main Jan 30, 2024
21 of 22 checks passed
@nordicjm nordicjm deleted the rejecttoolarge branch July 12, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

mgmt/mcumgr/lib: Image upload should reject image that is too big to mcuboot to swap
5 participants