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/lib: Image upload should reject image that is too big to mcuboot to swap #46194

Closed
de-nordic opened this issue Jun 2, 2022 · 4 comments · Fixed by #66615
Closed
Assignees
Labels
area: mcumgr Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug

Comments

@de-nordic
Copy link
Collaborator

Is your enhancement proposal related to a problem? Please describe.
Mcuboot may not be able to swap some of images, depending on compiled in upgrade mode, when image size does not leave some additional pages in primary image slot.
Mcumgr is not aware of mcuboot mode so it will just upload any image, to secondary slot, that will fit into primary slot, event if taking all free pages of that slot.
On restart mcuboot would not be able to swap image that is too big but user will not know this until upload is processed and reset is triggered, to perform upgrade.

Describe the solution you'd like
mcumgr update should be able reject images that would not be swappable by mcuboot.
Kconfig options should be added to allow user to compile mcumgr for the same mode of operation the mcuboot is compiled for and logic should be added to check if image, an user tries to upload, will fit the mode.

Describe alternatives you've considered

  1. Leaving as is: this issue can found during tests, prior to deployment, which means that such check would only take flash size, while should be detected "in factory";
  2. Using different slot sizes: making secondary slot shorter than primary, by required number of pages, will prevent mcumgr from uploading image that is too big to be swapped; problem is that different modes of operations of mcuboot would required different DTS overlays to be selected;
  3. Kconfig options that would compile mcumgr to the same mode of operation as mcuboot it is going to work with;
    then the mcumgr can decide whether it wants to accept the image or not, or which DTS partition scheme would be used.

Additional context
TODO

@de-nordic de-nordic added Enhancement Changes/Updates/Additions to existing features area: mcumgr labels Jun 2, 2022
@Laczen
Copy link
Collaborator

Laczen commented Jun 2, 2022

@de-nordic, isn't the dts already showing this information? Existence of a scratch sector, existence of only one image... Adding yet another method do define the same thing seems error prone and probably will not remove the requirement to changes in the dts.

@de-nordic
Copy link
Collaborator Author

@de-nordic, isn't the dts already showing this information? Existence of a scratch sector, existence of only one image... Adding yet another method do define the same thing seems error prone and probably will not remove the requirement to changes in the dts.

Yes it is, but the problem is that you do not have to use the scratch sector even if it exists, so it is not easy to tell, by the existence, what mode of operation does the mcuboot use.
It is also true that when there is only single image visible in DTS that should be only one available option for update.

Perfect situation would be if mcuboot would expose API to ask it for configuration.

@Laczen
Copy link
Collaborator

Laczen commented Jun 3, 2022

Even this perfect situation would not be perfect, it would still require the dts to reflect the correct mcuboot configuration. Otherwise the API returns config 1 (unknown at compile time), while the dts supports config 2.

The perfect situation would be that it doesn't matter what mode mcuboot is using, the dts is useable for all of them.

The origin of this problem is the not ideal method used to support the swap mechanism. This can be avoided but I will leave the mcuboot community to come up with a solution for that.

@nordicjm
Copy link
Collaborator

Idea is to use retention subsystem (on top of retained memory) to allow this information to be propagated from mcuboot to the application

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: mcumgr Enhancement Changes/Updates/Additions to existing features priority: low Low impact/importance bug
Projects
Status: Done
4 participants