Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 8d58f7e1e -> af9ffab13


Revert "boot - don't interpet end of image as trailer."

This reverts commit 80e7fa0dbd0a3b56f1941cd763297ff67bfab8c5.

The boot loader records its current state in the form of a pair of image
trailers, each located at the end of the corresponding image slot.  If
an image is so big that it extends into the trailer space of a slot, the
boot loader would read the end of the image and interpet it as the start
of a trailer.  The fix was to determine the size of each image upfront
by reading their headers, and only attempt to read an image's trailer if
the image is small enough that it doesn't extend into the trailer space.
If an image is too big to allow for a trailer, the boot loader fails
over to its "rescue mode": just boot into whatever is in slot 0.

The problem arises when the boot loader reads the image headers.  There
are certain points during a swap when an image header is not in the
expected location.  That is, if the device reboots at the wrong time
during an image swap, the boot loader will fail to read the image
headers when it comes up.

The image sectors are swapped in reverse order.  When a swap is
performed, the final sectors of each slot are swapped first, and the
first sectors (containing the image headers) get swapped last.  During
the final swap operation, there are two points at which the image
headers are not in the expected place:

    1. slot 1 erased; header 1 in scratch area.
    2. slot 0 erased; header 0 in scratch area.

In each case, the image header is not actually missing.  Rather, the
boot loader is just looking in the wrong place.  It should be looking in
the scratch area, not the start of the image slot.

The fix is to revert the original commit.  Now, the boot loader won't
fail when an image header read fails.  It is the user's responsibility
to ensure an image isn't too big.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/af9ffab1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/af9ffab1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/af9ffab1

Branch: refs/heads/develop
Commit: af9ffab13b2081f0c9e007a73912c6a729c9759e
Parents: 8d58f7e
Author: Christopher Collins <[email protected]>
Authored: Tue Jan 10 15:47:18 2017 -0800
Committer: Christopher Collins <[email protected]>
Committed: Tue Jan 10 15:47:18 2017 -0800

----------------------------------------------------------------------
 boot/bootutil/src/loader.c                      | 28 ---------
 boot/bootutil/test/src/boot_test.c              |  2 -
 boot/bootutil/test/src/boot_test.h              |  1 -
 boot/bootutil/test/src/boot_test_utils.c        | 26 +--------
 .../test/src/testcases/boot_test_img_too_big.c  | 60 --------------------
 5 files changed, 1 insertion(+), 116 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/af9ffab1/boot/bootutil/src/loader.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/src/loader.c b/boot/bootutil/src/loader.c
index c143eb6..fc20fdb 100644
--- a/boot/bootutil/src/loader.c
+++ b/boot/bootutil/src/loader.c
@@ -286,51 +286,23 @@ boot_write_sz(void)
     return elem_sz;
 }
 
