[nrf fromlist] bootutil: Unify app_max_size() implementations

tomchy · nordicjm · commit cf1f76cfb843 · 2025-07-14T08:59:33.000+01:00
Remove redundant application size calculations in favor of
a swap-specific function, implemented inside swap_&lt;type&gt;.c.
In this way, slot sizes use the same restrictions as image validation.

Upstream PR #: 2318

Signed-off-by: Tomasz Chyrowicz &lt;tomasz.chyrowicz@nordicsemi.no&gt;
diff --git a/boot/bootutil/include/bootutil/bootutil_public.h b/boot/bootutil/include/bootutil/bootutil_public.h
@@ -295,6 +295,12 @@ boot_set_next(const struct flash_area *fa, bool active, bool confirm);
 /**
  * Attempts to load image header from flash; verifies flash header fields.
  *
+ * The selected update method (i.e. swap move) may impose additional restrictions
+ * on the image size (i.e. due to the presence of the image trailer).
+ * Such restrictions are not verified by this function.
+ * These checks are implemented as part of the boot_image_validate(..) that uses
+ * sizes from the bootutil_max_image_size(..).
+ *
  * @param[in]   fa_p    flash area pointer
  * @param[out]  hdr     buffer for image header
  *
diff --git a/boot/bootutil/src/bootutil_misc.c b/boot/bootutil/src/bootutil_misc.c
@@ -42,6 +42,10 @@
 #ifdef MCUBOOT_ENC_IMAGES
 #include "bootutil/enc_key.h"
 #endif
+#if defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET) || \
+    defined(MCUBOOT_SWAP_USING_SCRATCH)
+#include "swap_priv.h"
+#endif
 
 #if defined(MCUBOOT_DECOMPRESS_IMAGES)
 #include <nrf_compress/implementation.h>
@@ -240,8 +244,7 @@ int boot_header_scramble_off_sz(const struct flash_area *fa, int slot, size_t *o
  * status during the swap of the last sector from primary/secondary (which
  * is the first swap operation) and thus only requires space for one swap.
  */
-static uint32_t
-boot_scratch_trailer_sz(uint32_t min_write_sz)
+uint32_t boot_scratch_trailer_sz(uint32_t min_write_sz)
 {
     return boot_status_entry_sz(min_write_sz) + boot_trailer_info_sz();
 }
@@ -427,44 +430,6 @@ boot_write_enc_key(const struct flash_area *fap, uint8_t slot,
 }
 #endif
 
