bootutil: Unify app_max_size() implementations

Remove redundant application size calculations in favour of
a common function, implemented inside bootutil_misc.c.
In this way, slot sizes use the same restrictions as image validation.

Signed-off-by: Tomasz Chyrowicz <tomasz.chyrowicz@nordicsemi.no>

Author: tomchy

boot/bootutil/include/bootutil/bootutil.h

@@ -92,6 +92,7 @@ struct boot_loader_state *boot_get_loader_state(void);
 const struct image_max_size *boot_get_max_app_size(void);
 uint32_t boot_get_state_secondary_offset(struct boot_loader_state *state,
                                          const struct flash_area *fap);
+uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct flash_area *fap);
 
 #define SPLIT_GO_OK                 (0)
 #define SPLIT_GO_NON_MATCHING       (-1)

boot/bootutil/include/bootutil/bootutil_public.h

@@ -302,6 +302,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
  *

boot/bootutil/src/bootutil_misc.c

@@ -479,25 +479,28 @@ uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct f
 #elif defined(MCUBOOT_SWAP_USING_MOVE) || defined(MCUBOOT_SWAP_USING_OFFSET)
     (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);
+    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, slot, 0);
+    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);
 
-    return flash_area_get_size(fap_padded_slot) - trailer_sz - padding_sz;
+    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;
+
+    /* 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
+    available_pri_sz -= padding_sz;
+#else
+    available_sec_sz -= padding_sz;
+#endif
+
+    return (available_pri_sz < available_sec_sz ? available_pri_sz : available_sec_sz);
 #elif defined(MCUBOOT_OVERWRITE_ONLY)
     (void) state;
     return boot_swap_info_off(fap);

boot/bootutil/src/bootutil_priv.h

@@ -544,8 +544,6 @@ int boot_load_image_to_sram(struct boot_loader_state *state);
 
 #endif /* MCUBOOT_RAM_LOAD */
 
-uint32_t bootutil_max_image_size(struct boot_loader_state *state, const struct flash_area *fap);
-
 int boot_read_image_size(struct boot_loader_state *state, int slot,
                          uint32_t *size);
 

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)
 {
@@ -258,19 +235,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");
@@ -280,7 +254,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);
     }
 
@@ -340,7 +314,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)
@@ -365,10 +338,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");
@@ -590,11 +561,7 @@ 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;
-
-    sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-
-    return app_max_sectors(state) * sector_sz_primary;
+    return bootutil_max_image_size(state, NULL);
 }
 
 #endif

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,7 @@ 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;
-
-    sector_sz_primary = boot_img_sector_size(state, BOOT_PRIMARY_SLOT, 0);
-
-    return app_max_sectors(state) * sector_sz_primary;
+    return bootutil_max_image_size(state, NULL);
 }
 
 /* Compute the total size of the given image. Includes the size of the TLVs. */

boot/bootutil/src/swap_scratch.c

@@ -907,7 +907,7 @@ int app_max_size(struct boot_loader_state *state)
 
     fap = BOOT_IMG_AREA(state, active_slot);
     assert(fap != NULL);
-    primary_sz = flash_area_get_size(fap);
+    primary_sz = bootutil_max_image_size(state, fap);
 
     if (active_slot == BOOT_PRIMARY_SLOT) {
         active_slot = BOOT_SECONDARY_SLOT;
@@ -917,7 +917,7 @@ int app_max_size(struct boot_loader_state *state)
 
     fap = BOOT_IMG_AREA(state, active_slot);
     assert(fap != NULL);
-    secondary_sz = flash_area_get_size(fap);
+    secondary_sz = bootutil_max_image_size(state, fap);
 
     return (secondary_sz < primary_sz ? secondary_sz : primary_sz);
 }