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

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 ?

Parents

0 Simon over 1 year ago

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

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?

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 ?

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

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

diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index 2a5c89cd..7dd8fa43 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -50,6 +50,8 @@
  
 #if defined(CONFIG_SOC_NRF5340_CPUAPP) && defined(PM_CPUNET_B0N_ADDRESS)
 #include <dfu/pcd.h>
+#include "flash_map_backend/flash_map_backend.h"
+#include <zephyr/drivers/flash/flash_simulator.h>
 #endif
  
 #ifdef MCUBOOT_ENC_IMAGES
@@ -917,9 +919,7 @@ boot_validated_swap_type(struct boot_loader_state *state,
 #if defined(PM_S1_ADDRESS) || defined(CONFIG_SOC_NRF5340_CPUAPP)
     const struct flash_area *secondary_fa =
         BOOT_IMG_AREA(state, BOOT_SECONDARY_SLOT);
-    struct image_header *hdr = (struct image_header *)secondary_fa->fa_off;
-    uint32_t vtable_addr = 0;
-    uint32_t *vtable = 0;
+    struct image_header *hdr = boot_img_hdr(state, BOOT_SECONDARY_SLOT);
     uint32_t reset_addr = 0;
     /* Patch needed for NCS. Since image 0 (the app) and image 1 (the other
      * B1 slot S0 or S1) share the same secondary slot, we need to check
@@ -930,9 +930,12 @@ boot_validated_swap_type(struct boot_loader_state *state,
      */
  
     if (hdr->ih_magic == IMAGE_MAGIC) {
-        vtable_addr = (uint32_t)hdr + hdr->ih_hdr_size;
-        vtable = (uint32_t *)(vtable_addr);
-        reset_addr = vtable[1];
+        int rc = flash_area_read(secondary_fa, hdr->ih_hdr_size +
+                                sizeof(uint32_t), &reset_addr,
+                                sizeof(reset_addr));
+        if (rc != 0) {
+            return BOOT_SWAP_TYPE_FAIL;
+        }
 #ifdef PM_S1_ADDRESS
 #ifdef PM_CPUNET_B0N_ADDRESS
         if(reset_addr < PM_CPUNET_B0N_ADDRESS)
@@ -981,11 +984,29 @@ boot_validated_swap_type(struct boot_loader_state *state,
          * available
          */
         if (upgrade_valid && reset_addr > PM_CPUNET_B0N_ADDRESS) {
-            uint32_t fw_size = hdr->ih_img_size;
-
             BOOT_LOG_INF("Starting network core update");
-            int rc = pcd_network_core_update(vtable, fw_size);
+            static const struct device *mock_flash_dev;
+            void *mock_flash;
+            size_t mock_size;
+
+            BOOT_LOG_INF("Reading update image to flash simulator");
+            //mock_flash_dev = device_get_binding(PM_MCUBOOT_PRIMARY_DEV_NAME);
+            //if (mock_flash_dev == NULL) {
+            mock_flash_dev = DEVICE_DT_GET(DT_NODELABEL(PM_MCUBOOT_PRIMARY_DEV));     
+            if (!device_is_ready(mock_flash_dev)) {
+                swap_type = BOOT_SWAP_TYPE_NONE;
+            }
+            mock_flash = flash_simulator_get_memory(mock_flash_dev, &mock_size);
+            int rc = flash_area_read(secondary_fa, 0U, mock_flash, hdr->ih_hdr_size + hdr->ih_img_size);
+            if (rc != 0) {
+                return BOOT_SWAP_TYPE_FAIL;
+            }
  
+            BOOT_LOG_INF("Starting PCD update to network core");
+            hdr = (struct image_header *) mock_flash;
+            uint32_t vtable_addr = (uint32_t)hdr + hdr->ih_hdr_size;
+            uint32_t *vtable = (uint32_t *)(vtable_addr);
+            rc = pcd_network_core_update(vtable, hdr->ih_img_size);
             if (rc != 0) {
                 swap_type = BOOT_SWAP_TYPE_FAIL;
             } else {
@@ -996,14 +1017,13 @@ boot_validated_swap_type(struct boot_loader_state *state,
                  * the image cannot be reverted.
                  */
                 rc = swap_erase_trailer_sectors(state,
-                        secondary_fa);
+                    secondary_fa);
 #endif
                 swap_type = BOOT_SWAP_TYPE_NONE;
             }
         }
 #endif /* CONFIG_SOC_NRF5340_CPUAPP */
     }
-
     return swap_type;
 }
 #endif

I will get back to you on Monday/Tuesday with more information about this.

Best regards,

Simon

Reply