When checking the secure_dfu code, I found the following two code fragments seemed problematic in nrf_bootloader_fw_activation.c
1. In the following one, if the ACTIVATION_ERROR is detected, it will still proceed to nrf_dfu_bank_invalidate() and invalidate bank 1. If ACTIVATION_ERROR was caused during copying bank 1 to bank 0, invalidating bank1 here will cause both applications in bank 1 and bank 0 become invalid.
nrf_bootloader_fw_activation_result_t nrf_bootloader_fw_activate(void)
{
nrf_bootloader_fw_activation_result_t result;
uint32_t ret_val = NRF_SUCCESS;
nrf_dfu_bank_t * p_bank = &s_dfu_settings.bank_1;
bool sd_update = false;
NRF_LOG_DEBUG("Enter nrf_bootloader_fw_activate");
switch (p_bank->bank_code)
{
case NRF_DFU_BANK_VALID_APP:
NRF_LOG_DEBUG("Valid App");
ret_val = app_activate();
break;
.
.
.
}
if (ret_val != NRF_SUCCESS)
{
NRF_LOG_ERROR("Activation failed with error %d (bank code: 0x%x)", ret_val, p_bank->bank_code);
result = ACTIVATION_ERROR;
// <-- The function is not returned here
}
// Invalidate bank, marking completion.
nrf_dfu_bank_invalidate(p_bank);
// Invalidate bank, marking completion.
nrf_dfu_bank_invalidate(p_bank);
m_flash_write_done = false;
ret_val = nrf_dfu_settings_write_and_backup(flash_write_callback);
ASSERT(m_flash_write_done); /* At this point flash module is performing blocking operation. It is expected that operation is already performed. */
if (ret_val == NRF_SUCCESS)
{
result = ACTIVATION_SUCCESS;
if (sd_update && (s_dfu_settings.bank_0.bank_code == NRF_DFU_BANK_VALID_APP))
{
//If SD was updated and application is valid we want to stay in DFU to receive application.
NRF_LOG_DEBUG("A SoftDevice has just been activated. It's likely that an application will come immediately");
result = ACTIVATION_SUCCESS_EXPECT_ADDITIONAL_UPDATE;
}
}
else
{
NRF_LOG_ERROR("Could not write settings.");
result = ACTIVATION_ERROR;
}
return result;
}
2. In the following one, if crc checking on the copied data was failed, the failure is not returned.
static uint32_t app_activate(void)
{
.
.
.
ret_val = image_copy(target_addr, src_addr, length_left, NRF_BL_FW_COPY_PROGRESS_STORE_STEP);
if (ret_val != NRF_SUCCESS)
{
NRF_LOG_ERROR("Failed to copy firmware.");
return ret_val;
}
// Check the CRC of the copied data. Enable if so.
crc = crc32_compute((uint8_t*)nrf_dfu_bank0_start_addr(), image_size, NULL);
if (crc == s_dfu_settings.bank_1.image_crc)
{
NRF_LOG_DEBUG("Setting app as valid");
s_dfu_settings.bank_0.bank_code = NRF_DFU_BANK_VALID_APP;
s_dfu_settings.bank_0.image_crc = crc;
s_dfu_settings.bank_0.image_size = image_size;
}
else
{
NRF_LOG_ERROR("CRC computation failed for copied app: "
"src crc: 0x%08x, res crc: 0x%08x",
s_dfu_settings.bank_1.image_crc,
crc);
// <-- CRC error is not returned
}
return ret_val;
}
I'm not sure if my understanding is correct, and please help to illustrate if otherwise. Thanks!
Regards,
Sam
