* Furi HAL: memory protection unit
* Core: prohibit NULL dereferencing, even for reads.
* Applications: fix NULL dereference
* Core: stack protection by MPU
* MPU: stack region alignment
* Apps: fix null pointer dereferences
* Threads: fix non-null arg check
* Desktop settings: fix null pointer dereference
* Core: documented null-check hack
* Fix null dereference issues
* Apps: args check
* Core: naming fixes
* format code
* Core: remove NONNULL specifier
* FurHal: move MPU initialization to begining, fix enum naming

Co-authored-by: あく <alleteam@gmail.com>
This commit is contained in:
SG 2022-08-04 01:47:10 +10:00 committed by GitHub
parent 4a6477aaa8
commit eed4296890
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
29 changed files with 238 additions and 62 deletions

View File

@ -37,7 +37,7 @@ void archive_scene_rename_on_enter(void* context) {
false);
ValidatorIsFile* validator_is_file = validator_is_file_alloc_init(
string_get_cstr(archive->browser->path), archive->file_extension, NULL);
string_get_cstr(archive->browser->path), archive->file_extension, "");
text_input_set_validator(text_input, validator_is_file_callback, validator_is_file);
string_clear(filename);

View File

@ -28,7 +28,7 @@ BadUsbApp* bad_usb_app_alloc(char* arg) {
string_init(app->file_path);
if(arg != NULL) {
if(arg && strlen(arg)) {
string_set_str(app->file_path, arg);
}
@ -79,7 +79,6 @@ void bad_usb_app_free(BadUsbApp* app) {
furi_assert(app);
// Views
view_dispatcher_remove_view(app->view_dispatcher, BadUsbAppViewFileSelect);
view_dispatcher_remove_view(app->view_dispatcher, BadUsbAppViewWork);
bad_usb_free(app->bad_usb_view);

View File

@ -38,6 +38,5 @@ struct BadUsbApp {
typedef enum {
BadUsbAppViewError,
BadUsbAppViewFileSelect,
BadUsbAppViewWork,
} BadUsbAppView;

View File

@ -347,7 +347,8 @@ static void bt_close_connection(Bt* bt) {
furi_event_flag_set(bt->api_event, BT_API_UNLOCK_EVENT);
}
int32_t bt_srv() {
int32_t bt_srv(void* p) {
UNUSED(p);
Bt* bt = bt_alloc();
if(furi_hal_rtc_get_boot_mode() != FuriHalRtcBootModeNormal) {

View File

@ -143,7 +143,7 @@ static void bubble_animation_activate(BubbleAnimationView* view, bool force) {
furi_assert(view);
bool activate = true;
BubbleAnimationViewModel* model = view_get_model(view->view);
if(!model->current) {
if(model->current == NULL) {
activate = false;
} else if(model->freeze_frame) {
activate = false;
@ -151,14 +151,16 @@ static void bubble_animation_activate(BubbleAnimationView* view, bool force) {
activate = false;
}
if(!force) {
if((model->active_ended_at + model->current->active_cooldown * 1000) >
xTaskGetTickCount()) {
activate = false;
} else if(model->active_shift) {
activate = false;
} else if(model->current_frame >= model->current->passive_frames) {
activate = false;
if(model->current != NULL) {
if(!force) {
if((model->active_ended_at + model->current->active_cooldown * 1000) >
xTaskGetTickCount()) {
activate = false;
} else if(model->active_shift) {
activate = false;
} else if(model->current_frame >= model->current->passive_frames) {
activate = false;
}
}
}
view_commit_model(view->view, false);
@ -288,7 +290,10 @@ static void bubble_animation_enter(void* context) {
bubble_animation_activate(view, false);
BubbleAnimationViewModel* model = view_get_model(view->view);
uint8_t frame_rate = model->current->icon_animation.frame_rate;
uint8_t frame_rate = 0;
if(model->current != NULL) {
frame_rate = model->current->icon_animation.frame_rate;
}
view_commit_model(view->view, false);
if(frame_rate) {

View File

@ -90,7 +90,7 @@ void desktop_settings_app_free(DesktopSettingsApp* app) {
extern int32_t desktop_settings_app(void* p) {
DesktopSettingsApp* app = desktop_settings_app_alloc();
LOAD_DESKTOP_SETTINGS(&app->settings);
if(!strcmp(p, DESKTOP_SETTINGS_RUN_PIN_SETUP_ARG)) {
if(p && (strcmp(p, DESKTOP_SETTINGS_RUN_PIN_SETUP_ARG) == 0)) {
scene_manager_next_scene(app->scene_manager, DesktopSettingsAppScenePinSetupHowto);
} else {
scene_manager_next_scene(app->scene_manager, DesktopSettingsAppSceneStart);

View File

@ -185,17 +185,19 @@ static void button_menu_process_ok(ButtonMenu* button_menu, InputType type) {
return false;
});
if(item->type == ButtonMenuItemTypeControl) {
if(type == InputTypeShort) {
if(item && item->callback) {
item->callback(item->callback_context, item->index, type);
if(item) {
if(item->type == ButtonMenuItemTypeControl) {
if(type == InputTypeShort) {
if(item && item->callback) {
item->callback(item->callback_context, item->index, type);
}
}
}
}
if(item->type == ButtonMenuItemTypeCommon) {
if((type == InputTypePress) || (type == InputTypeRelease)) {
if(item && item->callback) {
item->callback(item->callback_context, item->index, type);
if(item->type == ButtonMenuItemTypeCommon) {
if((type == InputTypePress) || (type == InputTypeRelease)) {
if(item && item->callback) {
item->callback(item->callback_context, item->index, type);
}
}
}
}

View File

@ -147,7 +147,7 @@ static void text_input_backspace_cb(TextInputModel* model) {
static void text_input_view_draw_callback(Canvas* canvas, void* _model) {
TextInputModel* model = _model;
uint8_t text_length = strlen(model->text_buffer);
uint8_t text_length = model->text_buffer ? strlen(model->text_buffer) : 0;
uint8_t needed_string_width = canvas_width(canvas) - 8;
uint8_t start_pos = 4;

View File

@ -1,7 +1,7 @@
#pragma once
// #include <gui/view.h>
#include <m-string.h>
#include <core/common_defines.h>
#ifdef __cplusplus
extern "C" {

View File

@ -353,7 +353,7 @@ int32_t ibutton_app(void* p) {
bool key_loaded = false;
bool rpc_mode = false;
if(p) {
if(p && strlen(p)) {
uint32_t rpc_ctx = 0;
if(sscanf(p, "RPC %lX", &rpc_ctx) == 1) {
FURI_LOG_D(TAG, "Running in RPC mode");

View File

@ -405,7 +405,7 @@ int32_t infrared_app(void* p) {
bool is_remote_loaded = false;
bool is_rpc_mode = false;
if(p) {
if(p && strlen(p)) {
uint32_t rpc_ctx = 0;
if(sscanf(p, "RPC %lX", &rpc_ctx) == 1) {
infrared->rpc_ctx = (void*)rpc_ctx;

View File

@ -64,7 +64,8 @@ const char* input_get_type_name(InputType type) {
return "Unknown";
}
int32_t input_srv() {
int32_t input_srv(void* p) {
UNUSED(p);
input = malloc(sizeof(Input));
input->thread_id = furi_thread_get_current_id();
input->event_pubsub = furi_pubsub_alloc();

View File

@ -74,7 +74,7 @@ void LfRfidApp::run(void* _args) {
make_app_folder();
if(strlen(args)) {
if(args && strlen(args)) {
uint32_t rpc_ctx_ptr = 0;
if(sscanf(args, "RPC %lX", &rpc_ctx_ptr) == 1) {
rpc_ctx = (RpcAppSystem*)rpc_ctx_ptr;

View File

@ -300,7 +300,7 @@ int32_t music_player_app(void* p) {
string_init(file_path);
do {
if(p) {
if(p && strlen(p)) {
string_cat_str(file_path, p);
} else {
string_set_str(file_path, MUSIC_PLAYER_APP_PATH_FOLDER);

View File

@ -238,7 +238,7 @@ int32_t nfc_app(void* p) {
char* args = p;
// Check argument and run corresponding scene
if((*args != '\0')) {
if(args && strlen(args)) {
nfc_device_set_loading_callback(nfc->dev, nfc_show_loading_popup, nfc);
uint32_t rpc_ctx = 0;
if(sscanf(p, "RPC %lX", &rpc_ctx) == 1) {

View File

@ -200,7 +200,7 @@ static void power_check_battery_level_change(Power* power) {
}
int32_t power_srv(void* p) {
(void)p;
UNUSED(p);
Power* power = power_alloc();
power_update_info(power);
furi_record_create(RECORD_POWER, power);

View File

@ -76,7 +76,7 @@ void power_settings_app_free(PowerSettingsApp* app) {
int32_t power_settings_app(void* p) {
uint32_t first_scene = PowerSettingsAppSceneStart;
if(p && !strcmp(p, "off")) {
if(p && strlen(p) && !strcmp(p, "off")) {
first_scene = PowerSettingsAppScenePowerOff;
}
PowerSettingsApp* app = power_settings_app_alloc(first_scene);

View File

@ -78,6 +78,8 @@ static void rpc_system_system_device_info_callback(
furi_assert(value);
RpcSystemContext* ctx = context;
furi_assert(key);
furi_assert(value);
char* str_key = strdup(key);
char* str_value = strdup(value);
@ -232,6 +234,8 @@ static void rpc_system_system_power_info_callback(
furi_assert(value);
RpcSystemContext* ctx = context;
furi_assert(key);
furi_assert(value);
char* str_key = strdup(key);
char* str_value = strdup(value);

View File

@ -59,8 +59,8 @@ void subghz_scene_save_name_on_enter(void* context) {
MAX_TEXT_INPUT_LEN, // buffer size
dev_name_empty);
ValidatorIsFile* validator_is_file = validator_is_file_alloc_init(
string_get_cstr(subghz->file_path), SUBGHZ_APP_EXTENSION, NULL);
ValidatorIsFile* validator_is_file =
validator_is_file_alloc_init(string_get_cstr(subghz->file_path), SUBGHZ_APP_EXTENSION, "");
text_input_set_validator(text_input, validator_is_file_callback, validator_is_file);
string_clear(file_name);

View File

@ -320,7 +320,7 @@ int32_t subghz_app(void* p) {
subghz_environment_load_keystore(
subghz->txrx->environment, EXT_PATH("subghz/assets/keeloq_mfcodes_user"));
// Check argument and run corresponding scene
if(p) {
if(p && strlen(p)) {
uint32_t rpc_ctx = 0;
if(sscanf(p, "RPC %lX", &rpc_ctx) == 1) {
subghz->rpc_ctx = (void*)rpc_ctx;

View File

@ -420,10 +420,12 @@ static void
mu_check(result_msg_file->size == expected_msg_file->size);
mu_check(result_msg_file->type == expected_msg_file->type);
mu_check(!result_msg_file->data == !expected_msg_file->data);
mu_check(result_msg_file->data->size == expected_msg_file->data->size);
for(int i = 0; i < result_msg_file->data->size; ++i) {
mu_check(result_msg_file->data->bytes[i] == expected_msg_file->data->bytes[i]);
if(result_msg_file->data && result_msg_file->type != PB_Storage_File_FileType_DIR) {
mu_check(!result_msg_file->data == !expected_msg_file->data); // Zlo: WTF???
mu_check(result_msg_file->data->size == expected_msg_file->data->size);
for(int i = 0; i < result_msg_file->data->size; ++i) {
mu_check(result_msg_file->data->bytes[i] == expected_msg_file->data->bytes[i]);
}
}
}
@ -1346,8 +1348,7 @@ static void test_rpc_storage_rename_run(
}
MU_TEST(test_storage_rename) {
test_rpc_storage_rename_run(
NULL, NULL, ++command_id, PB_CommandStatus_ERROR_STORAGE_INVALID_NAME);
test_rpc_storage_rename_run("", "", ++command_id, PB_CommandStatus_ERROR_STORAGE_INVALID_NAME);
furi_check(!test_is_exists(TEST_DIR "empty.txt"));
test_create_file(TEST_DIR "empty.txt", 0);

View File

@ -34,7 +34,7 @@ static void
Updater* updater_alloc(const char* arg) {
Updater* updater = malloc(sizeof(Updater));
if(arg) {
if(arg && strlen(arg)) {
string_init_set_str(updater->startup_arg, arg);
string_replace_str(updater->startup_arg, ANY_PATH(""), EXT_PATH(""));
} else {

View File

@ -32,7 +32,7 @@ extern uint32_t SystemCoreClock;
#define configUSE_16_BIT_TICKS 0
#define configUSE_MUTEXES 1
#define configQUEUE_REGISTRY_SIZE 0
#define configCHECK_FOR_STACK_OVERFLOW 2
#define configCHECK_FOR_STACK_OVERFLOW 0
#define configUSE_RECURSIVE_MUTEXES 1
#define configUSE_COUNTING_SEMAPHORES 1
#define configENABLE_BACKWARD_COMPATIBILITY 0
@ -145,3 +145,7 @@ standard names. */
#define USE_CUSTOM_SYSTICK_HANDLER_IMPLEMENTATION 1
#define configOVERRIDE_DEFAULT_TICK_CONFIGURATION \
1 /* required only for Keil but does not hurt otherwise */
#define traceTASK_SWITCHED_IN() \
extern void furi_hal_mpu_set_stack_protection(uint32_t* stack); \
furi_hal_mpu_set_stack_protection((uint32_t*)pxCurrentTCB->pxStack)

View File

@ -1,4 +1,5 @@
#include <furi_hal.h>
#include <furi_hal_mpu.h>
#include <stm32wbxx_ll_cortex.h>
@ -35,6 +36,7 @@ void furi_hal_deinit_early() {
}
void furi_hal_init() {
furi_hal_mpu_init();
furi_hal_clock_init();
furi_hal_console_init();
furi_hal_rtc_init();
@ -80,17 +82,6 @@ void furi_hal_init() {
// FatFS driver initialization
MX_FATFS_Init();
FURI_LOG_I(TAG, "FATFS OK");
// 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);
}
void furi_hal_switch(void* address) {

View File

@ -6,6 +6,7 @@
#include <stm32wbxx.h>
#include <stm32wbxx_ll_tim.h>
#include <stm32wbxx_ll_rcc.h>
#include <stm32wbxx_ll_cortex.h>
#define TAG "FuriHalInterrupt"
@ -95,6 +96,10 @@ void furi_hal_interrupt_init() {
LL_SYSCFG_DisableIT_FPU_IDC();
LL_SYSCFG_DisableIT_FPU_IXC();
LL_HANDLER_EnableFault(LL_HANDLER_FAULT_USG);
LL_HANDLER_EnableFault(LL_HANDLER_FAULT_BUS);
LL_HANDLER_EnableFault(LL_HANDLER_FAULT_MEM);
FURI_LOG_I(TAG, "Init OK");
}
@ -241,6 +246,20 @@ void HardFault_Handler() {
}
void MemManage_Handler() {
if(FURI_BIT(SCB->CFSR, SCB_CFSR_MMARVALID_Pos)) {
uint32_t memfault_address = SCB->MMFAR;
if(memfault_address < (1024 * 1024)) {
// from 0x00 to 1MB, see FuriHalMpuRegionNULL
furi_crash("NULL pointer dereference");
} else {
// write or read of MPU region 1 (FuriHalMpuRegionStack)
furi_crash("MPU fault, possibly stack overflow");
}
} else if(FURI_BIT(SCB->CFSR, SCB_CFSR_MSTKERR_Pos)) {
// push to stack on MPU region 1 (FuriHalMpuRegionStack)
furi_crash("MemManage fault, possibly stack overflow");
}
furi_crash("MemManage");
}

View File

@ -0,0 +1,66 @@
#include <furi_hal_mpu.h>
#include <stm32wbxx_ll_cortex.h>
#define FURI_HAL_MPU_ATTRIBUTES \
(LL_MPU_ACCESS_BUFFERABLE | LL_MPU_ACCESS_CACHEABLE | LL_MPU_ACCESS_SHAREABLE | \
LL_MPU_TEX_LEVEL1 | LL_MPU_INSTRUCTION_ACCESS_ENABLE)
#define FURI_HAL_MPU_STACK_PROTECT_REGION FuriHalMPURegionSize32B
void furi_hal_mpu_init() {
furi_hal_mpu_enable();
// NULL pointer dereference protection
furi_hal_mpu_protect_no_access(FuriHalMpuRegionNULL, 0x00, FuriHalMPURegionSize1MB);
}
void furi_hal_mpu_enable() {
LL_MPU_Enable(LL_MPU_CTRL_PRIVILEGED_DEFAULT);
}
void furi_hal_mpu_disable() {
LL_MPU_Disable();
}
void furi_hal_mpu_protect_no_access(
FuriHalMpuRegion region,
uint32_t address,
FuriHalMPURegionSize size) {
uint32_t size_ll = size;
size_ll = size_ll << MPU_RASR_SIZE_Pos;
furi_hal_mpu_disable();
LL_MPU_ConfigRegion(
region, 0x00, address, FURI_HAL_MPU_ATTRIBUTES | LL_MPU_REGION_NO_ACCESS | size_ll);
furi_hal_mpu_enable();
}
void furi_hal_mpu_protect_read_only(
FuriHalMpuRegion region,
uint32_t address,
FuriHalMPURegionSize size) {
uint32_t size_ll = size;
size_ll = size_ll << MPU_RASR_SIZE_Pos;
furi_hal_mpu_disable();
LL_MPU_ConfigRegion(
region, 0x00, address, FURI_HAL_MPU_ATTRIBUTES | LL_MPU_REGION_PRIV_RO_URO | size_ll);
furi_hal_mpu_enable();
}
void furi_hal_mpu_protect_disable(FuriHalMpuRegion region) {
furi_hal_mpu_disable();
LL_MPU_DisableRegion(region);
furi_hal_mpu_enable();
}
void furi_hal_mpu_set_stack_protection(uint32_t* stack) {
// Protection area address must be aligned to region size
uint32_t stack_ptr = (uint32_t)stack;
uint32_t mask = ((1 << (FURI_HAL_MPU_STACK_PROTECT_REGION + 2)) - 1);
stack_ptr &= ~mask;
if(stack_ptr < (uint32_t)stack) stack_ptr += (mask + 1);
furi_hal_mpu_protect_read_only(
FuriHalMpuRegionStack, stack_ptr, FURI_HAL_MPU_STACK_PROTECT_REGION);
}

View File

@ -0,0 +1,86 @@
/**
* @file furi_hal_light.h
* Light control HAL API
*/
#pragma once
#include <stdbool.h>
#include <stdint.h>
#ifdef __cplusplus
extern "C" {
#endif
typedef enum {
FuriHalMpuRegionNULL = 0x00, // region 0 used to protect null pointer dereference
FuriHalMpuRegionStack = 0x01, // region 1 used to protect stack
FuriHalMpuRegion2 = 0x02,
FuriHalMpuRegion3 = 0x03,
FuriHalMpuRegion4 = 0x04,
FuriHalMpuRegion5 = 0x05,
FuriHalMpuRegion6 = 0x06,
FuriHalMpuRegion7 = 0x07,
} FuriHalMpuRegion;
typedef enum {
FuriHalMPURegionSize32B = 0x04U,
FuriHalMPURegionSize64B = 0x05U,
FuriHalMPURegionSize128B = 0x06U,
FuriHalMPURegionSize256B = 0x07U,
FuriHalMPURegionSize512B = 0x08U,
FuriHalMPURegionSize1KB = 0x09U,
FuriHalMPURegionSize2KB = 0x0AU,
FuriHalMPURegionSize4KB = 0x0BU,
FuriHalMPURegionSize8KB = 0x0CU,
FuriHalMPURegionSize16KB = 0x0DU,
FuriHalMPURegionSize32KB = 0x0EU,
FuriHalMPURegionSize64KB = 0x0FU,
FuriHalMPURegionSize128KB = 0x10U,
FuriHalMPURegionSize256KB = 0x11U,
FuriHalMPURegionSize512KB = 0x12U,
FuriHalMPURegionSize1MB = 0x13U,
FuriHalMPURegionSize2MB = 0x14U,
FuriHalMPURegionSize4MB = 0x15U,
FuriHalMPURegionSize8MB = 0x16U,
FuriHalMPURegionSize16MB = 0x17U,
FuriHalMPURegionSize32MB = 0x18U,
FuriHalMPURegionSize64MB = 0x19U,
FuriHalMPURegionSize128MB = 0x1AU,
FuriHalMPURegionSize256MB = 0x1BU,
FuriHalMPURegionSize512MB = 0x1CU,
FuriHalMPURegionSize1GB = 0x1DU,
FuriHalMPURegionSize2GB = 0x1EU,
FuriHalMPURegionSize4GB = 0x1FU,
} FuriHalMPURegionSize;
/**
* @brief Initialize memory protection unit
*/
void furi_hal_mpu_init();
/**
* @brief Enable memory protection unit
*/
void furi_hal_mpu_enable();
/**
* @brief Disable memory protection unit
*/
void furi_hal_mpu_disable();
void furi_hal_mpu_protect_no_access(
FuriHalMpuRegion region,
uint32_t address,
FuriHalMPURegionSize size);
void furi_hal_mpu_protect_read_only(
FuriHalMpuRegion region,
uint32_t address,
FuriHalMPURegionSize size);
void furi_hal_mpu_protect_disable(FuriHalMpuRegion region);
#ifdef __cplusplus
}
#endif

View File

@ -36,10 +36,8 @@ void* calloc(size_t count, size_t size) {
}
char* strdup(const char* s) {
const char* s_null = s;
if(s_null == NULL) {
return NULL;
}
// arg s marked as non-null, so we need hack to check for NULL
furi_check(((uint32_t)s << 2) != 0);
size_t siz = strlen(s) + 1;
char* y = pvPortMalloc(siz);

View File

@ -88,7 +88,7 @@ void furi_thread_set_name(FuriThread* thread, const char* name) {
furi_assert(thread);
furi_assert(thread->state == FuriThreadStateStopped);
if(thread->name) free((void*)thread->name);
thread->name = strdup(name);
thread->name = name ? strdup(name) : NULL;
}
void furi_thread_set_stack_size(FuriThread* thread, size_t stack_size) {