-#ifdef MCUBOOT_SWAP_USING_SCRATCH
-size_t
-boot_get_first_trailer_sector(struct boot_loader_state *state, size_t slot, size_t trailer_sz)
-{
-    size_t first_trailer_sector = boot_img_num_sectors(state, slot) - 1;
-    size_t sector_sz = boot_img_sector_size(state, slot, first_trailer_sector);
-    size_t trailer_sector_sz = sector_sz;
-
-    while (trailer_sector_sz < trailer_sz) {
-        /* Consider that the image trailer may span across sectors of different sizes */
-        --first_trailer_sector;
-        sector_sz = boot_img_sector_size(state, slot, first_trailer_sector);
-
-        trailer_sector_sz += sector_sz;
-    }
-
-    return first_trailer_sector;
-}
-
-/**
- * Returns the offset to the end of the first sector of a given slot that holds image trailer data.
- *
- * @param state      Current bootloader's state.
- * @param slot       The index of the slot to consider.
- * @param trailer_sz The size of the trailer, in bytes.
- *
- * @return The offset to the end of the first sector of the slot that holds image trailer data.
- */
-static uint32_t
-get_first_trailer_sector_end_off(struct boot_loader_state *state, size_t slot, size_t trailer_sz)
-{
-    size_t first_trailer_sector = boot_get_first_trailer_sector(state, slot, trailer_sz);
-
-    return boot_img_sector_off(state, slot, first_trailer_sector) +
-           boot_img_sector_size(state, slot, first_trailer_sector);
-}
-#endif /* MCUBOOT_SWAP_USING_SCRATCH */
-
 uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct flash_area *fap)
 {
 #if defined(CONFIG_MCUBOOT_MCUBOOT_IMAGE_NUMBER) && CONFIG_MCUBOOT_MCUBOOT_IMAGE_NUMBER != -1
@@ -484,61 +449,10 @@ uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct f
     defined(MCUBOOT_SINGLE_APPLICATION_SLOT_RAM_LOAD)
     (void) state;
     return boot_status_off(fap);
-#elif defined(MCUBOOT_SWAP_USING_SCRATCH)
-    size_t slot_trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
-    size_t slot_trailer_off = flash_area_get_size(fap) - slot_trailer_sz;
-
-    /* If the trailer doesn't fit in the last sector of the primary or secondary slot, some padding
-     * might have to be inserted between the end of the firmware image and the beginning of the
-     * trailer to ensure there is enough space for the trailer in the scratch area when the last
-     * sector of the secondary will be copied to the scratch area.
-     *
-     * The value of the padding depends on the amount of trailer data that is contained in the first
-     * trailer containing part of the trailer in the primary and secondary slot.
-     */
-    size_t trailer_sector_primary_end_off =
-        get_first_trailer_sector_end_off(state, BOOT_PRIMARY_SLOT, slot_trailer_sz);
-    size_t trailer_sector_secondary_end_off =
-        get_first_trailer_sector_end_off(state, BOOT_SECONDARY_SLOT, slot_trailer_sz);
-
-    size_t trailer_sz_in_first_sector;
-
-    if (trailer_sector_primary_end_off > trailer_sector_secondary_end_off) {
-        trailer_sz_in_first_sector = trailer_sector_primary_end_off - slot_trailer_off;
-    } else {
-        trailer_sz_in_first_sector = trailer_sector_secondary_end_off - slot_trailer_off;
-    }
-
-    size_t trailer_padding = 0;
-    size_t scratch_trailer_sz = boot_scratch_trailer_sz(BOOT_WRITE_SZ(state));
-
-    if (scratch_trailer_sz > trailer_sz_in_first_sector) {
-        trailer_padding = scratch_trailer_sz - trailer_sz_in_first_sector;
-    }
-
-    return slot_trailer_off - trailer_padding;
-#elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET)
+#elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET) \
+      || defined(MCUBOOT_SWAP_USING_SCRATCH)
     (void) fap;
-
-    /* The slot whose size is used to compute the maximum image size must be the one containing the
-     * padding required for the swap. */
-#ifdef MCUBOOT_SWAP_USING_MOVE
-    size_t slot = BOOT_PRIMARY_SLOT;
-#else
-    size_t slot = BOOT_SECONDARY_SLOT;
-#endif
-
-    const struct flash_area *fap_padded_slot = BOOT_IMG_AREA(state, slot);
-    assert(fap_padded_slot != NULL);
-
-    size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
-    size_t sector_sz = boot_img_sector_size(state, slot, 0);
-    size_t padding_sz = sector_sz;
-
-    /* The trailer size needs to be sector-aligned */
-    trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
-
-    return flash_area_get_size(fap_padded_slot) - trailer_sz - padding_sz;
+    return app_max_size(state);
 #elif defined(MCUBOOT_OVERWRITE_ONLY)
     (void) state;
     return boot_swap_info_off(fap);
diff --git a/boot/bootutil/src/bootutil_priv.h b/boot/bootutil/src/bootutil_priv.h
@@ -363,18 +363,14 @@ int boot_read_enc_key(const struct flash_area *fap, uint8_t slot,
                       struct boot_status *bs);
 #endif
 
-#ifdef MCUBOOT_SWAP_USING_SCRATCH
-/**
- * Finds the first sector of a given slot that holds image trailer data.
- *
- * @param state      Current bootloader's state.
- * @param slot       The index of the slot to consider.
- * @param trailer_sz The size of the trailer, in bytes.
- *
- * @return The index of the first sector of the slot that holds image trailer data.
+#if MCUBOOT_SWAP_USING_SCRATCH
+/*
+ * Similar to `boot_trailer_sz` but this function returns the space used to
+ * store status in the scratch partition. The scratch partition only stores
+ * status during the swap of the last sector from primary/secondary (which
+ * is the first swap operation) and thus only requires space for one swap.
  */
-size_t
-boot_get_first_trailer_sector(struct boot_loader_state *state, size_t slot, size_t trailer_sz);
+uint32_t boot_scratch_trailer_sz(uint32_t min_write_sz);
 #endif
 
 /**
diff --git a/boot/bootutil/src/swap_move.c b/boot/bootutil/src/swap_move.c
@@ -227,29 +227,6 @@ boot_status_internal_off(const struct boot_status *bs, int elem_sz)
     return off;
 }
 
-static int app_max_sectors(struct boot_loader_state *state)
-{
-    uint32_t sz = 0;
-    uint32_t sector_sz;
-    uint32_t trailer_sz;
-    uint32_t first_trailer_idx;
-
-    sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-    trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
-    /* subtract 1 for swap and at least 1 for trailer */
-    first_trailer_idx = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - 2;
-
-    while (1) {
-        sz += sector_sz;
-        if  (sz >= trailer_sz) {
-            break;
-        }
-        first_trailer_idx--;
-    }
-
-    return first_trailer_idx;
-}
-
 int
 boot_slots_compatible(struct boot_loader_state *state)
 {
@@ -270,19 +247,16 @@ boot_slots_compatible(struct boot_loader_state *state)
     size_t sector_sz_pri = 0;
     size_t sector_sz_sec = 0;
     size_t i;
-    size_t num_usable_sectors_pri;
 
     num_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT);
     num_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT);
