From 5209701add1fd6b9aaea387beb5b99fcb3b8afe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=81=8F?= Date: Wed, 10 Nov 2021 12:53:00 +0300 Subject: [PATCH] [FL-1991] Rewrite flash driver (#813) * Toolbox: fix null pointer dereference in manchester decoder. * FuriHal: use MPU to protect from null pointer dereference, replace flash driver with our own implementation. --- applications/storage/storages/storage-int.c | 2 +- firmware/targets/f6/Inc/FreeRTOSConfig.h | 2 +- firmware/targets/f6/furi-hal/furi-hal-bt.c | 95 +------ firmware/targets/f6/furi-hal/furi-hal-flash.c | 268 +++++++++++++++--- firmware/targets/f6/furi-hal/furi-hal-flash.h | 19 +- firmware/targets/f6/furi-hal/furi-hal.c | 16 ++ firmware/targets/f6/target.mk | 2 - firmware/targets/f7/Inc/FreeRTOSConfig.h | 2 +- firmware/targets/f7/furi-hal/furi-hal-bt.c | 95 +------ firmware/targets/f7/furi-hal/furi-hal-flash.c | 268 +++++++++++++++--- firmware/targets/f7/furi-hal/furi-hal-flash.h | 19 +- firmware/targets/f7/furi-hal/furi-hal.c | 16 ++ firmware/targets/f7/target.mk | 2 - .../targets/furi-hal-include/furi-hal-bt.h | 18 +- lib/toolbox/manchester-decoder.c | 4 +- 15 files changed, 535 insertions(+), 293 deletions(-) diff --git a/applications/storage/storages/storage-int.c b/applications/storage/storages/storage-int.c index 4a1dca7c..3eabdec7 100644 --- a/applications/storage/storages/storage-int.c +++ b/applications/storage/storages/storage-int.c @@ -121,7 +121,7 @@ static int storage_int_device_erase(const struct lfs_config* c, lfs_block_t bloc FURI_LOG_D(TAG, "Device erase: page %d, translated page: %x", block, page); - if(furi_hal_flash_erase(page, 1)) { + if(furi_hal_flash_erase(page)) { return 0; } else { return -1; diff --git a/firmware/targets/f6/Inc/FreeRTOSConfig.h b/firmware/targets/f6/Inc/FreeRTOSConfig.h index 228b530d..2e0706a3 100644 --- a/firmware/targets/f6/Inc/FreeRTOSConfig.h +++ b/firmware/targets/f6/Inc/FreeRTOSConfig.h @@ -57,7 +57,7 @@ #endif /* CMSIS_device_header */ #define configENABLE_FPU 1 -#define configENABLE_MPU 1 +#define configENABLE_MPU 0 #define configUSE_PREEMPTION 1 #define configSUPPORT_STATIC_ALLOCATION 0 diff --git a/firmware/targets/f6/furi-hal/furi-hal-bt.c b/firmware/targets/f6/furi-hal/furi-hal-bt.c index 5b68cdd0..95a07b9d 100644 --- a/firmware/targets/f6/furi-hal/furi-hal-bt.c +++ b/firmware/targets/f6/furi-hal/furi-hal-bt.c @@ -12,6 +12,16 @@ void furi_hal_bt_init() { furi_hal_bt_core2_mtx = osMutexNew(NULL); } +void furi_hal_bt_lock_core2() { + furi_assert(furi_hal_bt_core2_mtx); + furi_check(osMutexAcquire(furi_hal_bt_core2_mtx, osWaitForever) == osOK); +} + +void furi_hal_bt_unlock_core2() { + furi_assert(furi_hal_bt_core2_mtx); + furi_check(osMutexRelease(furi_hal_bt_core2_mtx) == osOK); +} + static bool furi_hal_bt_wait_startup() { uint16_t counter = 0; while (!(ble_glue_get_status() == BleGlueStatusStarted || ble_glue_get_status() == BleGlueStatusBleStackMissing)) { @@ -130,91 +140,6 @@ bool furi_hal_bt_is_active() { return gap_get_state() > GapStateIdle; } -static void furi_hal_bt_lock_flash_core2(bool erase_flag) { - // Take flash controller ownership - while (HAL_HSEM_FastTake(CFG_HW_FLASH_SEMID) != HAL_OK) { - taskYIELD(); - } - - // Unlock flash operation - HAL_FLASH_Unlock(); - - // Erase activity notification - if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_ON); - - while(true) { - // Wait till flash controller become usable - while(LL_FLASH_IsActiveFlag_OperationSuspended()) { - taskYIELD(); - }; - - // Just a little more love - taskENTER_CRITICAL(); - - // Actually we already have mutex for it, but specification is specification - if (HAL_HSEM_IsSemTaken(CFG_HW_BLOCK_FLASH_REQ_BY_CPU1_SEMID)) { - taskEXIT_CRITICAL(); - continue; - } - - // Take sempahopre and prevent core2 from anyting funky - if (HAL_HSEM_FastTake(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID) != HAL_OK) { - taskEXIT_CRITICAL(); - continue; - } - - break; - } -} - -void furi_hal_bt_lock_flash(bool erase_flag) { - // Acquire dangerous ops mutex - osMutexAcquire(furi_hal_bt_core2_mtx, osWaitForever); - - // If Core2 is running use IPC locking - BleGlueStatus status = ble_glue_get_status(); - if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { - furi_hal_bt_lock_flash_core2(erase_flag); - } else { - HAL_FLASH_Unlock(); - } -} - -static void furi_hal_bt_unlock_flash_core2(bool erase_flag) { - // Funky ops are ok at this point - HAL_HSEM_Release(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID, 0); - - // Task switching is ok - taskEXIT_CRITICAL(); - - // Doesn't make much sense, does it? - while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { - taskYIELD(); - } - - // Erase activity over, core2 can continue - if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_OFF); - - // Lock flash controller - HAL_FLASH_Lock(); - - // Release flash controller ownership - HAL_HSEM_Release(CFG_HW_FLASH_SEMID, 0); -} - -void furi_hal_bt_unlock_flash(bool erase_flag) { - // If Core2 is running use IPC locking - BleGlueStatus status = ble_glue_get_status(); - if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { - furi_hal_bt_unlock_flash_core2(erase_flag); - } else { - HAL_FLASH_Lock(); - } - - // Release dangerous ops mutex - osMutexRelease(furi_hal_bt_core2_mtx); -} - void furi_hal_bt_start_tone_tx(uint8_t channel, uint8_t power) { aci_hal_set_tx_power_level(0, power); aci_hal_tone_start(channel, 0); diff --git a/firmware/targets/f6/furi-hal/furi-hal-flash.c b/firmware/targets/f6/furi-hal/furi-hal-flash.c index 1e270bc0..8bee45c0 100644 --- a/firmware/targets/f6/furi-hal/furi-hal-flash.c +++ b/firmware/targets/f6/furi-hal/furi-hal-flash.c @@ -1,12 +1,11 @@ #include #include #include +#include +#include #include -/* Free flash space borders, exported by linker */ -extern const void __free_flash_start__; - #define FURI_HAL_TAG "FuriHalFlash" #define FURI_HAL_CRITICAL_MSG "Critical flash operation fail" #define FURI_HAL_FLASH_READ_BLOCK 8 @@ -14,6 +13,9 @@ extern const void __free_flash_start__; #define FURI_HAL_FLASH_PAGE_SIZE 4096 #define FURI_HAL_FLASH_CYCLES_COUNT 10000 +/* Free flash space borders, exported by linker */ +extern const void __free_flash_start__; + size_t furi_hal_flash_get_base() { return FLASH_BASE; } @@ -39,9 +41,9 @@ const void* furi_hal_flash_get_free_start_address() { } const void* furi_hal_flash_get_free_end_address() { - FLASH_OBProgramInitTypeDef pOBInit; - HAL_FLASHEx_OBGetConfig(&pOBInit); - return (const void *)pOBInit.SecureFlashStartAddr; + uint32_t sfr_reg_val = READ_REG(FLASH->SFR); + uint32_t sfsa = (READ_BIT(sfr_reg_val, FLASH_SFR_SFSA) >> FLASH_SFR_SFSA_Pos); + return (const void *)((sfsa * FLASH_PAGE_SIZE) + FLASH_BASE); } size_t furi_hal_flash_get_free_page_start_address() { @@ -59,47 +61,241 @@ size_t furi_hal_flash_get_free_page_count() { return (end-page_start) / FURI_HAL_FLASH_PAGE_SIZE; } -bool furi_hal_flash_erase(uint8_t page, uint8_t count) { - furi_hal_bt_lock_flash(true); +static void furi_hal_flash_unlock() { + /* verify Flash is locked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) != 0U); - FLASH_EraseInitTypeDef erase; - erase.TypeErase = FLASH_TYPEERASE_PAGES; - erase.Page = page; - erase.NbPages = count; + /* Authorize the FLASH Registers access */ + WRITE_REG(FLASH->KEYR, FLASH_KEY1); + WRITE_REG(FLASH->KEYR, FLASH_KEY2); - uint32_t error_page = 0; - HAL_StatusTypeDef status = HAL_FLASHEx_Erase(&erase, &error_page); - if (status != HAL_OK) { - FURI_LOG_E(FURI_HAL_TAG, "Erase failed, ret: %d, page: %d", status, error_page); - furi_crash(FURI_HAL_CRITICAL_MSG); + /* verify Flash is unlock */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) == 0U); +} + +static void furi_hal_flash_lock(void) { + /* verify Flash is unlocked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) == 0U); + + /* Set the LOCK Bit to lock the FLASH Registers access */ + /* @Note The lock and unlock procedure is done only using CR registers even from CPU2 */ + SET_BIT(FLASH->CR, FLASH_CR_LOCK); + + /* verify Flash is locked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) != 0U); +} + +static void furi_hal_flash_begin_with_core2(bool erase_flag) { + // Take flash controller ownership + while (HAL_HSEM_FastTake(CFG_HW_FLASH_SEMID) != HAL_OK) { + taskYIELD(); } - furi_hal_bt_unlock_flash(true); + // Unlock flash operation + furi_hal_flash_unlock(); + + // Erase activity notification + if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_ON); + + while(true) { + // Wait till flash controller become usable + while(LL_FLASH_IsActiveFlag_OperationSuspended()) { + taskYIELD(); + }; + + // Just a little more love + taskENTER_CRITICAL(); + + // Actually we already have mutex for it, but specification is specification + if (HAL_HSEM_IsSemTaken(CFG_HW_BLOCK_FLASH_REQ_BY_CPU1_SEMID)) { + taskEXIT_CRITICAL(); + continue; + } + + // Take sempahopre and prevent core2 from anyting funky + if (HAL_HSEM_FastTake(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID) != HAL_OK) { + taskEXIT_CRITICAL(); + continue; + } + + break; + } +} + +static void furi_hal_flash_begin(bool erase_flag) { + // Acquire dangerous ops mutex + furi_hal_bt_lock_core2(); + + // If Core2 is running use IPC locking + BleGlueStatus status = ble_glue_get_status(); + if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { + furi_hal_flash_begin_with_core2(erase_flag); + } else { + furi_hal_flash_unlock(); + } +} + +static void furi_hal_flash_end_with_core2(bool erase_flag) { + // Funky ops are ok at this point + HAL_HSEM_Release(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID, 0); + + // Task switching is ok + taskEXIT_CRITICAL(); + + // Doesn't make much sense, does it? + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { + taskYIELD(); + } + + // Erase activity over, core2 can continue + if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_OFF); + + // Lock flash controller + furi_hal_flash_lock(); + + // Release flash controller ownership + HAL_HSEM_Release(CFG_HW_FLASH_SEMID, 0); +} + +static void furi_hal_flash_end(bool erase_flag) { + // If Core2 is running use IPC locking + BleGlueStatus status = ble_glue_get_status(); + if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { + furi_hal_flash_end_with_core2(erase_flag); + } else { + furi_hal_flash_lock(); + } + + // Release dangerous ops mutex + furi_hal_bt_unlock_core2(); +} + +static void furi_hal_flush_cache(void) { + /* Flush instruction cache */ + if (READ_BIT(FLASH->ACR, FLASH_ACR_ICEN) == FLASH_ACR_ICEN) { + /* Disable instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_DISABLE(); + /* Reset instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_RESET(); + /* Enable instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_ENABLE(); + } + + /* Flush data cache */ + if (READ_BIT(FLASH->ACR, FLASH_ACR_DCEN) == FLASH_ACR_DCEN) { + /* Disable data cache */ + __HAL_FLASH_DATA_CACHE_DISABLE(); + /* Reset data cache */ + __HAL_FLASH_DATA_CACHE_RESET(); + /* Enable data cache */ + __HAL_FLASH_DATA_CACHE_ENABLE(); + } +} + +HAL_StatusTypeDef furi_hal_flash_wait_last_operation(uint32_t timeout) { + uint32_t error = 0; + uint32_t countdown = 0; + + // Wait for the FLASH operation to complete by polling on BUSY flag to be reset. + // Even if the FLASH operation fails, the BUSY flag will be reset and an error + // flag will be set + countdown = timeout; + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { + if(LL_SYSTICK_IsActiveCounterFlag()) { + countdown--; + } + if (countdown == 0) { + return HAL_TIMEOUT; + } + } + + /* Check FLASH operation error flags */ + error = FLASH->SR; + + /* Check FLASH End of Operation flag */ + if ((error & FLASH_FLAG_EOP) != 0U) { + /* Clear FLASH End of Operation pending bit */ + __HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_EOP); + } + + /* Now update error variable to only error value */ + error &= FLASH_FLAG_SR_ERRORS; + + furi_check(error == 0); + + /* clear error flags */ + __HAL_FLASH_CLEAR_FLAG(error); + + /* Wait for control register to be written */ + countdown = timeout; + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_CFGBSY)) { + if(LL_SYSTICK_IsActiveCounterFlag()) { + countdown--; + } + if (countdown == 0) { + return HAL_TIMEOUT; + } + } + + return HAL_OK; +} + +bool furi_hal_flash_erase(uint8_t page) { + furi_hal_flash_begin(true); + + // Ensure that controller state is valid + furi_check(FLASH->SR == 0); + + /* Verify that next operation can be proceed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* Select page and start operation */ + MODIFY_REG(FLASH->CR, FLASH_CR_PNB, ((page << FLASH_CR_PNB_Pos) | FLASH_CR_PER | FLASH_CR_STRT)); + + /* Wait for last operation to be completed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* If operation is completed or interrupted, disable the Page Erase Bit */ + CLEAR_BIT(FLASH->CR, (FLASH_CR_PER | FLASH_CR_PNB)); + + /* Flush the caches to be sure of the data consistency */ + furi_hal_flush_cache(); + + furi_hal_flash_end(true); return true; } bool furi_hal_flash_write_dword(size_t address, uint64_t data) { - furi_hal_bt_lock_flash(false); + furi_hal_flash_begin(false); - HAL_StatusTypeDef status = HAL_FLASH_Program(FLASH_TYPEPROGRAM_DOUBLEWORD, address, data); - if (status != HAL_OK) { - FURI_LOG_E(FURI_HAL_TAG, "Programming failed, ret: %d, address: %p", status, address); - furi_crash(FURI_HAL_CRITICAL_MSG); - } + // Ensure that controller state is valid + furi_check(FLASH->SR == 0); - furi_hal_bt_unlock_flash(false); - - return true; -} - -bool furi_hal_flash_write_row(size_t address, size_t source_address) { - furi_hal_bt_lock_flash(false); - - HAL_StatusTypeDef status = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FAST, address, source_address); - furi_check(status == HAL_OK); - - furi_hal_bt_unlock_flash(false); + /* Check the parameters */ + furi_check(IS_ADDR_ALIGNED_64BITS(address)); + furi_check(IS_FLASH_PROGRAM_ADDRESS(address)); + + /* Set PG bit */ + SET_BIT(FLASH->CR, FLASH_CR_PG); + + /* Program first word */ + *(uint32_t *)address = (uint32_t)data; + + // Barrier to ensure programming is performed in 2 steps, in right order + // (independently of compiler optimization behavior) + __ISB(); + + /* Program second word */ + *(uint32_t *)(address + 4U) = (uint32_t)(data >> 32U); + + /* Wait for last operation to be completed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* If the program operation is completed, disable the PG or FSTPG Bit */ + CLEAR_BIT(FLASH->CR, FLASH_CR_PG); + + furi_hal_flash_end(false); return true; } diff --git a/firmware/targets/f6/furi-hal/furi-hal-flash.h b/firmware/targets/f6/furi-hal/furi-hal-flash.h index 92a65a65..3d8031e6 100644 --- a/firmware/targets/f6/furi-hal/furi-hal-flash.h +++ b/firmware/targets/f6/furi-hal/furi-hal-flash.h @@ -60,18 +60,17 @@ size_t furi_hal_flash_get_free_page_count(); /** Erase Flash * - * Locking operation, uses HSEM to manage shared access. + * @warning locking operation with critical section, stales execution * - * @param page page number - * @param count page count to erase + * @param page The page to erase * * @return true on success */ -bool furi_hal_flash_erase(uint8_t page, uint8_t count); +bool furi_hal_flash_erase(uint8_t page); /** Write double word (64 bits) * - * Locking operation, uses HSEM to manage shared access. + * @warning locking operation with critical section, stales execution * * @param address destination address, must be double word aligned. * @param data data to write @@ -80,13 +79,3 @@ bool furi_hal_flash_erase(uint8_t page, uint8_t count); */ bool furi_hal_flash_write_dword(size_t address, uint64_t data); -/** Write row: 64 double word (64 bits) from address - * - * Locking operation, uses HSEM to manage shared access. - * - * @param address destination address, must be block aligned - * @param source_address source address - * - * @return true on success - */ -bool furi_hal_flash_write_row(size_t address, size_t source_address); diff --git a/firmware/targets/f6/furi-hal/furi-hal.c b/firmware/targets/f6/furi-hal/furi-hal.c index aa7536ef..f332d187 100644 --- a/firmware/targets/f6/furi-hal/furi-hal.c +++ b/firmware/targets/f6/furi-hal/furi-hal.c @@ -5,6 +5,8 @@ #include #include +#include + void furi_hal_init() { furi_hal_clock_init(); furi_hal_console_init(); @@ -52,4 +54,18 @@ void furi_hal_init() { // FreeRTOS glue furi_hal_os_init(); + + // Partial null pointer dereference protection + LL_MPU_Disable(); + LL_MPU_ConfigRegion( + LL_MPU_REGION_NUMBER0, 0x00, 0x0, + LL_MPU_REGION_SIZE_1MB + | LL_MPU_REGION_PRIV_RO_URO + | LL_MPU_ACCESS_BUFFERABLE + | LL_MPU_ACCESS_CACHEABLE + | LL_MPU_ACCESS_SHAREABLE + | LL_MPU_TEX_LEVEL1 + | LL_MPU_INSTRUCTION_ACCESS_ENABLE + ); + LL_MPU_Enable(LL_MPU_CTRL_PRIVILEGED_DEFAULT); } diff --git a/firmware/targets/f6/target.mk b/firmware/targets/f6/target.mk index 0b18617e..879d6272 100644 --- a/firmware/targets/f6/target.mk +++ b/firmware/targets/f6/target.mk @@ -49,8 +49,6 @@ C_SOURCES += \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_cortex.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_cryp.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_exti.c \ - $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_flash.c \ - $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_flash_ex.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_gpio.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_hsem.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_ipcc.c \ diff --git a/firmware/targets/f7/Inc/FreeRTOSConfig.h b/firmware/targets/f7/Inc/FreeRTOSConfig.h index 228b530d..2e0706a3 100644 --- a/firmware/targets/f7/Inc/FreeRTOSConfig.h +++ b/firmware/targets/f7/Inc/FreeRTOSConfig.h @@ -57,7 +57,7 @@ #endif /* CMSIS_device_header */ #define configENABLE_FPU 1 -#define configENABLE_MPU 1 +#define configENABLE_MPU 0 #define configUSE_PREEMPTION 1 #define configSUPPORT_STATIC_ALLOCATION 0 diff --git a/firmware/targets/f7/furi-hal/furi-hal-bt.c b/firmware/targets/f7/furi-hal/furi-hal-bt.c index 5b68cdd0..95a07b9d 100644 --- a/firmware/targets/f7/furi-hal/furi-hal-bt.c +++ b/firmware/targets/f7/furi-hal/furi-hal-bt.c @@ -12,6 +12,16 @@ void furi_hal_bt_init() { furi_hal_bt_core2_mtx = osMutexNew(NULL); } +void furi_hal_bt_lock_core2() { + furi_assert(furi_hal_bt_core2_mtx); + furi_check(osMutexAcquire(furi_hal_bt_core2_mtx, osWaitForever) == osOK); +} + +void furi_hal_bt_unlock_core2() { + furi_assert(furi_hal_bt_core2_mtx); + furi_check(osMutexRelease(furi_hal_bt_core2_mtx) == osOK); +} + static bool furi_hal_bt_wait_startup() { uint16_t counter = 0; while (!(ble_glue_get_status() == BleGlueStatusStarted || ble_glue_get_status() == BleGlueStatusBleStackMissing)) { @@ -130,91 +140,6 @@ bool furi_hal_bt_is_active() { return gap_get_state() > GapStateIdle; } -static void furi_hal_bt_lock_flash_core2(bool erase_flag) { - // Take flash controller ownership - while (HAL_HSEM_FastTake(CFG_HW_FLASH_SEMID) != HAL_OK) { - taskYIELD(); - } - - // Unlock flash operation - HAL_FLASH_Unlock(); - - // Erase activity notification - if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_ON); - - while(true) { - // Wait till flash controller become usable - while(LL_FLASH_IsActiveFlag_OperationSuspended()) { - taskYIELD(); - }; - - // Just a little more love - taskENTER_CRITICAL(); - - // Actually we already have mutex for it, but specification is specification - if (HAL_HSEM_IsSemTaken(CFG_HW_BLOCK_FLASH_REQ_BY_CPU1_SEMID)) { - taskEXIT_CRITICAL(); - continue; - } - - // Take sempahopre and prevent core2 from anyting funky - if (HAL_HSEM_FastTake(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID) != HAL_OK) { - taskEXIT_CRITICAL(); - continue; - } - - break; - } -} - -void furi_hal_bt_lock_flash(bool erase_flag) { - // Acquire dangerous ops mutex - osMutexAcquire(furi_hal_bt_core2_mtx, osWaitForever); - - // If Core2 is running use IPC locking - BleGlueStatus status = ble_glue_get_status(); - if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { - furi_hal_bt_lock_flash_core2(erase_flag); - } else { - HAL_FLASH_Unlock(); - } -} - -static void furi_hal_bt_unlock_flash_core2(bool erase_flag) { - // Funky ops are ok at this point - HAL_HSEM_Release(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID, 0); - - // Task switching is ok - taskEXIT_CRITICAL(); - - // Doesn't make much sense, does it? - while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { - taskYIELD(); - } - - // Erase activity over, core2 can continue - if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_OFF); - - // Lock flash controller - HAL_FLASH_Lock(); - - // Release flash controller ownership - HAL_HSEM_Release(CFG_HW_FLASH_SEMID, 0); -} - -void furi_hal_bt_unlock_flash(bool erase_flag) { - // If Core2 is running use IPC locking - BleGlueStatus status = ble_glue_get_status(); - if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { - furi_hal_bt_unlock_flash_core2(erase_flag); - } else { - HAL_FLASH_Lock(); - } - - // Release dangerous ops mutex - osMutexRelease(furi_hal_bt_core2_mtx); -} - void furi_hal_bt_start_tone_tx(uint8_t channel, uint8_t power) { aci_hal_set_tx_power_level(0, power); aci_hal_tone_start(channel, 0); diff --git a/firmware/targets/f7/furi-hal/furi-hal-flash.c b/firmware/targets/f7/furi-hal/furi-hal-flash.c index 1e270bc0..8bee45c0 100644 --- a/firmware/targets/f7/furi-hal/furi-hal-flash.c +++ b/firmware/targets/f7/furi-hal/furi-hal-flash.c @@ -1,12 +1,11 @@ #include #include #include +#include +#include #include -/* Free flash space borders, exported by linker */ -extern const void __free_flash_start__; - #define FURI_HAL_TAG "FuriHalFlash" #define FURI_HAL_CRITICAL_MSG "Critical flash operation fail" #define FURI_HAL_FLASH_READ_BLOCK 8 @@ -14,6 +13,9 @@ extern const void __free_flash_start__; #define FURI_HAL_FLASH_PAGE_SIZE 4096 #define FURI_HAL_FLASH_CYCLES_COUNT 10000 +/* Free flash space borders, exported by linker */ +extern const void __free_flash_start__; + size_t furi_hal_flash_get_base() { return FLASH_BASE; } @@ -39,9 +41,9 @@ const void* furi_hal_flash_get_free_start_address() { } const void* furi_hal_flash_get_free_end_address() { - FLASH_OBProgramInitTypeDef pOBInit; - HAL_FLASHEx_OBGetConfig(&pOBInit); - return (const void *)pOBInit.SecureFlashStartAddr; + uint32_t sfr_reg_val = READ_REG(FLASH->SFR); + uint32_t sfsa = (READ_BIT(sfr_reg_val, FLASH_SFR_SFSA) >> FLASH_SFR_SFSA_Pos); + return (const void *)((sfsa * FLASH_PAGE_SIZE) + FLASH_BASE); } size_t furi_hal_flash_get_free_page_start_address() { @@ -59,47 +61,241 @@ size_t furi_hal_flash_get_free_page_count() { return (end-page_start) / FURI_HAL_FLASH_PAGE_SIZE; } -bool furi_hal_flash_erase(uint8_t page, uint8_t count) { - furi_hal_bt_lock_flash(true); +static void furi_hal_flash_unlock() { + /* verify Flash is locked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) != 0U); - FLASH_EraseInitTypeDef erase; - erase.TypeErase = FLASH_TYPEERASE_PAGES; - erase.Page = page; - erase.NbPages = count; + /* Authorize the FLASH Registers access */ + WRITE_REG(FLASH->KEYR, FLASH_KEY1); + WRITE_REG(FLASH->KEYR, FLASH_KEY2); - uint32_t error_page = 0; - HAL_StatusTypeDef status = HAL_FLASHEx_Erase(&erase, &error_page); - if (status != HAL_OK) { - FURI_LOG_E(FURI_HAL_TAG, "Erase failed, ret: %d, page: %d", status, error_page); - furi_crash(FURI_HAL_CRITICAL_MSG); + /* verify Flash is unlock */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) == 0U); +} + +static void furi_hal_flash_lock(void) { + /* verify Flash is unlocked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) == 0U); + + /* Set the LOCK Bit to lock the FLASH Registers access */ + /* @Note The lock and unlock procedure is done only using CR registers even from CPU2 */ + SET_BIT(FLASH->CR, FLASH_CR_LOCK); + + /* verify Flash is locked */ + furi_check(READ_BIT(FLASH->CR, FLASH_CR_LOCK) != 0U); +} + +static void furi_hal_flash_begin_with_core2(bool erase_flag) { + // Take flash controller ownership + while (HAL_HSEM_FastTake(CFG_HW_FLASH_SEMID) != HAL_OK) { + taskYIELD(); } - furi_hal_bt_unlock_flash(true); + // Unlock flash operation + furi_hal_flash_unlock(); + + // Erase activity notification + if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_ON); + + while(true) { + // Wait till flash controller become usable + while(LL_FLASH_IsActiveFlag_OperationSuspended()) { + taskYIELD(); + }; + + // Just a little more love + taskENTER_CRITICAL(); + + // Actually we already have mutex for it, but specification is specification + if (HAL_HSEM_IsSemTaken(CFG_HW_BLOCK_FLASH_REQ_BY_CPU1_SEMID)) { + taskEXIT_CRITICAL(); + continue; + } + + // Take sempahopre and prevent core2 from anyting funky + if (HAL_HSEM_FastTake(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID) != HAL_OK) { + taskEXIT_CRITICAL(); + continue; + } + + break; + } +} + +static void furi_hal_flash_begin(bool erase_flag) { + // Acquire dangerous ops mutex + furi_hal_bt_lock_core2(); + + // If Core2 is running use IPC locking + BleGlueStatus status = ble_glue_get_status(); + if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { + furi_hal_flash_begin_with_core2(erase_flag); + } else { + furi_hal_flash_unlock(); + } +} + +static void furi_hal_flash_end_with_core2(bool erase_flag) { + // Funky ops are ok at this point + HAL_HSEM_Release(CFG_HW_BLOCK_FLASH_REQ_BY_CPU2_SEMID, 0); + + // Task switching is ok + taskEXIT_CRITICAL(); + + // Doesn't make much sense, does it? + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { + taskYIELD(); + } + + // Erase activity over, core2 can continue + if(erase_flag) SHCI_C2_FLASH_EraseActivity(ERASE_ACTIVITY_OFF); + + // Lock flash controller + furi_hal_flash_lock(); + + // Release flash controller ownership + HAL_HSEM_Release(CFG_HW_FLASH_SEMID, 0); +} + +static void furi_hal_flash_end(bool erase_flag) { + // If Core2 is running use IPC locking + BleGlueStatus status = ble_glue_get_status(); + if(status == BleGlueStatusStarted || status == BleGlueStatusBleStackMissing) { + furi_hal_flash_end_with_core2(erase_flag); + } else { + furi_hal_flash_lock(); + } + + // Release dangerous ops mutex + furi_hal_bt_unlock_core2(); +} + +static void furi_hal_flush_cache(void) { + /* Flush instruction cache */ + if (READ_BIT(FLASH->ACR, FLASH_ACR_ICEN) == FLASH_ACR_ICEN) { + /* Disable instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_DISABLE(); + /* Reset instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_RESET(); + /* Enable instruction cache */ + __HAL_FLASH_INSTRUCTION_CACHE_ENABLE(); + } + + /* Flush data cache */ + if (READ_BIT(FLASH->ACR, FLASH_ACR_DCEN) == FLASH_ACR_DCEN) { + /* Disable data cache */ + __HAL_FLASH_DATA_CACHE_DISABLE(); + /* Reset data cache */ + __HAL_FLASH_DATA_CACHE_RESET(); + /* Enable data cache */ + __HAL_FLASH_DATA_CACHE_ENABLE(); + } +} + +HAL_StatusTypeDef furi_hal_flash_wait_last_operation(uint32_t timeout) { + uint32_t error = 0; + uint32_t countdown = 0; + + // Wait for the FLASH operation to complete by polling on BUSY flag to be reset. + // Even if the FLASH operation fails, the BUSY flag will be reset and an error + // flag will be set + countdown = timeout; + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) { + if(LL_SYSTICK_IsActiveCounterFlag()) { + countdown--; + } + if (countdown == 0) { + return HAL_TIMEOUT; + } + } + + /* Check FLASH operation error flags */ + error = FLASH->SR; + + /* Check FLASH End of Operation flag */ + if ((error & FLASH_FLAG_EOP) != 0U) { + /* Clear FLASH End of Operation pending bit */ + __HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_EOP); + } + + /* Now update error variable to only error value */ + error &= FLASH_FLAG_SR_ERRORS; + + furi_check(error == 0); + + /* clear error flags */ + __HAL_FLASH_CLEAR_FLAG(error); + + /* Wait for control register to be written */ + countdown = timeout; + while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_CFGBSY)) { + if(LL_SYSTICK_IsActiveCounterFlag()) { + countdown--; + } + if (countdown == 0) { + return HAL_TIMEOUT; + } + } + + return HAL_OK; +} + +bool furi_hal_flash_erase(uint8_t page) { + furi_hal_flash_begin(true); + + // Ensure that controller state is valid + furi_check(FLASH->SR == 0); + + /* Verify that next operation can be proceed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* Select page and start operation */ + MODIFY_REG(FLASH->CR, FLASH_CR_PNB, ((page << FLASH_CR_PNB_Pos) | FLASH_CR_PER | FLASH_CR_STRT)); + + /* Wait for last operation to be completed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* If operation is completed or interrupted, disable the Page Erase Bit */ + CLEAR_BIT(FLASH->CR, (FLASH_CR_PER | FLASH_CR_PNB)); + + /* Flush the caches to be sure of the data consistency */ + furi_hal_flush_cache(); + + furi_hal_flash_end(true); return true; } bool furi_hal_flash_write_dword(size_t address, uint64_t data) { - furi_hal_bt_lock_flash(false); + furi_hal_flash_begin(false); - HAL_StatusTypeDef status = HAL_FLASH_Program(FLASH_TYPEPROGRAM_DOUBLEWORD, address, data); - if (status != HAL_OK) { - FURI_LOG_E(FURI_HAL_TAG, "Programming failed, ret: %d, address: %p", status, address); - furi_crash(FURI_HAL_CRITICAL_MSG); - } + // Ensure that controller state is valid + furi_check(FLASH->SR == 0); - furi_hal_bt_unlock_flash(false); - - return true; -} - -bool furi_hal_flash_write_row(size_t address, size_t source_address) { - furi_hal_bt_lock_flash(false); - - HAL_StatusTypeDef status = HAL_FLASH_Program(FLASH_TYPEPROGRAM_FAST, address, source_address); - furi_check(status == HAL_OK); - - furi_hal_bt_unlock_flash(false); + /* Check the parameters */ + furi_check(IS_ADDR_ALIGNED_64BITS(address)); + furi_check(IS_FLASH_PROGRAM_ADDRESS(address)); + + /* Set PG bit */ + SET_BIT(FLASH->CR, FLASH_CR_PG); + + /* Program first word */ + *(uint32_t *)address = (uint32_t)data; + + // Barrier to ensure programming is performed in 2 steps, in right order + // (independently of compiler optimization behavior) + __ISB(); + + /* Program second word */ + *(uint32_t *)(address + 4U) = (uint32_t)(data >> 32U); + + /* Wait for last operation to be completed */ + furi_check(furi_hal_flash_wait_last_operation(FLASH_TIMEOUT_VALUE) == HAL_OK); + + /* If the program operation is completed, disable the PG or FSTPG Bit */ + CLEAR_BIT(FLASH->CR, FLASH_CR_PG); + + furi_hal_flash_end(false); return true; } diff --git a/firmware/targets/f7/furi-hal/furi-hal-flash.h b/firmware/targets/f7/furi-hal/furi-hal-flash.h index 92a65a65..3d8031e6 100644 --- a/firmware/targets/f7/furi-hal/furi-hal-flash.h +++ b/firmware/targets/f7/furi-hal/furi-hal-flash.h @@ -60,18 +60,17 @@ size_t furi_hal_flash_get_free_page_count(); /** Erase Flash * - * Locking operation, uses HSEM to manage shared access. + * @warning locking operation with critical section, stales execution * - * @param page page number - * @param count page count to erase + * @param page The page to erase * * @return true on success */ -bool furi_hal_flash_erase(uint8_t page, uint8_t count); +bool furi_hal_flash_erase(uint8_t page); /** Write double word (64 bits) * - * Locking operation, uses HSEM to manage shared access. + * @warning locking operation with critical section, stales execution * * @param address destination address, must be double word aligned. * @param data data to write @@ -80,13 +79,3 @@ bool furi_hal_flash_erase(uint8_t page, uint8_t count); */ bool furi_hal_flash_write_dword(size_t address, uint64_t data); -/** Write row: 64 double word (64 bits) from address - * - * Locking operation, uses HSEM to manage shared access. - * - * @param address destination address, must be block aligned - * @param source_address source address - * - * @return true on success - */ -bool furi_hal_flash_write_row(size_t address, size_t source_address); diff --git a/firmware/targets/f7/furi-hal/furi-hal.c b/firmware/targets/f7/furi-hal/furi-hal.c index aa7536ef..f332d187 100644 --- a/firmware/targets/f7/furi-hal/furi-hal.c +++ b/firmware/targets/f7/furi-hal/furi-hal.c @@ -5,6 +5,8 @@ #include #include +#include + void furi_hal_init() { furi_hal_clock_init(); furi_hal_console_init(); @@ -52,4 +54,18 @@ void furi_hal_init() { // FreeRTOS glue furi_hal_os_init(); + + // Partial null pointer dereference protection + LL_MPU_Disable(); + LL_MPU_ConfigRegion( + LL_MPU_REGION_NUMBER0, 0x00, 0x0, + LL_MPU_REGION_SIZE_1MB + | LL_MPU_REGION_PRIV_RO_URO + | LL_MPU_ACCESS_BUFFERABLE + | LL_MPU_ACCESS_CACHEABLE + | LL_MPU_ACCESS_SHAREABLE + | LL_MPU_TEX_LEVEL1 + | LL_MPU_INSTRUCTION_ACCESS_ENABLE + ); + LL_MPU_Enable(LL_MPU_CTRL_PRIVILEGED_DEFAULT); } diff --git a/firmware/targets/f7/target.mk b/firmware/targets/f7/target.mk index 184bb9d2..761621d7 100644 --- a/firmware/targets/f7/target.mk +++ b/firmware/targets/f7/target.mk @@ -49,8 +49,6 @@ C_SOURCES += \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_cortex.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_cryp.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_exti.c \ - $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_flash.c \ - $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_flash_ex.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_gpio.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_hsem.c \ $(CUBE_DIR)/Drivers/STM32WBxx_HAL_Driver/Src/stm32wbxx_hal_ipcc.c \ diff --git a/firmware/targets/furi-hal-include/furi-hal-bt.h b/firmware/targets/furi-hal-include/furi-hal-bt.h index 089ca9ae..acbdfc36 100644 --- a/firmware/targets/furi-hal-include/furi-hal-bt.h +++ b/firmware/targets/furi-hal-include/furi-hal-bt.h @@ -23,6 +23,12 @@ extern "C" { */ void furi_hal_bt_init(); +/** Lock core2 state transition */ +void furi_hal_bt_lock_core2(); + +/** Lock core2 state transition */ +void furi_hal_bt_unlock_core2(); + /** Start 2nd core and BLE stack * * @return true on success @@ -103,18 +109,6 @@ void furi_hal_bt_notify_buffer_is_empty(); */ bool furi_hal_bt_tx(uint8_t* data, uint16_t size); -/** Lock shared access to flash controller - * - * @param[in] erase_flag true if erase operation - */ -void furi_hal_bt_lock_flash(bool erase_flag); - -/** Unlock shared access to flash controller - * - * @param[in] erase_flag true if erase operation - */ -void furi_hal_bt_unlock_flash(bool erase_flag); - /** Start ble tone tx at given channel and power * * @param[in] channel The channel diff --git a/lib/toolbox/manchester-decoder.c b/lib/toolbox/manchester-decoder.c index cbeab44a..2affe5cf 100644 --- a/lib/toolbox/manchester-decoder.c +++ b/lib/toolbox/manchester-decoder.c @@ -20,10 +20,10 @@ bool manchester_advance( new_state = manchester_reset_state; } else { if(new_state == ManchesterStateMid0) { - *data = false; + if (data) *data = false; result = true; } else if(new_state == ManchesterStateMid1) { - *data = true; + if (data) *data = true; result = true; } }