Merge branch 'bugfix/bootloader_iram_overlap_check' into 'master'

bootloader: verify that loaded image does not overlap bootloader code

See merge request idf/esp-idf!3409

components/bootloader/subproject/main/esp32.bootloader.ld

@@ -35,7 +35,6 @@ SECTIONS
   .iram_loader.text :
   {
     . = ALIGN (16);
-    _stext = .;
     _loader_text_start = ABSOLUTE(.);
     *(.stub .gnu.warning .gnu.linkonce.literal.* .gnu.linkonce.t.*.literal .gnu.linkonce.t.*)
      *(.iram1 .iram1.*) /* catch stray IRAM_ATTR */
@@ -59,7 +58,6 @@ SECTIONS
     *(.fini)
     *(.gnu.version)
     _loader_text_end = ABSOLUTE(.);
-    _etext = .;
   } > iram_loader_seg
 
   .iram.text :

components/bootloader_support/include/bootloader_util.h

@@ -0,0 +1,34 @@
+// Copyright 2018 Espressif Systems (Shanghai) PTE LTD
+//
+// Licensed 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.
+
+#pragma once
+
+#include <stddef.h>
+
+/**
+ * @brief Check if half-open intervals overlap
+ *
+ * @param start1  interval 1 start
+ * @param end1    interval 1 end
+ * @param start2  interval 2 start
+ * @param end2    interval 2 end
+ * @return true iff [start1; end1) overlaps [start2; end2)
+ */
+static inline bool bootloader_util_regions_overlap(
+        const intptr_t start1, const intptr_t end1,
+        const intptr_t start2, const intptr_t end2)
+{
+    return (end1 > start2 && end2 > start1) ||
+           !(end1 <= start2 || end2 <= start1);
+}

components/bootloader_support/src/esp_image_format.c

@@ -23,6 +23,7 @@
 #include <bootloader_flash.h>
 #include <bootloader_random.h>
 #include <bootloader_sha.h>
+#include "bootloader_util.h"
 
 /* Checking signatures as part of verifying images is necessary:
    - Always if secure boot is enabled
@@ -56,6 +57,10 @@ static const char *TAG = "esp_image";
    (Means loaded code isn't executable until after the secure boot check.)
 */
 static uint32_t ram_obfs_value[2];
+
+/* Range of IRAM used by the loader, defined in ld script */
+extern int _loader_text_start;
+extern int _loader_text_end;
 #endif
 
 /* Return true if load_addr is an address the bootloader should load into */
@@ -328,18 +333,41 @@ static esp_err_t process_segment(int index, uint32_t flash_addr, esp_image_segme
                  (do_load)?"load":(is_mapping)?"map":"");
     }
 
+
+#ifdef BOOTLOADER_BUILD
+    /* Before loading segment, check it doesn't clobber bootloader RAM. */
     if (do_load) {
-        /* Before loading segment, check it doesn't clobber bootloader RAM... */
-        uint32_t end_addr = load_addr + data_len;
-        if (end_addr < 0x40000000) {
+        const intptr_t load_end = load_addr + data_len;
+        if (load_end <= (intptr_t) SOC_DIRAM_DRAM_HIGH) {
+            /* Writing to DRAM */
             intptr_t sp = (intptr_t)get_sp();
-            if (end_addr > sp - STACK_LOAD_HEADROOM) {
-                ESP_LOGE(TAG, "Segment %d end address 0x%08x too high (bootloader stack 0x%08x liimit 0x%08x)",
-                         index, end_addr, sp, sp - STACK_LOAD_HEADROOM);
+            if (load_end > sp - STACK_LOAD_HEADROOM) {
+                /* Bootloader .data/.rodata/.bss is above the stack, so this
+                 * also checks that we aren't overwriting these segments.
+                 *
+                 * TODO: This assumes specific arrangement of sections we have
+                 * in the ESP32. Rewrite this in a generic way to support other
+                 * layouts.
+                 */
+                ESP_LOGE(TAG, "Segment %d end address 0x%08x too high (bootloader stack 0x%08x limit 0x%08x)",
+                         index, load_end, sp, sp - STACK_LOAD_HEADROOM);
+                return ESP_ERR_IMAGE_INVALID;
+            }
+        } else {
+            /* Writing to IRAM */
+            const intptr_t loader_iram_start = (intptr_t) &_loader_text_start;
+            const intptr_t loader_iram_end = (intptr_t) &_loader_text_end;
+
+            if (bootloader_util_regions_overlap(loader_iram_start, loader_iram_end,
+                    load_addr, load_end)) {
+                ESP_LOGE(TAG, "Segment %d (0x%08x-0x%08x) overlaps bootloader IRAM (0x%08x-0x%08x)",
+                         index, load_addr, load_end, loader_iram_start, loader_iram_end);
                 return ESP_ERR_IMAGE_INVALID;
             }
         }
     }
+#endif // BOOTLOADER_BUILD
+
 #ifndef BOOTLOADER_BUILD
     uint32_t free_page_count = spi_flash_mmap_get_free_pages(SPI_FLASH_MMAP_DATA);
     ESP_LOGD(TAG, "free data page_count 0x%08x",free_page_count);

components/bootloader_support/test/test_verify_image.c

@@ -14,6 +14,7 @@
 #include "freertos/xtensa_api.h"
 #include "unity.h"
 #include "bootloader_common.h"
+#include "bootloader_util.h"
 #include "esp_partition.h"
 #include "esp_ota_ops.h"
 #include "esp_image_format.h"
@@ -92,3 +93,23 @@ TEST_CASE("Test label_search", "[bootloader_support]")
     check_label_search(25,  "phy, 1234567890123456, nvs1",  "12345678901234567",    true);
 
 }
+
+
+TEST_CASE("Test regions_overlap", "[bootloader_support]")
+{
+    TEST_ASSERT( bootloader_util_regions_overlap(1, 2, 1, 2) );
+
+    TEST_ASSERT( bootloader_util_regions_overlap(1, 2, 0, 2) );
+    TEST_ASSERT( bootloader_util_regions_overlap(1, 2, 1, 3) );
+    TEST_ASSERT( bootloader_util_regions_overlap(1, 2, 0, 3) );
+
+    TEST_ASSERT( bootloader_util_regions_overlap(0, 2, 1, 2) );
+    TEST_ASSERT( bootloader_util_regions_overlap(1, 3, 1, 2) );
+    TEST_ASSERT( bootloader_util_regions_overlap(0, 3, 1, 2) );
+
+    TEST_ASSERT( !bootloader_util_regions_overlap(2, 3, 1, 2) );
+    TEST_ASSERT( !bootloader_util_regions_overlap(1, 2, 2, 3) );
+
+    TEST_ASSERT( !bootloader_util_regions_overlap(3, 4, 1, 2) );
+    TEST_ASSERT( !bootloader_util_regions_overlap(1, 2, 3, 4) );
+}