-    num_usable_sectors_pri = app_max_sectors(state);
 
     if ((num_sectors_pri != num_sectors_sec) &&
-            (num_sectors_pri != (num_sectors_sec + 1)) &&
-            (num_usable_sectors_pri != (num_sectors_sec + 1))) {
+            (num_sectors_pri != (num_sectors_sec + 1))) {
         BOOT_LOG_WRN("Cannot upgrade: not a compatible amount of sectors");
         BOOT_LOG_DBG("slot0 sectors: %d, slot1 sectors: %d, usable slot0 sectors: %d",
                      (int)num_sectors_pri, (int)num_sectors_sec,
-                     (int)(num_usable_sectors_pri - 1));
+                     (int)(num_sectors_pri - 1));
         return 0;
     } else if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
         BOOT_LOG_WRN("Cannot upgrade: more sectors than allowed");
@@ -292,7 +266,7 @@ boot_slots_compatible(struct boot_loader_state *state)
     /* Optimal says primary has one more than secondary. Always. Both have trailers. */
     if (num_sectors_pri != (num_sectors_sec + 1)) {
         BOOT_LOG_DBG("Non-optimal sector distribution, slot0 has %d usable sectors (%d assigned) "
-                     "but slot1 has %d assigned", (int)num_usable_sectors_pri,
+                     "but slot1 has %d assigned", (int)(num_sectors_pri - 1),
                      (int)num_sectors_pri, (int)num_sectors_sec);
     }
 
@@ -353,7 +327,6 @@ swap_status_source(struct boot_loader_state *state)
     struct boot_swap_state state_primary_slot;
     struct boot_swap_state state_secondary_slot;
     int rc;
-    uint8_t source;
     uint8_t image_index;
 
 #if (BOOT_IMAGE_NUMBER == 1)
@@ -378,10 +351,8 @@ swap_status_source(struct boot_loader_state *state)
             state_primary_slot.copy_done == BOOT_FLAG_UNSET &&
             state_secondary_slot.magic != BOOT_MAGIC_GOOD) {
 
-        source = BOOT_STATUS_SOURCE_PRIMARY_SLOT;
-
         BOOT_LOG_INF("Boot source: primary slot");
-        return source;
+        return BOOT_STATUS_SOURCE_PRIMARY_SLOT;
     }
 
     BOOT_LOG_INF("Boot source: none");
@@ -603,11 +574,23 @@ swap_run(struct boot_loader_state *state, struct boot_status *bs,
 
 int app_max_size(struct boot_loader_state *state)
 {
-    uint32_t sector_sz_primary;
+    uint32_t available_pri_sz;
+    uint32_t available_sec_sz;
+
+    size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
+    size_t sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
+    size_t padding_sz = sector_sz;
 
-    sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
+    /* The trailer size needs to be sector-aligned */
+    trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
+
+    /* The slot whose size is used to compute the maximum image size must be the one containing the
+     * padding required for the swap.
+     */
+    available_pri_sz = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) * sector_sz - trailer_sz - padding_sz;
+    available_sec_sz = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) * sector_sz - trailer_sz;
 
-    return app_max_sectors(state) * sector_sz_primary;
+    return (available_pri_sz < available_sec_sz ? available_pri_sz : available_sec_sz);
 }
 
 #endif
diff --git a/boot/bootutil/src/swap_offset.c b/boot/bootutil/src/swap_offset.c
@@ -302,66 +302,37 @@ uint32_t boot_status_internal_off(const struct boot_status *bs, int elem_sz)
     return off;
 }
 
-static int app_max_sectors(struct boot_loader_state *state)
-{
-    uint32_t sz = 0;
-    uint32_t sector_sz;
-    uint32_t trailer_sz;
-    uint32_t available_sectors_pri;
-    uint32_t available_sectors_sec;
-    uint32_t trailer_sectors = 0;
-
-    sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-    trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
-
-    while (1) {
-        sz += sector_sz;
-        ++trailer_sectors;
-
-        if  (sz >= trailer_sz) {
-            break;
-        }
-    }
-
-    available_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) - trailer_sectors;
-    available_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) - 1;
-
-    return (available_sectors_pri < available_sectors_sec ? available_sectors_pri : available_sectors_sec);
-}
-
 int boot_slots_compatible(struct boot_loader_state *state)
 {
     size_t num_sectors_pri;
     size_t num_sectors_sec;
     size_t sector_sz_pri = 0;
     size_t sector_sz_sec = 0;
     size_t i;
-    size_t num_usable_sectors;
 
     num_sectors_pri = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT);
     num_sectors_sec = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT);
