From 6d66dc2d7e04bc456d0a83b2851d48f74f142666 Mon Sep 17 00:00:00 2001 From: Matthew Little Date: Sun, 12 Apr 2026 11:53:47 -0600 Subject: [PATCH 1/4] fix(esp-bootloader-esp-idf): fix OTA partition selection producing ota_seq == 0 `Ota::set_current_app_partition` produces `ota_seq = 0` when selecting certain partitions from uninitialized otadata (the state espflash leaves after a direct flash). The esp-idf bootloader ignores `ota_seq == 0`, so the device boots the old firmware instead of the newly flashed image. The root cause is the Factory-case increment formula: ((app_number + 1) + count) % count For Ota1 with 2 partitions this gives `((1+1)+2) % 2 = 0`. With both otadata slots uninitialized, `new_seq = inc = 0`. Additionally, `get_slot_seq` treats `ota_seq == 0` as a valid sequence number, causing `current_app_partition` to return a wrong result via unsigned underflow (`(0u32 - 1) % 2 = 1`). This makes the broken state sticky: subsequent calls to `set_current_app_partition` see the target as already selected and become no-ops. Fix: - Replace the Factory-case formula with `app_number + 1`, matching the esp-idf C implementation (`SUB_TYPE_ID(subtype) + 1`). - Normalize `ota_seq == 0` to `UNINITALIZED_SEQUENCE` in `get_slot_seq` so it is handled the same as an erased slot. Refs: - esp-idf uninitialized case: https://github.com/espressif/esp-idf/blob/v5.4.1/components/app_update/esp_ota_ops.c#L430-L434 - esp-idf active case: https://github.com/espressif/esp-idf/blob/v5.4.1/components/app_update/esp_ota_ops.c#L418-L428 --- esp-bootloader-esp-idf/CHANGELOG.md | 1 + esp-bootloader-esp-idf/src/ota.rs | 77 +++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/esp-bootloader-esp-idf/CHANGELOG.md b/esp-bootloader-esp-idf/CHANGELOG.md index 94e5edb14f9..d4d16b08a90 100644 --- a/esp-bootloader-esp-idf/CHANGELOG.md +++ b/esp-bootloader-esp-idf/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fix `set_current_app_partition` producing `ota_seq == 0` when selecting from uninitialized otadata, which the bootloader ignores (#5346) ### Removed diff --git a/esp-bootloader-esp-idf/src/ota.rs b/esp-bootloader-esp-idf/src/ota.rs index 3adc181d36d..132a3ee174c 100644 --- a/esp-bootloader-esp-idf/src/ota.rs +++ b/esp-bootloader-esp-idf/src/ota.rs @@ -220,8 +220,23 @@ where let mut buffer2 = OtaSelectEntry::default(); self.flash.read(SLOT0_DATA_OFFSET, buffer1.as_bytes_mut())?; self.flash.read(SLOT1_DATA_OFFSET, buffer2.as_bytes_mut())?; - let seq0 = buffer1.ota_seq; - let seq1 = buffer2.ota_seq; + // The esp-idf C writer never produces ota_seq == 0 (see + // `esp_rewrite_ota_data` in esp-idf's `esp_ota_ops.c`), so if it + // appears on flash it was written by a buggy caller. Normalize it + // to UNINITALIZED_SEQUENCE so the rest of the selection logic does + // not misinterpret it via unsigned underflow in `(0u32 - 1)`. + // + // Ref: https://github.com/espressif/esp-idf/blob/v5.4.1/components/app_update/esp_ota_ops.c#L390-L435 + let seq0 = if buffer1.ota_seq == 0 { + UNINITALIZED_SEQUENCE + } else { + buffer1.ota_seq + }; + let seq1 = if buffer2.ota_seq == 0 { + UNINITALIZED_SEQUENCE + } else { + buffer2.ota_seq + }; Ok((seq0, seq1)) } @@ -264,8 +279,20 @@ where // calculate the needed increment of the sequence-number to select the requested OTA-app // partition let inc = if current == AppPartitionSubType::Factory { - (((app.ota_app_number()) as i32 + 1) + (self.ota_partition_count as i32)) as u32 - % self.ota_partition_count as u32 + // Both slots are uninitialized when current == Factory, so + // new_seq = inc directly (not base + inc). The bootloader + // selects partition (ota_seq - 1) % count, so the minimum + // valid ota_seq that selects `app` is app_number + 1. + // + // This matches the esp-idf C implementation which uses + // `SUB_TYPE_ID(subtype) + 1` for the uninitialized case: + // https://github.com/espressif/esp-idf/blob/v5.4.1/components/app_update/esp_ota_ops.c#L430-L434 + // + // Note: the previous formula `((app_number + 1) + count) % + // count` produces 0 when (app_number + 1) is a multiple of + // count (e.g. Ota1 with 2 partitions), and the bootloader + // ignores ota_seq == 0. + app.ota_app_number() as u32 + 1 } else { ((((app.ota_app_number()) as i32) - ((current.ota_app_number()) as i32)) + (self.ota_partition_count as i32)) as u32 @@ -464,6 +491,48 @@ mod tests { assert_eq!(SLOT_INITIAL, &mock_flash.data[0x1000..][..0x20],); } + /// Regression test: selecting Ota1 from uninitialized otadata (Factory + /// state) must produce ota_seq >= 1. Previously the increment formula + /// `((app_number + 1) + count) % count` gave 0 for Ota1 with 2 + /// partitions, and the bootloader treats ota_seq == 0 as unused. + #[test] + fn test_factory_to_ota1_produces_valid_seq() { + let mut binary = PARTITION_RAW; + + let mock_entry = PartitionEntry { + binary: &mut binary, + }; + + let mut mock_flash = MockFlash { + data: [0xff; 0x2000], + }; + + let mock_region = FlashRegion { + raw: mock_entry, + flash: &mut mock_flash, + }; + + let mut sut = Ota::new(mock_region, 2).unwrap(); + assert_eq!( + sut.current_app_partition().unwrap(), + AppPartitionSubType::Factory + ); + + // Select Ota1 directly from uninitialized (Factory) state. + sut.set_current_app_partition(AppPartitionSubType::Ota1) + .unwrap(); + assert_eq!( + sut.current_app_partition().unwrap(), + AppPartitionSubType::Ota1 + ); + + // The written ota_seq must be >= 1 (bootloader ignores seq == 0). + let seq = u32::from_le_bytes(mock_flash.data[0x0000..0x0004].try_into().unwrap()); + assert!(seq >= 1, "ota_seq must be >= 1, got {seq}"); + // (seq - 1) % 2 must equal 1 to select Ota1. + assert_eq!((seq - 1) % 2, 1, "ota_seq {seq} does not select Ota1"); + } + #[test] fn test_slot0_valid_next_slot() { let mut binary = PARTITION_RAW; From 5454b3885ecb19ddb153949ce0ebbe43727ab975 Mon Sep 17 00:00:00 2001 From: Matthew Little Date: Sun, 12 Apr 2026 12:58:04 -0600 Subject: [PATCH 2/4] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esp-bootloader-esp-idf/src/ota.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esp-bootloader-esp-idf/src/ota.rs b/esp-bootloader-esp-idf/src/ota.rs index 132a3ee174c..c7c43e0669e 100644 --- a/esp-bootloader-esp-idf/src/ota.rs +++ b/esp-bootloader-esp-idf/src/ota.rs @@ -527,7 +527,9 @@ mod tests { ); // The written ota_seq must be >= 1 (bootloader ignores seq == 0). - let seq = u32::from_le_bytes(mock_flash.data[0x0000..0x0004].try_into().unwrap()); + let slot0_offset = SLOT0_DATA_OFFSET as usize; + let seq = + u32::from_le_bytes(mock_flash.data[slot0_offset..slot0_offset + 4].try_into().unwrap()); assert!(seq >= 1, "ota_seq must be >= 1, got {seq}"); // (seq - 1) % 2 must equal 1 to select Ota1. assert_eq!((seq - 1) % 2, 1, "ota_seq {seq} does not select Ota1"); From 4d382b768919d1b6802a8d0fcf3589dc1f438071 Mon Sep 17 00:00:00 2001 From: Matthew Little Date: Sun, 12 Apr 2026 13:04:19 -0600 Subject: [PATCH 3/4] chore: formatting fix --- esp-bootloader-esp-idf/src/ota.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esp-bootloader-esp-idf/src/ota.rs b/esp-bootloader-esp-idf/src/ota.rs index c7c43e0669e..05c78be1773 100644 --- a/esp-bootloader-esp-idf/src/ota.rs +++ b/esp-bootloader-esp-idf/src/ota.rs @@ -528,8 +528,11 @@ mod tests { // The written ota_seq must be >= 1 (bootloader ignores seq == 0). let slot0_offset = SLOT0_DATA_OFFSET as usize; - let seq = - u32::from_le_bytes(mock_flash.data[slot0_offset..slot0_offset + 4].try_into().unwrap()); + let seq = u32::from_le_bytes( + mock_flash.data[slot0_offset..slot0_offset + 4] + .try_into() + .unwrap(), + ); assert!(seq >= 1, "ota_seq must be >= 1, got {seq}"); // (seq - 1) % 2 must equal 1 to select Ota1. assert_eq!((seq - 1) % 2, 1, "ota_seq {seq} does not select Ota1"); From 75f374158cade37807d47eab3ad25a4e250649f7 Mon Sep 17 00:00:00 2001 From: Matthew Little Date: Mon, 13 Apr 2026 18:11:35 -0600 Subject: [PATCH 4/4] fix merge conflict --- esp-bootloader-esp-idf/src/ota.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esp-bootloader-esp-idf/src/ota.rs b/esp-bootloader-esp-idf/src/ota.rs index e0c1d9bc81b..a7fbf7da3b1 100644 --- a/esp-bootloader-esp-idf/src/ota.rs +++ b/esp-bootloader-esp-idf/src/ota.rs @@ -223,17 +223,17 @@ where // The esp-idf C writer never produces ota_seq == 0 (see // `esp_rewrite_ota_data` in esp-idf's `esp_ota_ops.c`), so if it // appears on flash it was written by a buggy caller. Normalize it - // to UNINITALIZED_SEQUENCE so the rest of the selection logic does + // to UNINITIALIZED_SEQUENCE so the rest of the selection logic does // not misinterpret it via unsigned underflow in `(0u32 - 1)`. // // Ref: https://github.com/espressif/esp-idf/blob/v5.4.1/components/app_update/esp_ota_ops.c#L390-L435 let seq0 = if buffer1.ota_seq == 0 { - UNINITALIZED_SEQUENCE + UNINITIALIZED_SEQUENCE } else { buffer1.ota_seq }; let seq1 = if buffer2.ota_seq == 0 { - UNINITALIZED_SEQUENCE + UNINITIALIZED_SEQUENCE } else { buffer2.ota_seq };