MCUBoot Update hooks - Rejects any mcuboot updates when used with immutable bootlader

[optical]

Dear nordic,

state of your MCUBoot + Immutable bootloader is just awful. 

Please fix this hook, when enabled it disables updates of the mcuboot bootloader when used with immutable bootloader.
This issue is detected upon next mcuboot update, however then it is too late it will brick all devices due to next update rejection.

https://github.com/nrfconnect/sdk-mcuboot/blob/bf00840a090f396ec1554968e19fa0e02c077d38/boot/zephyr/hooks_sample.c#L57

PS: 

MCUBoot + Immutable bootloader used with external flash does not work due reading the header directly from memory instead of the Flash_area. Honestly did you even tried it ? 
https://github.com/nrfconnect/sdk-mcuboot/blob/bf00840a090f396ec1554968e19fa0e02c077d38/boot/bootutil/src/loader.c#L920
https://github.com/nrfconnect/sdk-mcuboot/blob/bf00840a090f396ec1554968e19fa0e02c077d38/boot/bootutil/src/loader.c#L934

I would submit a patch however you have disabled any issue to the repository. Should i just create pull request ?

[optical]

This hook is not specific to the Nordic downstream MCUboot version, it exist in the upstream MCUboot version as well (see https://github.com/mcu-tools/mcuboot/blob/main/boot/zephyr/hooks_sample.c). Can you create an issue in https://github.com/mcu-tools/mcuboot/issues if there are any issues with this hook?

---

The issue is caused by configuration done by nordic. The mainstream sample prevent the secondary Image(Bootloader) (BootImages compiled into the mcuboot. 0 - App, 1 - Bootloader). The hooks automaticly rejects the secondary image. This should be fixed in the nordic repository to prevent any issue for the end user.
The procedure to test all updates when releasing new version is very tiresome and can lead to user error. We exactly encountered this issue.

I suggest inclusion of following patch to the ncs branch to prevent any future confusion for the users. 

0001-fix-Boot-sample-hooks-modified-to-prevent-any-furthe.patch

The main problem is that this will appear after another bootloader version release, which is not common

. 
Is there a supported way from nordic how to verify that the configured bootloader will perform another update ? Some kind of integration or unit tests ? This is by my opinion a crucial part of the development of critical application part such as bootloader.

---

Are you using nRF9160 with MCUboot+immutable bootloader+external flash?

---

Yes, we are.  For ilustration the partition manager flash map. We are also using Mcuboot scratch partition for the swap operation. We encountered using the default method that when the bootloader expands to the last sector the update process fails. This should be checked by the compilation. It is not. This creates a false notion for the user that he has larger flash area to work with. This will for some users endu fatal during NCS updates of the bootloader. 

  external_flash (0x200000 - 2048kB): 
+-----------------------------------------------+
| 0x0: mcuboot_secondary (0xa8000 - 672kB)      |
| 0xa8000: mcuboot_scratch (0x8000 - 32kB)      |
| 0xb0000: external_storage (0x148000 - 1312kB) |
| 0x1f8000: external_settings (0x8000 - 32kB)   |
| 0x200000: external_flash (0x0 - 0B)           |
+-----------------------------------------------+

  flash_primary (0x100000 - 1024kB): 
+--------------------------------------------------+
+---0x0: b0_container (0x8000 - 32kB)--------------+
| 0x0: b0 (0x8000 - 32kB)                          |
+---0x8000: s0 (0x20200 - 128kB)-------------------+
| 0x8000: s0_pad (0x200 - 512B)                    |
+---0x8200: s0_image (0x20000 - 128kB)-------------+
| 0x8200: mcuboot (0x20000 - 128kB)                |
+--------------------------------------------------+
| 0x28200: EMPTY_0 (0x7e00 - 31kB)                 |
+---0x30000: s1 (0x20200 - 128kB)------------------+
| 0x30000: s1_pad (0x200 - 512B)                   |
| 0x30200: s1_image (0x20000 - 128kB)              |
+--------------------------------------------------+
| 0x50200: EMPTY_1 (0x7e00 - 31kB)                 |
+---0x58000: mcuboot_primary (0xa8000 - 672kB)-----+
| 0x58000: mcuboot_pad (0x200 - 512B)              |
+---0x58200: app_image (0xa7e00 - 671kB)-----------+
+---0x58200: mcuboot_primary_app (0xa7e00 - 671kB)-+
| 0x58200: spm (0x8000 - 32kB)                     |
| 0x60200: app (0x9fe00 - 639kB)                   |
+--------------------------------------------------+

Patch needed for user to be able to define scratch partition using pm_static.yml file inside external flash.
0001-feat-Adding-option-to-specify-user-scratch-partition.patch

---

Can you try this patch on MCUboot associated with NCS v2.1.0? Let me know if it works or not.

---

We are still using the v2.0.0, we will update to the NCS v2.1.0 during following months. However, we are reluctant with the updates due to lack of test for the update process from our side.

We are currently using this patch, which is verified to work.
0001-fix-MCUBoot-Fixed-immutable-bootloader-when-external.patch

Both of the patches are based on following version: 129b6312d61a9dc2c3b0c8810326678cdbd27b80 - v1.9.99-ncs1-rc2

---

What is the correct procedure for updating the bootloader ? We are currently using mcumgr upload -> test -> restart. This is works the main down fall of this method is that after second update(v1(s0) -> v2(s1) -> v3(s0)) the image swaped out from the s0 image will be marked as Test. This leads to bricking the device due to mcumgr not able to erase image marked as test and the update procedure will not unmark(test) the image in the secondary slot.


Do you experience this issue as well ? We solved this issue by modifying zephyr to allow erase of a Test images. By my opinion this should be in upstream as well there are cases where user wants to cancel the update without restarting the device twice.

[Sigurd Hellesvik]

Hi,

Simon is out of office.
Therefore I will continue handling this case.

I will need to take some time to get into what this is about, but I will return with an answer on Monday.

Regards,
Sigurd Hellesvik