From efce1e781fff170cf27c27abe12681a0f7c5cf7a Mon Sep 17 00:00:00 2001 From: Jeroen Wesbeek Date: Sun, 22 Mar 2026 15:21:52 +0100 Subject: [PATCH 1/2] fix: restore OTA CRC integrity check and add hard reset after BLE DFU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs caused BLE OTA to silently succeed while the application never booted: 1. dfu_init_postvalidate() computed and validated the image CRC but discarded it without writing it back to the caller. m_image_crc remained 0 after every OTA, so bank_0_crc = 0 was persisted to bootloader settings. bootloader_app_is_valid() skips the CRC check when bank_0_crc is 0, meaning any image — including a corrupted one — was unconditionally accepted at boot. Fix: add uint16_t *p_crc_out to dfu_init_postvalidate() and write the validated CRC through it. Update the call site in dfu_single_bank.c to pass &m_image_crc so the value is captured and stored in bootloader_settings_t.bank_0_crc. 2. After BLE OTA activation, check_dfu_mode() tore down the SoftDevice and USB and then returned to main(), which jumped directly to the application without issuing NVIC_SystemReset(). This left nRF52840 peripheral registers and radio state in a post-DFU condition. BLE applications (e.g. MeshCore on RAK4631) depend on a clean hardware reset to initialise their radio, SoftDevice, and peripheral stack. The direct jump caused silent initialisation failure and the device never came online. Additionally, sd_softdevice_vector_table_base_set() fails when the SD is already disabled, falling back to writing the forwarding address to 0x20000000, which the application's .bss initialisation can overwrite before the SD is re-enabled. Fix: add NVIC_SystemReset() after BLE OTA teardown. GPREGRET is already cleared to 0 earlier in check_dfu_mode(), so the subsequent boot goes straight to the application with a fully reset hardware state. The serial/USB DFU path is unaffected. --- .../components/libraries/bootloader_dfu/dfu_init.h | 13 +++++++------ .../libraries/bootloader_dfu/dfu_single_bank.c | 2 +- src/dfu_init.c | 7 ++++--- src/main.c | 3 ++- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h b/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h index 8b4807a9..cee408cf 100644 --- a/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h +++ b/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h @@ -116,18 +116,19 @@ uint32_t dfu_init_prevalidate(uint8_t * p_init_data, uint32_t init_data_len, uin * - A signature to ensure the image originates from a trusted source. * Checks are intended to be expanded for customer-specific requirements. * - * @param[in] p_image Pointer to the received image. The init data provided in the call - * \ref dfu_init_prevalidate will be used for validating the image. - * @param[in] image_len Length of the image data. + * @param[in] p_image Pointer to the received image. The init data provided in the call + * \ref dfu_init_prevalidate will be used for validating the image. + * @param[in] image_len Length of the image data. + * @param[out] p_crc_out Pointer to store the computed CRC of the image. Set on success. * * @retval NRF_SUCCESS If the post-validation succeeded, that meant the integrity of the - * image has been verified and the image originates from a trusted + * image has been verified and the image originates from a trusted * source (signing). - * @retval NRF_ERROR_INVALID_DATA If the post-validation failed, that meant the post check of the + * @retval NRF_ERROR_INVALID_DATA If the post-validation failed, that meant the post check of the * image failed such as the CRC is not matching the image transfered * or the verification of the image fails (signing). */ -uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len); +uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len, uint16_t * p_crc_out); #endif // DFU_INIT_H__ diff --git a/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c b/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c index f8c90ef8..72aec6b0 100644 --- a/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c +++ b/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c @@ -497,7 +497,7 @@ uint32_t dfu_image_validate() { m_dfu_state = DFU_STATE_VALIDATE; - err_code = dfu_init_postvalidate((uint8_t *)mp_storage_handle_active->block_id, m_image_size); + err_code = dfu_init_postvalidate((uint8_t *)mp_storage_handle_active->block_id, m_image_size, &m_image_crc); VERIFY_SUCCESS(err_code); m_dfu_state = DFU_STATE_WAIT_4_ACTIVATE; } diff --git a/src/dfu_init.c b/src/dfu_init.c index e19cffa3..adffe9bf 100644 --- a/src/dfu_init.c +++ b/src/dfu_init.c @@ -188,11 +188,11 @@ uint32_t dfu_init_prevalidate(uint8_t * p_init_data, uint32_t init_data_len, uin } -uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) +uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len, uint16_t * p_crc_out) { uint16_t image_crc; uint16_t received_crc; - + // In order to support hashing (and signing) then the (decrypted) hash should be fetched and // the corresponding hash should be calculated over the image at this location. // If hashing (or signing) is added to the system then the CRC validation should be removed. @@ -200,7 +200,7 @@ uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) // calculate CRC from active block. image_crc = crc16_compute(p_image, image_len, NULL); - // Decode the received CRC from extended data. + // Decode the received CRC from extended data. received_crc = uint16_decode((uint8_t *)&m_extended_packet[0]); // Compare the received and calculated CRC. @@ -209,6 +209,7 @@ uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) return NRF_ERROR_INVALID_DATA; } + *p_crc_out = image_crc; return NRF_SUCCESS; } diff --git a/src/main.c b/src/main.c index bd997137..05f73985 100644 --- a/src/main.c +++ b/src/main.c @@ -317,7 +317,8 @@ static void check_dfu_mode(void) { if (_ota_dfu) { sd_softdevice_disable(); - usb_teardown(); // allow booting to app after ota even if usb is connected + usb_teardown(); + NVIC_SystemReset(); // clean reset after BLE OTA; GPREGRET is already 0, boots straight to app } else { usb_teardown(); } From 1256ae2e37aef276b06be15f5f4f4409a0f7e489 Mon Sep 17 00:00:00 2001 From: Jeroen Wesbeek Date: Sun, 22 Mar 2026 15:21:52 +0100 Subject: [PATCH 2/2] fix: restore OTA CRC integrity check and add hard reset after BLE DFU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs caused BLE OTA to silently succeed while the application never booted: 1. dfu_init_postvalidate() computed and validated the image CRC but discarded it without writing it back to the caller. m_image_crc remained 0 after every OTA, so bank_0_crc = 0 was persisted to bootloader settings. bootloader_app_is_valid() skips the CRC check when bank_0_crc is 0, meaning any image — including a corrupted one — was unconditionally accepted at boot. Fix: add uint16_t *p_crc_out to dfu_init_postvalidate() and write the validated CRC through it. Update the call site in dfu_single_bank.c to pass &m_image_crc so the value is captured and stored in bootloader_settings_t.bank_0_crc. 2. After BLE OTA activation, check_dfu_mode() tore down the SoftDevice and USB and then returned to main(), which jumped directly to the application without issuing NVIC_SystemReset(). This left nRF52840 peripheral registers and radio state in a post-DFU condition. BLE applications (e.g. MeshCore on RAK4631) depend on a clean hardware reset to initialise their radio, SoftDevice, and peripheral stack. The direct jump caused silent initialisation failure and the device never came online. Additionally, sd_softdevice_vector_table_base_set() fails when the SD is already disabled, falling back to writing the forwarding address to 0x20000000, which the application's .bss initialisation can overwrite before the SD is re-enabled. Fix: add NVIC_SystemReset() after BLE OTA teardown. GPREGRET is already cleared to 0 earlier in check_dfu_mode(), so the subsequent boot goes straight to the application with a fully reset hardware state. The serial/USB DFU path is unaffected. --- .../components/libraries/bootloader_dfu/dfu_init.h | 13 +++++++------ .../libraries/bootloader_dfu/dfu_single_bank.c | 2 +- src/dfu_init.c | 7 ++++--- src/main.c | 3 ++- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h b/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h index 8b4807a9..cee408cf 100644 --- a/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h +++ b/lib/sdk11/components/libraries/bootloader_dfu/dfu_init.h @@ -116,18 +116,19 @@ uint32_t dfu_init_prevalidate(uint8_t * p_init_data, uint32_t init_data_len, uin * - A signature to ensure the image originates from a trusted source. * Checks are intended to be expanded for customer-specific requirements. * - * @param[in] p_image Pointer to the received image. The init data provided in the call - * \ref dfu_init_prevalidate will be used for validating the image. - * @param[in] image_len Length of the image data. + * @param[in] p_image Pointer to the received image. The init data provided in the call + * \ref dfu_init_prevalidate will be used for validating the image. + * @param[in] image_len Length of the image data. + * @param[out] p_crc_out Pointer to store the computed CRC of the image. Set on success. * * @retval NRF_SUCCESS If the post-validation succeeded, that meant the integrity of the - * image has been verified and the image originates from a trusted + * image has been verified and the image originates from a trusted * source (signing). - * @retval NRF_ERROR_INVALID_DATA If the post-validation failed, that meant the post check of the + * @retval NRF_ERROR_INVALID_DATA If the post-validation failed, that meant the post check of the * image failed such as the CRC is not matching the image transfered * or the verification of the image fails (signing). */ -uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len); +uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len, uint16_t * p_crc_out); #endif // DFU_INIT_H__ diff --git a/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c b/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c index f8c90ef8..72aec6b0 100644 --- a/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c +++ b/lib/sdk11/components/libraries/bootloader_dfu/dfu_single_bank.c @@ -497,7 +497,7 @@ uint32_t dfu_image_validate() { m_dfu_state = DFU_STATE_VALIDATE; - err_code = dfu_init_postvalidate((uint8_t *)mp_storage_handle_active->block_id, m_image_size); + err_code = dfu_init_postvalidate((uint8_t *)mp_storage_handle_active->block_id, m_image_size, &m_image_crc); VERIFY_SUCCESS(err_code); m_dfu_state = DFU_STATE_WAIT_4_ACTIVATE; } diff --git a/src/dfu_init.c b/src/dfu_init.c index e19cffa3..adffe9bf 100644 --- a/src/dfu_init.c +++ b/src/dfu_init.c @@ -188,11 +188,11 @@ uint32_t dfu_init_prevalidate(uint8_t * p_init_data, uint32_t init_data_len, uin } -uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) +uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len, uint16_t * p_crc_out) { uint16_t image_crc; uint16_t received_crc; - + // In order to support hashing (and signing) then the (decrypted) hash should be fetched and // the corresponding hash should be calculated over the image at this location. // If hashing (or signing) is added to the system then the CRC validation should be removed. @@ -200,7 +200,7 @@ uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) // calculate CRC from active block. image_crc = crc16_compute(p_image, image_len, NULL); - // Decode the received CRC from extended data. + // Decode the received CRC from extended data. received_crc = uint16_decode((uint8_t *)&m_extended_packet[0]); // Compare the received and calculated CRC. @@ -209,6 +209,7 @@ uint32_t dfu_init_postvalidate(uint8_t * p_image, uint32_t image_len) return NRF_ERROR_INVALID_DATA; } + *p_crc_out = image_crc; return NRF_SUCCESS; } diff --git a/src/main.c b/src/main.c index bd997137..05f73985 100644 --- a/src/main.c +++ b/src/main.c @@ -317,7 +317,8 @@ static void check_dfu_mode(void) { if (_ota_dfu) { sd_softdevice_disable(); - usb_teardown(); // allow booting to app after ota even if usb is connected + usb_teardown(); + NVIC_SystemReset(); // clean reset after BLE OTA; GPREGRET is already 0, boots straight to app } else { usb_teardown(); }