-static uint32_t
-boot_total_img_size(int slot)
-{
-    const struct image_header *hdr;
-
-    hdr = &boot_data.imgs[slot].hdr;
-    return hdr->ih_hdr_size + hdr->ih_img_size + hdr->ih_tlv_size;
-}
-
 static int
 boot_slots_compatible(void)
 {
     const struct flash_area *sector0;
     const struct flash_area *sector1;
-    uint32_t slot_size;
     int i;
 
     /* Ensure both image slots have identical sector layouts. */
     if (boot_data.imgs[0].num_sectors != boot_data.imgs[1].num_sectors) {
         return 0;
     }
-
-    slot_size = 0;
     for (i = 0; i < boot_data.imgs[0].num_sectors; i++) {
         sector0 = boot_data.imgs[0].sectors + i;
         sector1 = boot_data.imgs[1].sectors + i;
         if (sector0->fa_size != sector1->fa_size) {
             return 0;
         }
-
-        slot_size += sector0->fa_size;
-    }
-
-    /* Ensure images are small enough that they leave room for the image
-     * trailers.
-     */
-    if (slot_size <
-        boot_total_img_size(0) + boot_trailer_sz(boot_data.write_sz)) {
-
-        return 0;
-    }
-    if (slot_size <
-        boot_total_img_size(1) + boot_trailer_sz(boot_data.write_sz)) {
-
-        return 0;
     }
 
     return 1;

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/af9ffab1/boot/bootutil/test/src/boot_test.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/test/src/boot_test.c 
b/boot/bootutil/test/src/boot_test.c
index 478bf85..98972f3 100644
--- a/boot/bootutil/test/src/boot_test.c
+++ b/boot/bootutil/test/src/boot_test.c
@@ -53,7 +53,6 @@ TEST_CASE_DECL(boot_test_revert)
 TEST_CASE_DECL(boot_test_revert_continue)
 TEST_CASE_DECL(boot_test_permanent)
 TEST_CASE_DECL(boot_test_permanent_continue)
-TEST_CASE_DECL(boot_test_img_too_big);
 
 TEST_SUITE(boot_test_main)
 {
@@ -76,7 +75,6 @@ TEST_SUITE(boot_test_main)
     boot_test_revert_continue();
     boot_test_permanent();
     boot_test_permanent_continue();
-    boot_test_img_too_big();
 }
 
 int

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/af9ffab1/boot/bootutil/test/src/boot_test.h
----------------------------------------------------------------------
diff --git a/boot/bootutil/test/src/boot_test.h 
b/boot/bootutil/test/src/boot_test.h
index cd15e0d..d0c1319 100644
--- a/boot/bootutil/test/src/boot_test.h
+++ b/boot/bootutil/test/src/boot_test.h
@@ -60,7 +60,6 @@ extern struct boot_test_img_addrs boot_test_img_addrs[];
 
 uint8_t boot_test_util_byte_at(int img_msb, uint32_t image_offset);
 uint8_t boot_test_util_flash_align(void);
-uint32_t boot_test_util_slot_size(int slot);
 void boot_test_util_init_flash(void);
 void boot_test_util_copy_area(int from_area_idx, int to_area_idx);
 void boot_test_util_swap_areas(int area_idx1, int area_idx2);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/af9ffab1/boot/bootutil/test/src/boot_test_utils.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/test/src/boot_test_utils.c 
b/boot/bootutil/test/src/boot_test_utils.c
index c8583f0..30297c8 100644
--- a/boot/bootutil/test/src/boot_test_utils.c
+++ b/boot/bootutil/test/src/boot_test_utils.c
@@ -60,36 +60,12 @@ uint8_t
 boot_test_util_flash_align(void)
 {
     const struct flash_area *fap;
-    uint8_t align;
     int rc;
 
     rc = flash_area_open(FLASH_AREA_IMAGE_0, &fap);
     TEST_ASSERT_FATAL(rc == 0);
 
-    align = flash_area_align(fap);
-
-    flash_area_close(fap);
-
-    return align;
-}
-
-uint32_t
-boot_test_util_slot_size(int slot)
-{
-    const struct flash_area *fap;
-    uint32_t size;
-    int area_id;
-    int rc;
-
-    area_id = flash_area_id_from_image_slot(slot);
-    rc = flash_area_open(area_id, &fap);
-    TEST_ASSERT_FATAL(rc == 0);
-
-    size = fap->fa_size;
-
-    flash_area_close(fap);
-
-    return size;
+    return flash_area_align(fap);
 }
 
 void

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/af9ffab1/boot/bootutil/test/src/testcases/boot_test_img_too_big.c
----------------------------------------------------------------------
diff --git a/boot/bootutil/test/src/testcases/boot_test_img_too_big.c 
b/boot/bootutil/test/src/testcases/boot_test_img_too_big.c
deleted file mode 100644
index c72607b..0000000
--- a/boot/bootutil/test/src/testcases/boot_test_img_too_big.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *  http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-#include "boot_test.h"
-
-TEST_CASE(boot_test_img_too_big)
-{
-    uint32_t slot_size;
-
-    struct image_header hdr0 = {
-        .ih_magic = IMAGE_MAGIC,
-        .ih_tlv_size = 4 + 32,
-        .ih_hdr_size = BOOT_TEST_HEADER_SIZE,
-        .ih_img_size = 5 * 1024,
-        .ih_flags = IMAGE_F_SHA256,
-        .ih_ver = { 0, 5, 21, 432 },
-    };
-
-    struct image_header hdr1 = {
-        .ih_magic = IMAGE_MAGIC,
-        .ih_tlv_size = 4 + 32,
-        .ih_hdr_size = BOOT_TEST_HEADER_SIZE,
-        .ih_img_size = 0,
-        .ih_flags = IMAGE_F_SHA256,
-        .ih_ver = { 1, 2, 3, 432 },
-    };
-
-    boot_test_util_init_flash();
-
-    /* Make image in slot 1 so large that there is no room for an image
-     * trailer.
-     */
-    slot_size = boot_test_util_slot_size(1);
-    hdr1.ih_img_size = slot_size - 10;
-
-    boot_test_util_write_image(&hdr0, 0);
-    boot_test_util_write_hash(&hdr0, 0);
-    boot_test_util_write_image(&hdr1, 1);
-    boot_test_util_write_hash(&hdr1, 1);
-
-    /* Ensure the boot loader does not interpret the end of the image as a
-     * trailer.
-     */
-    boot_test_util_verify_all(BOOT_SWAP_TYPE_NONE, &hdr0, &hdr1);
-}

Reply via email to