From 2173ad3b4543553383edd795e7fa04416d682ebc Mon Sep 17 00:00:00 2001 From: Tian Zhong Xing Date: Tue, 24 Jan 2017 18:30:13 +0800 Subject: [PATCH 1/7] bootloader_support: fix bug OTA & flash encryption incompatible ota data partition should be encrypted unconditionally when flash encrypt enable --- components/bootloader_support/src/flash_encrypt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/bootloader_support/src/flash_encrypt.c b/components/bootloader_support/src/flash_encrypt.c index 8b9ca60cd1..8ba068d03b 100644 --- a/components/bootloader_support/src/flash_encrypt.c +++ b/components/bootloader_support/src/flash_encrypt.c @@ -285,6 +285,9 @@ static esp_err_t encrypt_partition(int index, const esp_partition_info_t *partit } else { should_encrypt = false; } + } else if (partition->type == PART_TYPE_DATA && partition->subtype == PART_SUBTYPE_DATA_OTA) { + /* check if we have ota data partition and the partition should be encrypted unconditionally */ + should_encrypt = true; } if (!should_encrypt) { From 813395adcb76819b18de13569699997abd3490f4 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 16:19:45 +1100 Subject: [PATCH 2/7] OTA: Fall back to factory partition if ota data partition is invalid --- components/app_update/esp_ota_ops.c | 15 ++++++++------- components/bootloader/src/main/bootloader_start.c | 11 +++++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c index 439117b596..8b1a43163e 100644 --- a/components/app_update/esp_ota_ops.c +++ b/components/app_update/esp_ota_ops.c @@ -349,33 +349,34 @@ const esp_partition_t *esp_ota_get_boot_partition(void) } ota_app_count = get_ota_partition_count(); - ESP_LOGD(TAG, "found ota bin max = %d", ota_app_count); + ESP_LOGD(TAG, "found ota app max = %d", ota_app_count); + if (s_ota_select[0].ota_seq == 0xFFFFFFFF && s_ota_select[1].ota_seq == 0xFFFFFFFF) { - ESP_LOGD(TAG, "finding factory bin......"); + ESP_LOGD(TAG, "finding factory app......"); return esp_partition_find_first(ESP_PARTITION_TYPE_APP, ESP_PARTITION_SUBTYPE_APP_FACTORY, NULL); } else if (ota_select_valid(&s_ota_select[0]) && ota_select_valid(&s_ota_select[1])) { - ESP_LOGD(TAG, "finding ota_%d bin......", \ + ESP_LOGD(TAG, "finding ota_%d app......", \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + ((OTA_MAX(s_ota_select[0].ota_seq, s_ota_select[1].ota_seq) - 1) % ota_app_count)); return esp_partition_find_first(ESP_PARTITION_TYPE_APP, \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + ((OTA_MAX(s_ota_select[0].ota_seq, s_ota_select[1].ota_seq) - 1) % ota_app_count), NULL); } else if (ota_select_valid(&s_ota_select[0])) { - ESP_LOGD(TAG, "finding ota_%d bin......", \ + ESP_LOGD(TAG, "finding ota_%d app......", \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + (s_ota_select[0].ota_seq - 1) % ota_app_count); return esp_partition_find_first(ESP_PARTITION_TYPE_APP, \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + (s_ota_select[0].ota_seq - 1) % ota_app_count, NULL); } else if (ota_select_valid(&s_ota_select[1])) { - ESP_LOGD(TAG, "finding ota_%d bin......", \ + ESP_LOGD(TAG, "finding ota_%d app......", \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + (s_ota_select[1].ota_seq - 1) % ota_app_count); return esp_partition_find_first(ESP_PARTITION_TYPE_APP, \ ESP_PARTITION_SUBTYPE_APP_OTA_MIN + (s_ota_select[1].ota_seq - 1) % ota_app_count, NULL); } else { - ESP_LOGE(TAG, "not found current bin"); - return NULL; + ESP_LOGE(TAG, "ota data invalid, no current app. Falling back to factory"); + return esp_partition_find_first(ESP_PARTITION_TYPE_APP, ESP_PARTITION_SUBTYPE_APP_FACTORY, NULL); } } diff --git a/components/bootloader/src/main/bootloader_start.c b/components/bootloader/src/main/bootloader_start.c index a374c4306d..43066aa3d7 100644 --- a/components/bootloader/src/main/bootloader_start.c +++ b/components/bootloader/src/main/bootloader_start.c @@ -323,12 +323,15 @@ void bootloader_main() } else { if(ota_select_valid(&sa) && ota_select_valid(&sb)) { load_part_pos = bs.ota[(((sa.ota_seq > sb.ota_seq)?sa.ota_seq:sb.ota_seq) - 1)%bs.app_count]; - }else if(ota_select_valid(&sa)) { + } else if(ota_select_valid(&sa)) { load_part_pos = bs.ota[(sa.ota_seq - 1) % bs.app_count]; - }else if(ota_select_valid(&sb)) { + } else if(ota_select_valid(&sb)) { load_part_pos = bs.ota[(sb.ota_seq - 1) % bs.app_count]; - }else { - ESP_LOGE(TAG, "ota data partition info error"); + } else if (bs.factory.offset != 0) { + ESP_LOGE(TAG, "ota data partition invalid, falling back to factory"); + load_part_pos = bs.factory; + } else { + ESP_LOGE(TAG, "ota data partition invalid and no factory, can't boot"); return; } } From 67336672bddb761b7f448b10c44f930655f6686e Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 16:18:58 +1100 Subject: [PATCH 3/7] OTA: Improve verification of OTA image before writing, incl. secure boot Verify 0xE9 magic byte on first write, verify entire image before switching. Enable verification for secure boot signature (was using invalid ifdef guard) --- components/app_update/esp_ota_ops.c | 25 +++++++++++++++++-------- examples/system/ota/main/ota.c | 19 ++++++------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c index 8b1a43163e..23b6f870c6 100644 --- a/components/app_update/esp_ota_ops.c +++ b/components/app_update/esp_ota_ops.c @@ -106,6 +106,7 @@ esp_err_t esp_ota_begin(const esp_partition_t *partition, size_t image_size, esp esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) { + const uint8_t *data_bytes = (const uint8_t *)data; esp_err_t ret; ota_ops_entry_t *it; @@ -119,6 +120,12 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) if (it->handle == handle) { // must erase the partition before writing to it assert(it->erased_size > 0 && "must erase the partition before writing to it"); + + if(it->wrote_size == 0 && size > 0 && data_bytes[0] != 0xE9) { + ESP_LOGE(TAG, "OTA image has invalid magic byte (expected 0xE9, saw 0x%02x", data_bytes[0]); + return ESP_ERR_OTA_VALIDATE_FAILED; + } + ret = esp_partition_write(&it->part, it->wrote_size, data, size); if(ret == ESP_OK){ it->wrote_size += size; @@ -135,6 +142,7 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) esp_err_t esp_ota_end(esp_ota_handle_t handle) { ota_ops_entry_t *it; + size_t image_size; for (it = LIST_FIRST(&s_ota_ops_entries_head); it != NULL; it = LIST_NEXT(it, entries)) { if (it->handle == handle) { // an ota handle need to be ended after erased and wrote data in it @@ -142,12 +150,12 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) return ESP_ERR_INVALID_ARG; } -#ifdef CONFIG_SECUREBOOTLOADER - esp_err_t ret; - size_t image_size; - if (esp_image_basic_verify(it->part.address, &image_size) != ESP_OK) { + if (esp_image_basic_verify(it->part.address, true, &image_size) != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } + +#ifdef CONFIG_SECURE_BOOT_ENABLED + esp_err_t ret; ret = esp_secure_boot_verify_signature(it->part.address, image_size); if (ret != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; @@ -285,17 +293,18 @@ static esp_err_t esp_rewrite_ota_data(esp_partition_subtype_t subtype) esp_err_t esp_ota_set_boot_partition(const esp_partition_t *partition) { + size_t image_size; const esp_partition_t *find_partition = NULL; if (partition == NULL) { return ESP_ERR_INVALID_ARG; } -#ifdef CONFIG_SECUREBOOTLOADER - size_t image_size; - if (esp_image_basic_verify(partition->address, &image_size) != ESP_OK) { + if (esp_image_basic_verify(partition->address, true, &image_size) != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } - ret = esp_secure_boot_verify_signature(partition->address, image_size); + +#ifdef CONFIG_SECURE_BOOT_ENABLED + esp_err_t ret = esp_secure_boot_verify_signature(partition->address, image_size); if (ret != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } diff --git a/examples/system/ota/main/ota.c b/examples/system/ota/main/ota.c index 4956129e5d..21ebcc3c21 100644 --- a/examples/system/ota/main/ota.c +++ b/examples/system/ota/main/ota.c @@ -95,7 +95,7 @@ static void initialise_wifi(void) } /*read buffer by byte still delim ,return read bytes counts*/ -int read_until(char *buffer, char delim, int len) +static int read_until(char *buffer, char delim, int len) { // /*TODO: delim check,buffer check,further: do an buffer length limited*/ int i = 0; @@ -109,7 +109,7 @@ int read_until(char *buffer, char delim, int len) * return true if packet including \r\n\r\n that means http packet header finished,start to receive packet body * otherwise return false * */ -bool resolve_pkg(char text[], int total_len, esp_ota_handle_t out_handle) +static bool read_past_http_header(char text[], int total_len, esp_ota_handle_t out_handle) { /* i means current position */ int i = 0, i_read_len = 0; @@ -121,13 +121,6 @@ bool resolve_pkg(char text[], int total_len, esp_ota_handle_t out_handle) memset(ota_write_data, 0, BUFFSIZE); /*copy first http packet body to write buffer*/ memcpy(ota_write_data, &(text[i + 2]), i_write_len); - /*check write packet header first byte:0xE9 second byte:0x09 */ - if (ota_write_data[0] == 0xE9 && i_write_len >= 2 && ota_write_data[1] == 0x09) { - ESP_LOGI(TAG, "OTA Write Header format Check OK. first byte is %02x ,second byte is %02x", ota_write_data[0], ota_write_data[1]); - } else { - ESP_LOGE(TAG, "OTA Write Header format Check Failed! first byte is %02x ,second byte is %02x", ota_write_data[0], ota_write_data[1]); - return false; - } esp_err_t err = esp_ota_write( out_handle, (const void *)ota_write_data, i_write_len); if (err != ESP_OK) { @@ -266,7 +259,7 @@ void main_task(void *pvParameter) task_fatal_error(); } - bool pkg_body_start = false, flag = true; + bool resp_body_start = false, flag = true; /*deal with all receive packet*/ while (flag) { memset(text, 0, TEXT_BUFFSIZE); @@ -275,10 +268,10 @@ void main_task(void *pvParameter) if (buff_len < 0) { /*receive error*/ ESP_LOGE(TAG, "Error: receive data error! errno=%d", errno); task_fatal_error(); - } else if (buff_len > 0 && !pkg_body_start) { /*deal with packet header*/ + } else if (buff_len > 0 && !resp_body_start) { /*deal with response header*/ memcpy(ota_write_data, text, buff_len); - pkg_body_start = resolve_pkg(text, buff_len, out_handle); - } else if (buff_len > 0 && pkg_body_start) { /*deal with packet body*/ + resp_body_start = read_past_http_header(text, buff_len, out_handle); + } else if (buff_len > 0 && resp_body_start) { /*deal with response body*/ memcpy(ota_write_data, text, buff_len); err = esp_ota_write( out_handle, (const void *)ota_write_data, buff_len); if (err != ESP_OK) { From d8aae55eebc8c47703593d8c9222dd339d7f5b82 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 18:29:18 +1100 Subject: [PATCH 4/7] Flash encryption: Temporary fix for issue with stale cache reads Seems doing certain kinds of short reads while flash encryption is enabled will return stale data. This fixes it, but is probably a little heavy-handed performance wise. --- .../include/esp_flash_encrypt.h | 15 +++++-- components/spi_flash/flash_mmap.c | 45 ++++++++++++++----- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/components/bootloader_support/include/esp_flash_encrypt.h b/components/bootloader_support/include/esp_flash_encrypt.h index 015dea030a..ff1a5f3306 100644 --- a/components/bootloader_support/include/esp_flash_encrypt.h +++ b/components/bootloader_support/include/esp_flash_encrypt.h @@ -15,7 +15,8 @@ #define __ESP32_FLASH_ENCRYPT_H #include -#include +#include "esp_attr.h" +#include "esp_err.h" #include "esp_spi_flash.h" #include "soc/efuse_reg.h" @@ -30,9 +31,17 @@ * * @return true if flash encryption is enabled. */ -static inline bool esp_flash_encryption_enabled(void) { +static inline IRAM_ATTR bool esp_flash_encryption_enabled(void) { uint32_t flash_crypt_cnt = REG_GET_FIELD(EFUSE_BLK0_RDATA0_REG, EFUSE_RD_FLASH_CRYPT_CNT); - return __builtin_parity(flash_crypt_cnt) == 1; + /* __builtin_parity is in flash, so we calculate parity inline */ + bool enabled = false; + while(flash_crypt_cnt) { + if (flash_crypt_cnt & 1) { + enabled = !enabled; + } + flash_crypt_cnt >>= 1; + } + return enabled; } /* @brief Update on-device flash encryption diff --git a/components/spi_flash/flash_mmap.c b/components/spi_flash/flash_mmap.c index 837fe0cc37..f8d2e3297d 100644 --- a/components/spi_flash/flash_mmap.c +++ b/components/spi_flash/flash_mmap.c @@ -28,6 +28,7 @@ #include "esp_ipc.h" #include "esp_attr.h" #include "esp_spi_flash.h" +#include "esp_flash_encrypt.h" #include "esp_log.h" #include "cache_utils.h" @@ -52,8 +53,10 @@ This ensures stale cache entries are never read after fresh calls to spi_flash_mmap(), while keeping the number of cache flushes to a minimum. + + Returns true if cache was flushed. */ -static void spi_flash_ensure_unmodified_region(size_t start_addr, size_t length); +static bool spi_flash_ensure_unmodified_region(size_t start_addr, size_t length); typedef struct mmap_entry_{ uint32_t handle; @@ -89,6 +92,7 @@ esp_err_t IRAM_ATTR spi_flash_mmap(size_t src_addr, size_t size, spi_flash_mmap_ const void** out_ptr, spi_flash_mmap_handle_t* out_handle) { esp_err_t ret; + bool did_flush, need_flush = false; mmap_entry_t* new_entry = (mmap_entry_t*) malloc(sizeof(mmap_entry_t)); if (new_entry == 0) { return ESP_ERR_NO_MEM; @@ -102,7 +106,7 @@ esp_err_t IRAM_ATTR spi_flash_mmap(size_t src_addr, size_t size, spi_flash_mmap_ spi_flash_disable_interrupts_caches_and_other_cpu(); - spi_flash_ensure_unmodified_region(src_addr, size); + did_flush = spi_flash_ensure_unmodified_region(src_addr, size); if (s_mmap_page_refcnt[0] == 0) { spi_flash_mmap_init(); @@ -159,8 +163,11 @@ esp_err_t IRAM_ATTR spi_flash_mmap(size_t src_addr, size_t size, spi_flash_mmap_ (DPORT_PRO_FLASH_MMU_TABLE[i] == entry_val && DPORT_APP_FLASH_MMU_TABLE[i] == entry_val)); if (s_mmap_page_refcnt[i] == 0) { - DPORT_PRO_FLASH_MMU_TABLE[i] = entry_val; - DPORT_APP_FLASH_MMU_TABLE[i] = entry_val; + if (DPORT_PRO_FLASH_MMU_TABLE[i] != entry_val || DPORT_APP_FLASH_MMU_TABLE[i] != entry_val) { + DPORT_PRO_FLASH_MMU_TABLE[i] = entry_val; + DPORT_APP_FLASH_MMU_TABLE[i] = entry_val; + need_flush = true; + } } ++s_mmap_page_refcnt[i]; } @@ -173,6 +180,18 @@ esp_err_t IRAM_ATTR spi_flash_mmap(size_t src_addr, size_t size, spi_flash_mmap_ *out_ptr = (void*) (region_addr + start * SPI_FLASH_MMU_PAGE_SIZE); ret = ESP_OK; } + + /* This is a temporary fix for an issue where some + encrypted cache reads may see stale data. + + Working on a long term fix that doesn't require invalidating + entire cache. + */ + if (esp_flash_encryption_enabled() && !did_flush && need_flush) { + Cache_Flush(0); + Cache_Flush(1); + } + spi_flash_enable_interrupts_caches_and_other_cpu(); if (*out_ptr == NULL) { free(new_entry); @@ -240,25 +259,29 @@ void spi_flash_mmap_dump() */ static uint32_t written_pages[256/32]; -static void update_written_pages(size_t start_addr, size_t length, bool mark); +static bool update_written_pages(size_t start_addr, size_t length, bool mark); void IRAM_ATTR spi_flash_mark_modified_region(size_t start_addr, size_t length) { update_written_pages(start_addr, length, true); } -static void IRAM_ATTR spi_flash_ensure_unmodified_region(size_t start_addr, size_t length) +static IRAM_ATTR bool spi_flash_ensure_unmodified_region(size_t start_addr, size_t length) { - update_written_pages(start_addr, length, false); + return update_written_pages(start_addr, length, false); } /* generic implementation for the previous two functions */ -static inline IRAM_ATTR void update_written_pages(size_t start_addr, size_t length, bool mark) +static inline IRAM_ATTR bool update_written_pages(size_t start_addr, size_t length, bool mark) { - for (uint32_t addr = start_addr; addr < start_addr + length; addr += SPI_FLASH_MMU_PAGE_SIZE) { + /* align start_addr & length to full MMU pages */ + uint32_t page_start_addr = start_addr & ~(SPI_FLASH_MMU_PAGE_SIZE-1); + length += (start_addr - page_start_addr); + length = (length + SPI_FLASH_MMU_PAGE_SIZE - 1) & ~(SPI_FLASH_MMU_PAGE_SIZE-1); + for (uint32_t addr = page_start_addr; addr < page_start_addr + length; addr += SPI_FLASH_MMU_PAGE_SIZE) { int page = addr / SPI_FLASH_MMU_PAGE_SIZE; if (page >= 256) { - return; /* invalid address */ + return false; /* invalid address */ } int idx = page / 32; @@ -277,6 +300,8 @@ static inline IRAM_ATTR void update_written_pages(size_t start_addr, size_t leng Cache_Flush(1); #endif bzero(written_pages, sizeof(written_pages)); + return true; } } + return false; } From eea2788f5a9ea3eb8717145734f6ad6991fc66d3 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 18:30:32 +1100 Subject: [PATCH 5/7] OTA: Fix issues with encrypted OTA - OTA source can write non-16-byte multiples of data - Assumption that empty ota_data is 0xFFFFFFFF untrue when encrypted --- components/app_update/esp_ota_ops.c | 63 +++++++++++++++++-- .../bootloader_support/src/bootloader_flash.c | 7 ++- components/spi_flash/partition.c | 8 ++- examples/system/ota/main/ota.c | 4 +- 4 files changed, 71 insertions(+), 11 deletions(-) diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c index 23b6f870c6..2338753898 100644 --- a/components/app_update/esp_ota_ops.c +++ b/components/app_update/esp_ota_ops.c @@ -27,6 +27,7 @@ #include "esp_spi_flash.h" #include "esp_image_format.h" #include "esp_secure_boot.h" +#include "esp_flash_encrypt.h" #include "sdkconfig.h" #include "esp_ota_ops.h" @@ -44,6 +45,10 @@ typedef struct ota_ops_entry_ { esp_partition_t part; uint32_t erased_size; uint32_t wrote_size; +#ifdef CONFIG_FLASH_ENCRYPTION_ENABLED + uint8_t partial_bytes; + uint8_t partial_data[16]; +#endif LIST_ENTRY(ota_ops_entry_) entries; } ota_ops_entry_t; @@ -126,7 +131,41 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void *data, size_t size) return ESP_ERR_OTA_VALIDATE_FAILED; } - ret = esp_partition_write(&it->part, it->wrote_size, data, size); +#ifdef CONFIG_FLASH_ENCRYPTION_ENABLED + if (esp_flash_encryption_enabled()) { + /* Can only write 16 byte blocks to flash, so need to cache anything else */ + size_t copy_len; + + /* check if we have partially written data from earlier */ + if (it->partial_bytes != 0) { + copy_len = OTA_MIN(16 - it->partial_bytes, size); + memcpy(it->partial_data + it->partial_bytes, data_bytes, copy_len); + it->partial_bytes += copy_len; + if (it->partial_bytes != 16) { + return ESP_OK; /* nothing to write yet, just filling buffer */ + } + /* write 16 byte to partition */ + ret = esp_partition_write(&it->part, it->wrote_size, it->partial_data, 16); + if (ret != ESP_OK) { + return ret; + } + it->partial_bytes = 0; + memset(it->partial_data, 0xFF, 16); + it->wrote_size += 16; + data_bytes += copy_len; + size -= copy_len; + } + + /* check if we need to save trailing data that we're about to write */ + it->partial_bytes = size % 16; + if (it->partial_bytes != 0) { + size -= it->partial_bytes; + memcpy(it->partial_data, data_bytes + size, it->partial_bytes); + } + } +#endif + + ret = esp_partition_write(&it->part, it->wrote_size, data_bytes, size); if(ret == ESP_OK){ it->wrote_size += size; } @@ -143,6 +182,8 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) { ota_ops_entry_t *it; size_t image_size; + esp_err_t __attribute__((unused)) ret; + for (it = LIST_FIRST(&s_ota_ops_entries_head); it != NULL; it = LIST_NEXT(it, entries)) { if (it->handle == handle) { // an ota handle need to be ended after erased and wrote data in it @@ -150,6 +191,18 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) return ESP_ERR_INVALID_ARG; } +#ifdef CONFIG_FLASH_ENCRYPTION_ENABLED + if (it->partial_bytes > 0 && esp_flash_encryption_enabled()) { + /* Write out last 16 bytes, if necessary */ + ret = esp_partition_write(&it->part, it->wrote_size, it->partial_data, 16); + if (ret != ESP_OK) { + return ret; + } + it->wrote_size += 16; + it->partial_bytes = 0; + } +#endif + if (esp_image_basic_verify(it->part.address, true, &image_size) != ESP_OK) { return ESP_ERR_OTA_VALIDATE_FAILED; } @@ -279,11 +332,9 @@ static esp_err_t esp_rewrite_ota_data(esp_partition_subtype_t subtype) } return rewrite_ota_seq((SUB_TYPE_ID(subtype) + 1) % ota_app_count + i * ota_app_count, 0, find_partition); - } else if (s_ota_select[0].ota_seq == 0xFFFFFFFF && s_ota_select[1].ota_seq == 0xFFFFFFFF) { - return rewrite_ota_seq(SUB_TYPE_ID(subtype) + 1, 0, find_partition); - } else { - return ESP_ERR_OTA_SELECT_INFO_INVALID; + /* Both OTA slots are invalid, probably because unformatted... */ + return rewrite_ota_seq(SUB_TYPE_ID(subtype) + 1, 0, find_partition); } } else { @@ -385,7 +436,7 @@ const esp_partition_t *esp_ota_get_boot_partition(void) ESP_PARTITION_SUBTYPE_APP_OTA_MIN + (s_ota_select[1].ota_seq - 1) % ota_app_count, NULL); } else { - ESP_LOGE(TAG, "ota data invalid, no current app. Falling back to factory"); + ESP_LOGE(TAG, "ota data invalid, no current app. Assuming factory"); return esp_partition_find_first(ESP_PARTITION_TYPE_APP, ESP_PARTITION_SUBTYPE_APP_FACTORY, NULL); } } diff --git a/components/bootloader_support/src/bootloader_flash.c b/components/bootloader_support/src/bootloader_flash.c index 405da732b3..da77b1dc28 100644 --- a/components/bootloader_support/src/bootloader_flash.c +++ b/components/bootloader_support/src/bootloader_flash.c @@ -16,6 +16,7 @@ #include #include #include /* including in bootloader for error values */ +#include #ifndef BOOTLOADER_BUILD /* Normal app version maps to esp_spi_flash.h operations... @@ -48,7 +49,11 @@ void bootloader_munmap(const void *mapping) esp_err_t bootloader_flash_read(size_t src, void *dest, size_t size, bool allow_decrypt) { - return spi_flash_read(src, dest, size); + if (allow_decrypt && esp_flash_encryption_enabled()) { + return spi_flash_read_encrypted(src, dest, size); + } else { + return spi_flash_read(src, dest, size); + } } esp_err_t bootloader_flash_write(size_t dest_addr, void *src, size_t size, bool write_encrypted) diff --git a/components/spi_flash/partition.c b/components/spi_flash/partition.c index 45f66d696b..76036b305e 100644 --- a/components/spi_flash/partition.c +++ b/components/spi_flash/partition.c @@ -166,10 +166,14 @@ static esp_err_t load_partitions() item->info.type = it->type; item->info.subtype = it->subtype; item->info.encrypted = it->flags & PART_FLAG_ENCRYPTED; - if (esp_flash_encryption_enabled() && it->type == PART_TYPE_APP) { - /* All app partitions are encrypted if encryption is turned on */ + if (esp_flash_encryption_enabled() && ( + it->type == PART_TYPE_APP + || (it->type == PART_TYPE_DATA && it->subtype == PART_SUBTYPE_DATA_OTA))) { + /* If encryption is turned on, all app partitions and OTA data + are always encrypted */ item->info.encrypted = true; } + // it->label may not be zero-terminated strncpy(item->info.label, (const char*) it->label, sizeof(it->label)); item->info.label[sizeof(it->label)] = 0; diff --git a/examples/system/ota/main/ota.c b/examples/system/ota/main/ota.c index 21ebcc3c21..2dfda1c828 100644 --- a/examples/system/ota/main/ota.c +++ b/examples/system/ota/main/ota.c @@ -124,7 +124,7 @@ static bool read_past_http_header(char text[], int total_len, esp_ota_handle_t o esp_err_t err = esp_ota_write( out_handle, (const void *)ota_write_data, i_write_len); if (err != ESP_OK) { - ESP_LOGE(TAG, "Error: esp_ota_write failed! err=%x", err); + ESP_LOGE(TAG, "Error: esp_ota_write failed! err=0x%x", err); return false; } else { ESP_LOGI(TAG, "esp_ota_write header OK"); @@ -275,7 +275,7 @@ void main_task(void *pvParameter) memcpy(ota_write_data, text, buff_len); err = esp_ota_write( out_handle, (const void *)ota_write_data, buff_len); if (err != ESP_OK) { - ESP_LOGE(TAG, "Error: esp_ota_write failed! err=%x", err); + ESP_LOGE(TAG, "Error: esp_ota_write failed! err=0x%x", err); task_fatal_error(); } binary_file_length += buff_len; From b6a2329f0f6efa8c254a7ff980df7538dc0a3723 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 26 Jan 2017 18:31:15 +1100 Subject: [PATCH 6/7] FreeRTOS: esp_crosscore_int_send_yield() should be in IRAM Possible for xQueueGenericSendFromISR -> xTaskRemoveFromEventQueueList -> taskYIELD_OTHER_CORE code path to occur while cache is off. --- components/esp32/crosscore_int.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/esp32/crosscore_int.c b/components/esp32/crosscore_int.c index 1e131eeef7..f75f0eba7d 100644 --- a/components/esp32/crosscore_int.c +++ b/components/esp32/crosscore_int.c @@ -82,7 +82,7 @@ void esp_crosscore_int_init() { assert(err == ESP_OK); } -void esp_crosscore_int_send_yield(int coreId) { +void IRAM_ATTR esp_crosscore_int_send_yield(int coreId) { assert(coreId Date: Fri, 3 Feb 2017 10:07:30 +1100 Subject: [PATCH 7/7] OTA: Always clean up OTA handle regardless of esp_ota_end() result As reported on forum: http://esp32.com/viewtopic.php?f=14&t=1093 --- components/app_update/esp_ota_ops.c | 70 +++++++++++---------- components/app_update/include/esp_ota_ops.h | 13 ++-- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/components/app_update/esp_ota_ops.c b/components/app_update/esp_ota_ops.c index 2338753898..c702a203e2 100644 --- a/components/app_update/esp_ota_ops.c +++ b/components/app_update/esp_ota_ops.c @@ -182,40 +182,10 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) { ota_ops_entry_t *it; size_t image_size; - esp_err_t __attribute__((unused)) ret; + esp_err_t ret = ESP_OK; for (it = LIST_FIRST(&s_ota_ops_entries_head); it != NULL; it = LIST_NEXT(it, entries)) { if (it->handle == handle) { - // an ota handle need to be ended after erased and wrote data in it - if ((it->erased_size == 0) || (it->wrote_size == 0)) { - return ESP_ERR_INVALID_ARG; - } - -#ifdef CONFIG_FLASH_ENCRYPTION_ENABLED - if (it->partial_bytes > 0 && esp_flash_encryption_enabled()) { - /* Write out last 16 bytes, if necessary */ - ret = esp_partition_write(&it->part, it->wrote_size, it->partial_data, 16); - if (ret != ESP_OK) { - return ret; - } - it->wrote_size += 16; - it->partial_bytes = 0; - } -#endif - - if (esp_image_basic_verify(it->part.address, true, &image_size) != ESP_OK) { - return ESP_ERR_OTA_VALIDATE_FAILED; - } - -#ifdef CONFIG_SECURE_BOOT_ENABLED - esp_err_t ret; - ret = esp_secure_boot_verify_signature(it->part.address, image_size); - if (ret != ESP_OK) { - return ESP_ERR_OTA_VALIDATE_FAILED; - } -#endif - - LIST_REMOVE(it, entries); break; } } @@ -224,8 +194,44 @@ esp_err_t esp_ota_end(esp_ota_handle_t handle) return ESP_ERR_NOT_FOUND; } + /* 'it' holds the ota_ops_entry_t for 'handle' */ + + // esp_ota_end() is only valid if some data was written to this handle + if ((it->erased_size == 0) || (it->wrote_size == 0)) { + ret = ESP_ERR_INVALID_ARG; + goto cleanup; + } + +#ifdef CONFIG_FLASH_ENCRYPTION_ENABLED + if (it->partial_bytes > 0 && esp_flash_encryption_enabled()) { + /* Write out last 16 bytes, if necessary */ + ret = esp_partition_write(&it->part, it->wrote_size, it->partial_data, 16); + if (ret != ESP_OK) { + ret = ESP_ERR_INVALID_STATE; + goto cleanup; + } + it->wrote_size += 16; + it->partial_bytes = 0; + } +#endif + + if (esp_image_basic_verify(it->part.address, true, &image_size) != ESP_OK) { + ret = ESP_ERR_OTA_VALIDATE_FAILED; + goto cleanup; + } + +#ifdef CONFIG_SECURE_BOOT_ENABLED + ret = esp_secure_boot_verify_signature(it->part.address, image_size); + if (ret != ESP_OK) { + ret = ESP_ERR_OTA_VALIDATE_FAILED; + goto cleanup; + } +#endif + + cleanup: + LIST_REMOVE(it, entries); free(it); - return ESP_OK; + return ret; } static uint32_t ota_select_crc(const ota_select *s) diff --git a/components/app_update/include/esp_ota_ops.h b/components/app_update/include/esp_ota_ops.h index 846aa2b2cf..fe3307763f 100755 --- a/components/app_update/include/esp_ota_ops.h +++ b/components/app_update/include/esp_ota_ops.h @@ -73,11 +73,16 @@ esp_err_t esp_ota_write(esp_ota_handle_t handle, const void* data, size_t size); /** * @brief Finish the update and validate written data * - * @param handle Handle obtained from esp_ota_begin + * @param handle Handle obtained from esp_ota_begin. * - * @return: - * - ESP_OK: if validate ota image pass - * - ESP_ERR_OTA_VALIDATE_FAILED: validate the ota image is invalid + * @note After calling esp_ota_end(), the handle is no longer valid and any memory associated with it is freed (regardless of result). + * + * @return: + * - ESP_OK: Newly written OTA app image is valid. + * - ESP_ERR_NOT_FOUND: OTA handle was not found. + * - ESP_ERR_INVALID_ARG: Handle was never written to. + * - ESP_ERR_OTA_VALIDATE_FAILED: OTA image is invalid (either not a valid app image, or - if secure boot is enabled - signature failed to verify.) + * - ESP_ERR_INVALID_STATE: If flash encryption is enabled, this result indicates an internal error writing the final encrypted bytes to flash. */ esp_err_t esp_ota_end(esp_ota_handle_t handle);