diff --git a/components/kdrv/apps/include/kdrv_slot.h b/components/kdrv/apps/include/kdrv_slot.h index cad1617..8ca20df 100644 --- a/components/kdrv/apps/include/kdrv_slot.h +++ b/components/kdrv/apps/include/kdrv_slot.h @@ -65,7 +65,16 @@ kode_status_t kdrv_slot_begin(void); /** @brief Append to the open session. */ kode_status_t kdrv_slot_write(const void *data, size_t len); -/** @brief Close the session and verify what was written. */ +/** + * @brief Close the session and verify what was written. + * + * An image is taken only when it is sound and its description names the + * project that is running: the slot is this firmware's own update, and an + * app's image there would boot in its place. + * + * @return KODE_OK; KODE_DENIED with no session open; KODE_ERROR for an image + * that fails its checks, has no description or is another project's. + */ kode_status_t kdrv_slot_end(void); /** @brief Abandon the session, leaving the slot invalid. */ diff --git a/components/kdrv/apps/src/kdrv_slot.c b/components/kdrv/apps/src/kdrv_slot.c index 3c5e10f..b3559d8 100644 --- a/components/kdrv/apps/src/kdrv_slot.c +++ b/components/kdrv/apps/src/kdrv_slot.c @@ -181,6 +181,41 @@ kode_status_t kdrv_slot_write(const void *data, size_t len) return KODE_OK; } +/** + * @brief Tell whether the image in the slot is the project that is running. + * + * The slot is where this firmware's own update goes, and the next boot hands + * the board to whatever it holds. esp_ota_end() accepts any image built for + * the chip, an app's included, so the project's name is what tells an update + * from something else. + * + * @return KODE_OK; KODE_ERROR when the slot's description cannot be read or + * names another project. + */ +static kode_status_t slot_is_ours(void) +{ + const esp_partition_t * const p = slot_part(); + const esp_app_desc_t * const run = esp_app_get_description(); + esp_app_desc_t desc; + + if ( (NULL == p) || (NULL == run) + || (esp_ota_get_partition_description(p, &desc) != ESP_OK) ) + { + KLOG_E(TAG, "the image written has no description to read; " + "it is not an update"); + return KODE_ERROR; + } + if ( strncmp(desc.project_name, run->project_name, + sizeof(desc.project_name)) != 0 ) + { + KLOG_E(TAG, "the image written is '%.*s', not %s; it is not an update", + (int)sizeof(desc.project_name), desc.project_name, + run->project_name); + return KODE_ERROR; + } + return KODE_OK; +} + kode_status_t kdrv_slot_end(void) { if ( 0 == s_ota ) @@ -194,7 +229,7 @@ kode_status_t kdrv_slot_end(void) KLOG_E(TAG, "esp_ota_end: %s", esp_err_to_name(err)); return KODE_ERROR; } - return KODE_OK; + return slot_is_ours(); } void kdrv_slot_abort(void) @@ -251,7 +286,7 @@ static kode_status_t slot_sink_write(void *ctx, const void *data, size_t len) * @brief Close the slot's OTA session and verify what was written. * @param ctx Unused. * @return KODE_OK; KODE_DENIED with no session open; KODE_ERROR when - * esp_ota_end() refuses the image. + * kdrv_slot_end() refuses the image. */ static kode_status_t slot_sink_end(void *ctx) { diff --git a/tests/host_slot/cases/test_kdrv_slot.c b/tests/host_slot/cases/test_kdrv_slot.c index 2d25afb..c0421e2 100644 --- a/tests/host_slot/cases/test_kdrv_slot.c +++ b/tests/host_slot/cases/test_kdrv_slot.c @@ -733,6 +733,65 @@ static void expect_desc(esp_app_desc_t *desc, const char *project) esp_ota_get_partition_description_ReturnThruPtr_app_desc(desc); } +/* The slot is where this firmware's own update goes, and the next boot hands + * the board to whatever it holds. What esp_ota_end() accepts is any image + * built for the chip, an app's included, so the project's name is what tells + * an update from something else. */ +TEST_CASE("an image of the running project ends well", "[kdrv_slot]") +{ + esp_app_desc_t desc; + kdrv_slot_desc_t running; + + slot_fresh(ESP_PARTITION_SUBTYPE_APP_OTA_0); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_running_desc(&running)); + + expect_begin_ok(); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_slot_begin()); + + esp_ota_end_ExpectAnyArgsAndReturn(ESP_OK); + expect_desc(&desc, running.project); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_slot_end()); + + slot_done(); +} + +TEST_CASE("an image of another project is refused at the end", "[kdrv_slot]") +{ + esp_app_desc_t desc; + + slot_fresh(ESP_PARTITION_SUBTYPE_APP_OTA_0); + + expect_begin_ok(); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_slot_begin()); + + esp_ota_end_ExpectAnyArgsAndReturn(ESP_OK); + expect_desc(&desc, "buttons_led"); + TEST_ASSERT_EQUAL_INT_MESSAGE(KODE_ERROR, kdrv_slot_end(), + "an app's image in the slot would boot in the OS's place"); + + /* The session is over all the same, and a new one opens. */ + TEST_ASSERT_EQUAL_INT(KODE_DENIED, kdrv_slot_end()); + expect_begin_ok(); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_slot_begin()); + + slot_done(); +} + +TEST_CASE("an image with no description to read is refused at the end", + "[kdrv_slot]") +{ + slot_fresh(ESP_PARTITION_SUBTYPE_APP_OTA_0); + + expect_begin_ok(); + TEST_ASSERT_EQUAL_INT(KODE_OK, kdrv_slot_begin()); + + esp_ota_end_ExpectAnyArgsAndReturn(ESP_OK); + esp_ota_get_partition_description_ExpectAnyArgsAndReturn(ESP_ERR_NOT_FOUND); + TEST_ASSERT_EQUAL_INT(KODE_ERROR, kdrv_slot_end()); + + slot_done(); +} + TEST_CASE("every app partition can be described by id, and only the running " "one says so", "[kdrv_slot]") { diff --git a/tests/host_slot/main/CMakeLists.txt b/tests/host_slot/main/CMakeLists.txt index 11cbf71..21c6f82 100644 --- a/tests/host_slot/main/CMakeLists.txt +++ b/tests/host_slot/main/CMakeLists.txt @@ -11,6 +11,7 @@ file(GLOB khal_includes LIST_DIRECTORIES true "${SDK_DIR}/components/khal/*/incl idf_component_register( SRCS "runner.c" "log_stub.c" + "app_desc_stub.c" ${CASE_SOURCES} "${SDK_DIR}/components/kdrv/apps/src/kdrv_slot.c" INCLUDE_DIRS "." diff --git a/tests/host_slot/main/app_desc_stub.c b/tests/host_slot/main/app_desc_stub.c new file mode 100644 index 0000000..96d24f0 --- /dev/null +++ b/tests/host_slot/main/app_desc_stub.c @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-3.0-or-later */ +/** + * @file app_desc_stub.c + * @brief The running image's description, for a binary the linux target + * links none into. + */ +#include "esp_app_desc.h" + +/** @brief What the running image says it is: the project an update must name. */ +static const esp_app_desc_t s_desc = { + .project_name = "kodeos", + .version = "0.0.0", +}; + +const esp_app_desc_t *esp_app_get_description(void) +{ + return &s_desc; +}