Imgtool trailer size check incorrect for swap-using-move

[heinwessels]

What

The calculation of the trailer size (_trailer_size()) is incorrect when the bootloader strategy is swap using move. It does not take the flash sector size into account, but rather the flash write size. For devices such as the STM32H7 the write size (32) is much smaller than the sector size (128k), which results in the calculated trailer size being much smaller than it will be in reality.

This will result in the image being signed successfully. But when this image ends up in the primary slot, and a swap is requested, the swap will fail and the bootloader will panic.

I: Starting bootloader. optima-platform: v0.5.2-dev.49+C3F25B1AB
I: Primary image: magic=good, swap_type=0x2, copy_done=0x1, image_ok=0x1
I: Secondary image: magic=good, swap_type=0x2, copy_done=0x3, image_ok=0x3
I: Boot source: none
I: Swap type: test
I: Starting swap using move algorithm.
W: Not enough free space to run swap upgrade
W: required 1048576 bytes but only 917504 are available
E: panic!

In this example the available 917504 is correct (7 * 128k = 917504), but the image was not 1048576 and instead 787416. Note: these logs are from multiple different trail and errors, so the actual image size might be off by a few bytes.

It fails on this line:

if (it.tlv_end > bootutil_max_image_size(fap)) { 
        rc = -1;
        goto out;
}

where:

it.tlv_end = 787752
bootutil_max_image_size(fap) = 786432

which then prints the values relative to the sector sizes.

            BOOT_LOG_WRN("required %d bytes but only %d are available",
                         (last_idx + 1) * sector_sz,
                         first_trailer_idx * sector_sz);

Expected behaviour

The imgtool take take the boot stategy into account, and fail while signing. And not fail in when an update is requested.

Impact

Quite bad, as it can brick your device in some circumstances. For example in our case where we only have a connection to the device through the application, and rely on the application to update. If the bootloader can't boot then the device is bricked.

More notes:

As far as I know the imgtool doesn't currently know what the boot strategy is, and can therefore not really know what size the trailer will be. For example, the code in the bootloader that determines the max image size depends on the boot strategy:

uint32_t bootutil_max_image_size(const struct flash_area *fap)
{
#if defined(MCUBOOT_SWAP_USING_SCRATCH) || defined(MCUBOOT_SINGLE_APPLICATION_SLOT)
    return boot_status_off(fap);
#elif defined(MCUBOOT_SWAP_USING_MOVE)
    struct flash_sector sector;
    /* get the last sector offset */
    int rc = flash_area_get_sector(fap, boot_status_off(fap), &sector);
    if (rc) {
        BOOT_LOG_ERR("Unable to determine flash sector of the image trailer");
        return 0; /* Returning of zero here should cause any check which uses
                   * this value to fail.
                   */
    }
    return flash_sector_get_off(&sector);
#elif defined(MCUBOOT_OVERWRITE_ONLY)
    return boot_swap_info_off(fap);
#elif defined(MCUBOOT_DIRECT_XIP)
    return boot_swap_info_off(fap);
#elif defined(MCUBOOT_RAM_LOAD)
    return boot_swap_info_off(fap);
#endif

Maybe it's worth adding the swap strategy as argument to the imgtool. Then if it's available, and the stategy is swap-using-move, it can calculate the trailer size correctly.

Our setup

mcuboot: 1558e7ab0aadb4eac11f03befb5ccd3fa3f0aafe

The overlay file

        /* 128KB for bootloader in the 1st bank */
        boot_partition: partition@0 {
            label = "mcuboot";
            reg = <0x00000000 DT_SIZE_K(128)>;
            read-only;
        };

        /* application image slot: 896kB + 128KB in the 1st bank.
         * The extra slot is used for the bootloader's swap using move
         */
        slot0_partition: partition@20000 {
            label = "image-0";
            reg = <0x00020000 DT_SIZE_K(1024)>;
        };

        /* application image slot: 896 KB */
        slot1_partition: partition@120000 {
            label = "image-1";
            reg = <0x00120000 DT_SIZE_K(896)>;
        };

[nordicjm]

There is a way of knowing this before beginning an upload if you use MCUboot's data sharing, one of the values provided there is the maximum application size, so you can check the file size from the application to know if it would fit or not. But yes, ideally this should be fixed in imgtool. And it's not just a case of not the last sector as far as I'm aware, because it depends on how many sectors that your firmware used depends upon how many instances of this it requires: https://docs.mcuboot.com/design.html#swap-status - that might fit into a single sector if you have 128KB sectors but if you have e.g. 512 byte sectors, it could take many sectors.

[nordicjm]

Fix (for zephyr itself): zephyrproject-rtos/zephyr#64586 can see you are seeing a different issue

[heinwessels]

Fix (for zephyr itself): zephyrproject-rtos/zephyr#64586 can see you are seeing a different issue

True, but we might still be able to use this to prevent it from compiling, and not only fail during signing. We are using swap-using-move, so the code-partition slot the application is built for is 2 sectors bigger than we actually have available. Setting the kconfig offset you are adding to the size of two sectors should fix our problem.

Thanks!

[nordicjm]

Fix (for zephyr itself): zephyrproject-rtos/zephyr#64586 can see you are seeing a different issue

True, but we might still be able to use this to prevent it from compiling, and not only fail during signing. We are using swap-using-move, so the code-partition slot the application is built for is 2 sectors bigger than we actually have available. Setting the kconfig offset you are adding to the size of two sectors should fix our problem.

Thanks!

Since you're using an stm32 and I've not tested it with that, could you give the attached PR a try (configure the application as normal, note: requires using sysbuild) then run ninja menuconfig and check what FLASH_LOAD_OVERHEAD_SIZE is set to?

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.