-    num_usable_sectors = app_max_sectors(state);
 
     if (num_sectors_pri != num_sectors_sec &&
-        (num_sectors_pri + 1) != num_sectors_sec &&
-        num_usable_sectors != (num_sectors_sec - 1)) {
+        (num_sectors_pri + 1) != num_sectors_sec) {
         BOOT_LOG_WRN("Cannot upgrade: not a compatible amount of sectors");
         BOOT_LOG_DBG("slot0 sectors: %d, slot1 sectors: %d, usable sectors: %d",
                      (int)num_sectors_pri, (int)num_sectors_sec,
-                     (int)(num_usable_sectors));
+                     (int)(num_sectors_sec - 1));
         return 0;
     } else if (num_sectors_pri > BOOT_MAX_IMG_SECTORS) {
         BOOT_LOG_WRN("Cannot upgrade: more sectors than allowed");
         return 0;
     }
 
-    if ((num_usable_sectors + 1) != num_sectors_sec) {
+    /* Optimal says secondary has one more than primary. Always. Both have trailers. */
+    if ((num_sectors_pri + 1) != num_sectors_sec) {
         BOOT_LOG_DBG("Non-optimal sector distribution, slot0 has %d usable sectors "
-                     "but slot1 has %d usable sectors", (int)(num_usable_sectors),
+                     "but slot1 has %d usable sectors", (int)(num_sectors_pri),
                      ((int)num_sectors_sec - 1));
     }
 
-    for (i = 0; i < num_usable_sectors; i++) {
+    for (i = 0; i < (num_sectors_sec - 1); i++) {
         sector_sz_pri = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, i);
         sector_sz_sec = boot_img_sector_size(state, BOOT_SECONDARY_SLOT, i);
 
@@ -417,7 +388,6 @@ int swap_status_source(struct boot_loader_state *state)
     struct boot_swap_state state_primary_slot;
     struct boot_swap_state state_secondary_slot;
     int rc;
-    uint8_t source;
     uint8_t image_index;
 
 #if (BOOT_IMAGE_NUMBER == 1)
@@ -439,10 +409,8 @@ int swap_status_source(struct boot_loader_state *state)
         state_primary_slot.copy_done == BOOT_FLAG_UNSET &&
         state_secondary_slot.magic != BOOT_MAGIC_GOOD) {
 
-        source = BOOT_STATUS_SOURCE_PRIMARY_SLOT;
-
         BOOT_LOG_INF("Boot source: primary slot");
-        return source;
+        return BOOT_STATUS_SOURCE_PRIMARY_SLOT;
     }
 
     BOOT_LOG_INF("Boot source: none");
@@ -729,11 +697,23 @@ void swap_run(struct boot_loader_state *state, struct boot_status *bs,
 
 int app_max_size(struct boot_loader_state *state)
 {
-    uint32_t sector_sz_primary;
+    uint32_t available_pri_sz;
+    uint32_t available_sec_sz;
+
+    size_t trailer_sz = boot_trailer_sz(BOOT_WRITE_SZ(state));
+    size_t sector_sz = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
+    size_t padding_sz = sector_sz;
+
+    /* The trailer size needs to be sector-aligned */
+    trailer_sz = ALIGN_UP(trailer_sz, sector_sz);
 
-    sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
+    /* The slot whose size is used to compute the maximum image size must be the one containing the
+     * padding required for the swap.
+     */
+    available_pri_sz = boot_img_num_sectors(state, BOOT_PRIMARY_SLOT) * sector_sz - trailer_sz;
+    available_sec_sz = boot_img_num_sectors(state, BOOT_SECONDARY_SLOT) * sector_sz - trailer_sz - padding_sz;
 
-    return app_max_sectors(state) * sector_sz_primary;
+    return (available_pri_sz < available_sec_sz ? available_pri_sz : available_sec_sz);
 }
 
 /* Compute the total size of the given image. Includes the size of the TLVs. */
diff --git a/boot/bootutil/src/swap_scratch.c b/boot/bootutil/src/swap_scratch.c

Original file line number	Diff line number	Diff line change
`@@ -295,6 +295,12 @@ boot_set_next(const struct flash_area *fa, bool active, bool confirm);`
`295`	`295`	`/**`
`296`	`296`	`* Attempts to load image header from flash; verifies flash header fields.`
`297`	`297`	`*`
	`298`	`+ * The selected update method (i.e. swap move) may impose additional restrictions`
	`299`	`+ * on the image size (i.e. due to the presence of the image trailer).`
	`300`	`+ * Such restrictions are not verified by this function.`
	`301`	`+ * These checks are implemented as part of the boot_image_validate(..) that uses`
	`302`	`+ * sizes from the bootutil_max_image_size(..).`
	`303`	`+ *`
`298`	`304`	`* @param[in] fa_p flash area pointer`
`299`	`305`	`* @param[out] hdr buffer for image header`
`300`	`306`	`*`