From f970edf01962e117444dd05892fc7648bcae8265 Mon Sep 17 00:00:00 2001 From: Greeva Patel Date: Sat, 31 Oct 2026 16:04:49 +0530 Subject: [PATCH] fix(fw_update): refuse to erase the active slot in eos_fw_update_begin eos_fw_update_begin() validated only that the target was Slot A or B and then erased the whole slot. It never consulted the boot control block, so a caller (or a transport handed the wrong slot) could wipe the slot the device was running from before a byte of the new image arrived. The "must not be the active slot" contract in eos_fw_update.h was enforced only later, in eos_fw_request_upgrade(), after the erase had happened. begin() now loads the boot control block and returns EOS_ERR_INVALID, without touching flash, when the target is the active slot. When neither copy is valid eos_bootctl_load() falls back to the factory defaults (Slot A active), which is what the bootloader would boot, so that slot is protected as well; any other load failure refuses the update. test_fw_update: begin() on the active slot is refused and the slot's contents survive; with no valid boot control block Slot A is refused; the inactive slot is still accepted. Co-Authored-By: Claude Opus 5.5 --- core/fw_update.c | 12 +++++++++ tests/unit/test_fw_update.c | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/core/fw_update.c b/core/fw_update.c index 1df5558..16e06ad 100644 --- a/core/fw_update.c +++ b/core/fw_update.c @@ -12,6 +12,7 @@ #include "eos_fwsvc.h" #include "eos_rollback.h" #include "eos_image_tlv.h" +#include "eos_bootctl.h" #include int eos_fw_update_begin(eos_fw_update_ctx_t *ctx, eos_slot_t slot) @@ -19,6 +20,17 @@ int eos_fw_update_begin(eos_fw_update_ctx_t *ctx, eos_slot_t slot) if (!ctx) return EOS_ERR_INVALID; if (slot != EOS_SLOT_A && slot != EOS_SLOT_B) return EOS_ERR_INVALID; + /* Never erase the slot the device is running from. The boot control + * block names it; when neither copy is valid eos_bootctl_load() falls + * back to the factory defaults (Slot A active), which is the slot the + * bootloader would boot, so that slot is protected too. Any other load + * failure means the active slot is unknown, and erasing either slot + * could destroy the running image, so the update is refused. */ + eos_bootctl_t bctl; + int load_rc = eos_bootctl_load(&bctl); + if (load_rc != EOS_OK && load_rc != EOS_ERR_CRC) return load_rc; + if ((eos_slot_t)bctl.active_slot == slot) return EOS_ERR_INVALID; + memset(ctx, 0, sizeof(*ctx)); ctx->target_slot = slot; ctx->target_addr = eos_hal_slot_addr(slot); diff --git a/tests/unit/test_fw_update.c b/tests/unit/test_fw_update.c index 470f0f8..0723410 100644 --- a/tests/unit/test_fw_update.c +++ b/tests/unit/test_fw_update.c @@ -389,6 +389,54 @@ TEST(test_finalize_accepts_tlv_counter_equal_to_floor) ASSERT(eos_fw_update_finalize(&ctx, EOS_UPGRADE_TEST) == EOS_OK); } +/* ---- Active-slot protection ---- + * + * begin() erases the whole target slot before a byte of the new image + * arrives. Pointing it at the slot the device is running from destroyed + * the running firmware; it must refuse and leave that slot untouched. + */ + +static void persist_bootctl_active(eos_slot_t active) +{ + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + bctl.active_slot = active; + ASSERT(eos_bootctl_save(&bctl) == EOS_OK); +} + +TEST(test_begin_refuses_active_slot_and_leaves_it_intact) +{ + eos_fw_update_ctx_t ctx; + + persist_bootctl_active(EOS_SLOT_B); + memset(&sim_flash[SIM_SLOT_B_ADDR], 0x5A, SIM_SLOT_B_SIZE); + + ASSERT(eos_fw_update_begin(&ctx, EOS_SLOT_B) == EOS_ERR_INVALID); + ASSERT(sim_flash[SIM_SLOT_B_ADDR] == 0x5A); + ASSERT(sim_flash[SIM_SLOT_B_ADDR + SIM_SLOT_B_SIZE - 1] == 0x5A); +} + +TEST(test_begin_accepts_inactive_slot) +{ + eos_fw_update_ctx_t ctx; + + persist_bootctl_active(EOS_SLOT_A); + ASSERT(eos_fw_update_begin(&ctx, EOS_SLOT_B) == EOS_OK); + ASSERT(eos_fw_update_get_state(&ctx) == EOS_FW_STATE_HEADER); +} + +TEST(test_begin_refuses_default_active_slot_without_bootctl) +{ + eos_fw_update_ctx_t ctx; + + /* No valid boot control block: the bootloader boots its defaults, + * Slot A, so Slot A is the running image and must not be erased. */ + memset(&sim_flash[SIM_SLOT_A_ADDR], 0x5A, SIM_SLOT_A_SIZE); + + ASSERT(eos_fw_update_begin(&ctx, EOS_SLOT_A) == EOS_ERR_INVALID); + ASSERT(sim_flash[SIM_SLOT_A_ADDR] == 0x5A); +} + /* ---- Direct reads: eos_rollback_read_image_counter ---- * * The finalize tests above exercise the counter reader indirectly, @@ -469,6 +517,9 @@ int main(void) run_test_bytes_wanted_tracks_the_remaining_container(); run_test_trailing_byte_is_rejected_the_same_across_chunk_boundaries(); run_test_finalize_accepts_tlv_counter_equal_to_floor(); + run_test_begin_refuses_active_slot_and_leaves_it_intact(); + run_test_begin_accepts_inactive_slot(); + run_test_begin_refuses_default_active_slot_without_bootctl(); run_test_read_image_counter_returns_tlv_counter(); run_test_read_image_counter_no_tlv_reports_zero(); run_test_read_image_counter_rejects_null_out();