diff --git a/components/bootloader_support/src/bootloader_common.c b/components/bootloader_support/src/bootloader_common.c index d3db882434..a87e301aea 100644 --- a/components/bootloader_support/src/bootloader_common.c +++ b/components/bootloader_support/src/bootloader_common.c @@ -293,7 +293,9 @@ esp_err_t bootloader_common_check_chip_validity(const esp_image_header_t* img_hd ESP_LOGE(TAG, "can't run on lower chip revision, expected %d, found %d", revision, img_hdr->min_chip_rev); err = ESP_FAIL; } else if (revision != img_hdr->min_chip_rev) { - ESP_LOGI(TAG, "min. %s chip revision: %d", type == ESP_IMAGE_BOOTLOADER ? "bootloader" : "application", img_hdr->min_chip_rev); +#ifdef BOOTLOADER_BUILD + ESP_LOGI(TAG, "chip revision: %d, min. %s chip revision: %d", revision, type == ESP_IMAGE_BOOTLOADER ? "bootloader" : "application", img_hdr->min_chip_rev); +#endif } return err; } diff --git a/components/bootloader_support/src/esp_image_format.c b/components/bootloader_support/src/esp_image_format.c index ea19323072..8a22e8209c 100644 --- a/components/bootloader_support/src/esp_image_format.c +++ b/components/bootloader_support/src/esp_image_format.c @@ -222,6 +222,7 @@ static esp_err_t image_load(esp_image_load_mode_t mode, const esp_partition_pos_ // No secure boot, but SHA-256 can be appended for basic corruption detection if (sha_handle != NULL && !esp_cpu_in_ocd_debug_mode()) { err = verify_simple_hash(sha_handle, data); + sha_handle = NULL; // calling verify_simple_hash finishes sha_handle } #endif // SECURE_BOOT_CHECK_SIGNATURE } else { // verify_sha @@ -229,18 +230,19 @@ static esp_err_t image_load(esp_image_load_mode_t mode, const esp_partition_pos_ if (sha_handle != NULL) { bootloader_sha256_finish(sha_handle, NULL); } - - if (data->image.hash_appended) { - const void *hash = bootloader_mmap(data->start_addr + data->image_len - HASH_LEN, HASH_LEN); - if (hash == NULL) { - err = ESP_FAIL; - goto err; - } - memcpy(data->image_digest, hash, HASH_LEN); - bootloader_munmap(hash); - } sha_handle = NULL; - } // verify_sha + } //verify_sha + + // Separately, if there's a hash appended to the image then copy it out to the data->image_digest field + if (data->image.hash_appended) { + const void *hash = bootloader_mmap(data->start_addr + data->image_len - HASH_LEN, HASH_LEN); + if (hash == NULL) { + err = ESP_FAIL; + goto err; + } + memcpy(data->image_digest, hash, HASH_LEN); + bootloader_munmap(hash); + } } // do_verify if (err != ESP_OK) { @@ -310,9 +312,6 @@ static esp_err_t verify_image_header(uint32_t src_addr, const esp_image_header_t } err = ESP_ERR_IMAGE_INVALID; } - if (bootloader_common_check_chip_validity(image, ESP_IMAGE_APPLICATION) != ESP_OK) { - err = ESP_ERR_IMAGE_INVALID; - } if (!silent) { if (image->spi_mode > ESP_IMAGE_SPI_MODE_SLOW_READ) { ESP_LOGW(TAG, "image at 0x%x has invalid SPI mode %d", src_addr, image->spi_mode); @@ -324,6 +323,16 @@ static esp_err_t verify_image_header(uint32_t src_addr, const esp_image_header_t ESP_LOGW(TAG, "image at 0x%x has invalid SPI size %d", src_addr, image->spi_size); } } + + if (err == ESP_OK) { + // Checking the chip revision header *will* print a bunch of other info + // regardless of silent setting as this may be important, but don't bother checking it + // if it looks like the app partition is erased or otherwise garbage + if (bootloader_common_check_chip_validity(image, ESP_IMAGE_APPLICATION) != ESP_OK) { + err = ESP_ERR_IMAGE_INVALID; + } + } + return err; } diff --git a/components/spi_flash/test/test_partitions.c b/components/spi_flash/test/test_partitions.c index 705f0bed75..cd46f723c8 100644 --- a/components/spi_flash/test/test_partitions.c +++ b/components/spi_flash/test/test_partitions.c @@ -22,6 +22,8 @@ #include #include +#include +#include #include #include @@ -66,3 +68,57 @@ TEST_CASE("Test erase partition", "[spi_flash][esp_flash]") } } } + +static bool s_test_nonzero_sha_of_partition(const esp_partition_t *part, bool allow_invalid_image) +{ + uint8_t sha256[32] = { 0 }; + + TEST_ASSERT_NOT_NULL(part); + + esp_err_t err = esp_partition_get_sha256(part, sha256); + + if (allow_invalid_image && err == ESP_ERR_IMAGE_INVALID) { + printf("App partition at 0x%x doesn't hold a valid app\n", part->address); + return false; + } + + // Otherwise, err should be ESP_OK + ESP_ERROR_CHECK(err); + + ESP_LOG_BUFFER_HEX("sha", sha256, sizeof(sha256)); + + for (int i = 0; i < sizeof(sha256); i++) { + if (sha256[i] != 0) { + return true; // At least one non-zero byte! + } + } + TEST_FAIL_MESSAGE("SHA-256 of data partition should not be all zeroes"); + abort(); // unreachable +} + +TEST_CASE("Test esp_partition_get_sha256() with data", "[spi_flash]") +{ + const esp_partition_t *part = get_test_data_partition(); + s_test_nonzero_sha_of_partition(part, false); +} + +TEST_CASE("Test esp_partition_get_sha256() with app", "[spi_flash]") +{ + bool found_valid_app = false; + esp_partition_iterator_t it = esp_partition_find(ESP_PARTITION_TYPE_APP, + ESP_PARTITION_SUBTYPE_ANY, + NULL); + TEST_ASSERT_NOT_NULL(it); /* has to be at least one app partition */ + + while (it != NULL) { + const esp_partition_t *part = esp_partition_get(it); + printf("Hashing app partition at 0x%x\n", part->address); + bool valid = s_test_nonzero_sha_of_partition(part, true); + found_valid_app |= valid; + + it = esp_partition_next(it); + } + + TEST_ASSERT_MESSAGE(found_valid_app, "At least one app partition should be a valid app partition"); +} + diff --git a/tools/ci/config/target-test.yml b/tools/ci/config/target-test.yml index 827e318587..3e169f97a3 100644 --- a/tools/ci/config/target-test.yml +++ b/tools/ci/config/target-test.yml @@ -453,7 +453,7 @@ UT_034: UT_035: extends: .unit_test_template - parallel: 30 + parallel: 31 tags: - ESP32S2BETA_IDF - UT_T1_1