diff --git a/arch/arm/core/cortex_m/mpu/arm_mpu.c b/arch/arm/core/cortex_m/mpu/arm_mpu.c index 1568fcc1d2cad..dda44660ef742 100644 --- a/arch/arm/core/cortex_m/mpu/arm_mpu.c +++ b/arch/arm/core/cortex_m/mpu/arm_mpu.c @@ -108,10 +108,12 @@ void arm_core_mpu_configure(u8_t type, u32_t base, u32_t size) { SYS_LOG_DBG("Region info: 0x%x 0x%x", base, size); /* - * The new MPU regions are are allocated per type after the statically - * configured regions. + * The new MPU regions are allocated per type after the statically + * configured regions. The type is one-indexed rather than + * zero-indexed, therefore we need to subtract by one to get the region + * index. */ - u32_t region_index = mpu_config.num_regions + type; + u32_t region_index = mpu_config.num_regions + type - 1; u32_t region_attr = _get_region_attr_by_type(type, size); /* ARM MPU supports up to 16 Regions */ diff --git a/arch/arm/core/cortex_m/mpu/nxp_mpu.c b/arch/arm/core/cortex_m/mpu/nxp_mpu.c index d9c6076b32e44..54a638abc81c6 100644 --- a/arch/arm/core/cortex_m/mpu/nxp_mpu.c +++ b/arch/arm/core/cortex_m/mpu/nxp_mpu.c @@ -44,10 +44,33 @@ static inline u8_t _get_num_regions(void) static void _region_init(u32_t index, u32_t region_base, u32_t region_end, u32_t region_attr) { - SYSMPU->WORD[index][0] = region_base; - SYSMPU->WORD[index][1] = region_end; - SYSMPU->WORD[index][2] = region_attr; - SYSMPU->WORD[index][3] = SYSMPU_WORD_VLD_MASK; + if (index == 0) { + /* The MPU does not allow writes from the core to affect the + * RGD0 start or end addresses nor the permissions associated + * with the debugger; it can only write the permission fields + * associated with the other masters. These protections + * guarantee that the debugger always has access to the entire + * address space. + */ + __ASSERT(region_base == SYSMPU->WORD[index][0], + "Region %d base address got 0x%08x expected 0x%08x", + index, region_base, SYSMPU->WORD[index][0]); + + __ASSERT(region_end == SYSMPU->WORD[index][1], + "Region %d end address got 0x%08x expected 0x%08x", + index, region_end, SYSMPU->WORD[index][1]); + + /* Changes to the RGD0_WORD2 alterable fields should be done + * via a write to RGDAAC0. + */ + SYSMPU->RGDAAC[index] = region_attr; + + } else { + SYSMPU->WORD[index][0] = region_base; + SYSMPU->WORD[index][1] = region_end; + SYSMPU->WORD[index][2] = region_attr; + SYSMPU->WORD[index][3] = SYSMPU_WORD_VLD_MASK; + } SYS_LOG_DBG("[%d] 0x%08x 0x%08x 0x%08x 0x%08x", index, SYSMPU->WORD[index][0], @@ -97,10 +120,12 @@ void arm_core_mpu_configure(u8_t type, u32_t base, u32_t size) { SYS_LOG_DBG("Region info: 0x%x 0x%x", base, size); /* - * The new MPU regions are are allocated per type after the statically - * configured regions. + * The new MPU regions are allocated per type after the statically + * configured regions. The type is one-indexed rather than + * zero-indexed, therefore we need to subtract by one to get the region + * index. */ - u32_t region_index = mpu_config.num_regions + type; + u32_t region_index = mpu_config.num_regions + type - 1; u32_t region_attr = _get_region_attr_by_type(type); u32_t last_region = _get_num_regions() - 1; @@ -130,7 +155,7 @@ void arm_core_mpu_configure(u8_t type, u32_t base, u32_t size) * structure is mapped on the mpu_config.sram_region + 1 region of * the MPU. */ - _region_init(mpu_config.sram_region + 1, + _region_init(mpu_config.sram_region, mpu_config.mpu_regions[mpu_config.sram_region].base, ENDADDR_ROUND(base), mpu_config.mpu_regions[mpu_config.sram_region].attr); @@ -182,22 +207,9 @@ static void _nxp_mpu_config(void) /* MPU Configuration */ - /* Disable Region 0 */ - SYSMPU->WORD[0][2] = 0; - - SYS_LOG_DBG("[0] 0x%08x 0x%08x 0x%08x 0x%08x", - SYSMPU->WORD[0][0], - SYSMPU->WORD[0][1], - SYSMPU->WORD[0][2], - SYSMPU->WORD[0][3]); - - /* - * Configure regions: - * r_index starts from 0 but is passed to region_init as r_index + 1, - * region 0 is not configurable - */ + /* Configure regions */ for (r_index = 0; r_index < mpu_config.num_regions; r_index++) { - _region_init(r_index + 1, + _region_init(r_index, mpu_config.mpu_regions[r_index].base, mpu_config.mpu_regions[r_index].end, mpu_config.mpu_regions[r_index].attr); diff --git a/arch/arm/soc/nxp_kinetis/k6x/nxp_mpu_regions.c b/arch/arm/soc/nxp_kinetis/k6x/nxp_mpu_regions.c index 8a9af1fad37b8..16245d767700b 100644 --- a/arch/arm/soc/nxp_kinetis/k6x/nxp_mpu_regions.c +++ b/arch/arm/soc/nxp_kinetis/k6x/nxp_mpu_regions.c @@ -12,11 +12,16 @@ static struct nxp_mpu_region mpu_regions[] = { /* Region 0 */ + MPU_REGION_ENTRY("DEBUGGER_0", + 0, + 0xFFFFFFFF, + REGION_DEBUG_ATTR), + /* Region 1 */ MPU_REGION_ENTRY("FLASH_0", CONFIG_FLASH_BASE_ADDRESS, 0x07FFFFFF, REGION_FLASH_ATTR), - /* Region 1 */ + /* Region 2 */ /* * This region (Flexbus + FlexNVM) is bigger than the FLEXBUS one in * order to save 1 region allocation in the MPU. @@ -25,18 +30,18 @@ static struct nxp_mpu_region mpu_regions[] = { FLEXBUS_BASE_ADDRESS, 0x1BFFFFFF, REGION_IO_ATTR), - /* Region 2 */ + /* Region 3 */ MPU_REGION_ENTRY("RAM_L_0", SRAM_L_BASE_ADDRESS, 0x1FFFFFFF, REGION_RAM_ATTR), - /* Region 3 */ + /* Region 4 */ MPU_REGION_ENTRY("RAM_U_0", CONFIG_SRAM_BASE_ADDRESS, (CONFIG_SRAM_BASE_ADDRESS + (CONFIG_SRAM_SIZE * 1024) - 1), REGION_RAM_ATTR), - /* Region 4 */ + /* Region 5 */ MPU_REGION_ENTRY("DEVICE_0", DEVICE_S_BASE_ADDRESS, 0xFFFFFFFF, @@ -46,5 +51,5 @@ static struct nxp_mpu_region mpu_regions[] = { struct nxp_mpu_config mpu_config = { .num_regions = ARRAY_SIZE(mpu_regions), .mpu_regions = mpu_regions, - .sram_region = 3, + .sram_region = 4, }; diff --git a/include/arch/arm/cortex_m/mpu/nxp_mpu.h b/include/arch/arm/cortex_m/mpu/nxp_mpu.h index b451d8793856a..a4cb3895f5bcc 100644 --- a/include/arch/arm/cortex_m/mpu/nxp_mpu.h +++ b/include/arch/arm/cortex_m/mpu/nxp_mpu.h @@ -78,6 +78,9 @@ #define REGION_RO_ATTR (MPU_REGION_READ | \ MPU_REGION_SU) +#define REGION_DEBUG_ATTR MPU_REGION_SU + + /* Region definition data structure */ struct nxp_mpu_region { /* Region Base Address */ diff --git a/tests/kernel/protection/Makefile b/tests/kernel/protection/Makefile new file mode 100644 index 0000000000000..783c4018ad3dc --- /dev/null +++ b/tests/kernel/protection/Makefile @@ -0,0 +1,4 @@ +BOARD ?= frdm_k64f +CONF_FILE = prj.conf + +include ${ZEPHYR_BASE}/Makefile.test diff --git a/tests/kernel/protection/README.rst b/tests/kernel/protection/README.rst new file mode 100644 index 0000000000000..06f46f6e5fefa --- /dev/null +++ b/tests/kernel/protection/README.rst @@ -0,0 +1,84 @@ +.. _protection_tests: + +Protection tests +################################# + +Overview +******** +This test case verifies that protection is provided +against the following security issues: + +* Write to read-only data. +* Write to text. +* Execute from data. +* Execute from stack. +* Execute from heap. + +Building and Running +******************** + +This project can be built and executed as follows: + +.. code-block:: console + + $ cd tests/protection + $ make BOARD= + +Connect the board to your host computer using the USB port. +Flash the generated zephyr.bin on the board. +Reset the board. + +Sample Output +============= + +.. code-block:: console + + ***** BOOTING ZEPHYR OS v1.8.99 - BUILD: Jun 19 2017 12:44:27 ***** + Running test suite test_protection + tc_start() - write_ro + trying to write to rodata at 0x00003124 + ***** BUS FAULT ***** + Executing thread ID (thread): 0x200001bc + Faulting instruction address: 0x88c + Imprecise data bus error + Caught system error -- reason 0 + =================================================================== + PASS - write_ro. + tc_start() - write_text + trying to write to text at 0x000006c0 + ***** BUS FAULT ***** + Executing thread ID (thread): 0x200001bc + Faulting instruction address: 0xd60 + Imprecise data bus error + Caught system error -- reason 0 + =================================================================== + PASS - write_text. + tc_start() - exec_data + trying to call code written to 0x2000041d + ***** BUS FAULT ***** + Executing thread ID (thread): 0x200001bc + Faulting instruction address: 0x2000041c + Imprecise data bus error + Caught system error -- reason 0 + =================================================================== + PASS - exec_data. + tc_start() - exec_stack + trying to call code written to 0x20000929 + ***** BUS FAULT ***** + Executing thread ID (thread): 0x200001bc + Faulting instruction address: 0x20000928 + Imprecise data bus error + Caught system error -- reason 0 + =================================================================== + PASS - exec_stack. + tc_start() - exec_heap + trying to call code written to 0x20000455 + ***** BUS FAULT ***** + Executing thread ID (thread): 0x200001bc + Faulting instruction address: 0x20000454 + Imprecise data bus error + Caught system error -- reason 0 + =================================================================== + PASS - exec_heap. + =================================================================== + PROJECT EXECUTION SUCCESSFUL diff --git a/tests/kernel/protection/prj.conf b/tests/kernel/protection/prj.conf new file mode 100644 index 0000000000000..a3b3b01065cf3 --- /dev/null +++ b/tests/kernel/protection/prj.conf @@ -0,0 +1,2 @@ +CONFIG_HEAP_MEM_POOL_SIZE=256 +CONFIG_ZTEST=y diff --git a/tests/kernel/protection/src/Makefile b/tests/kernel/protection/src/Makefile new file mode 100644 index 0000000000000..e1f454f63be84 --- /dev/null +++ b/tests/kernel/protection/src/Makefile @@ -0,0 +1,5 @@ +include $(ZEPHYR_BASE)/tests/Makefile.test + +ccflags-y += -I${ZEPHYR_BASE}/tests/include + +obj-y = targets.o main.o diff --git a/tests/kernel/protection/src/main.c b/tests/kernel/protection/src/main.c new file mode 100644 index 0000000000000..258e0b0246ccb --- /dev/null +++ b/tests/kernel/protection/src/main.c @@ -0,0 +1,170 @@ +/* + * Parts derived from tests/kernel/fatal/src/main.c, which has the + * following copyright and license: + * + * Copyright (c) 2017 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include + +#include "targets.h" + +#define INFO(fmt, ...) printk(fmt, ##__VA_ARGS__) + +/* ARM is a special case, in that k_thread_abort() does indeed return + * instead of calling _Swap() directly. The PendSV exception is queued + * and immediately fires upon completing the exception path; the faulting + * thread is never run again. + */ +#ifndef CONFIG_ARM +FUNC_NORETURN +#endif +void _SysFatalErrorHandler(unsigned int reason, const NANO_ESF *pEsf) +{ + INFO("Caught system error -- reason %d\n", reason); + ztest_test_pass(); +#ifndef CONFIG_ARM + CODE_UNREACHABLE; +#endif +} + +#ifdef CONFIG_CPU_CORTEX_M +#include +/* Must clear LSB of function address to access as data. */ +#define FUNC_TO_PTR(x) (void *)((uintptr_t)(x) & ~0x1) +/* Must set LSB of function address to call in Thumb mode. */ +#define PTR_TO_FUNC(x) (int (*)(int))((uintptr_t)(x) | 0x1) +/* Flush preceding data writes and instruction fetches. */ +#define DO_BARRIERS() do { __DSB(); __ISB(); } while (0) +#else +#define FUNC_TO_PTR(x) (void *)(x) +#define PTR_TO_FUNC(x) (int (*)(int))(x) +#define DO_BARRIERS() do { } while (0) +#endif + +static int __attribute__((noinline)) add_one(int i) +{ + return (i + 1); +} + +static void execute_from_buffer(u8_t *dst) +{ + void *src = FUNC_TO_PTR(add_one); + int (*func)(int i) = PTR_TO_FUNC(dst); + int i = 1; + + /* Copy add_one() code to destination buffer. */ + memcpy(dst, src, BUF_SIZE); + DO_BARRIERS(); + + /* + * Try executing from buffer we just filled. + * Optimally, this triggers a fault. + * If not, we check to see if the function + * returned the expected result as confirmation + * that we truly executed the code we wrote. + */ + INFO("trying to call code written to %p\n", func); + i = func(i); + INFO("returned from code at %p\n", func); + if (i == 2) { + INFO("Execute from target buffer succeeded!\n"); + } else { + INFO("Did not get expected return value!\n"); + } +} + +static void write_ro(void) +{ + u32_t *ptr = (u32_t *)&rodata_var; + + /* + * Try writing to rodata. Optimally, this triggers a fault. + * If not, we check to see if the rodata value actually changed. + */ + INFO("trying to write to rodata at %p\n", ptr); + *ptr = ~RODATA_VALUE; + + DO_BARRIERS(); + + if (*ptr == RODATA_VALUE) { + INFO("rodata value still the same\n"); + } else if (*ptr == ~RODATA_VALUE) { + INFO("rodata modified!\n"); + } else { + INFO("something went wrong!\n"); + } + + zassert_unreachable("Write to rodata did not fault"); +} + +static void write_text(void) +{ + void *src = FUNC_TO_PTR(add_one); + void *dst = FUNC_TO_PTR(overwrite_target); + int i = 1; + + /* + * Try writing to a function in the text section. + * Optimally, this triggers a fault. + * If not, we try calling the function after overwriting + * to see if it returns the expected result as + * confirmation that we truly executed the code we wrote. + */ + INFO("trying to write to text at %p\n", dst); + memcpy(dst, src, BUF_SIZE); + DO_BARRIERS(); + i = overwrite_target(i); + if (i == 2) { + INFO("Overwrite of text succeeded!\n"); + } else { + INFO("Did not get expected return value!\n"); + } + + zassert_unreachable("Write to text did not fault"); +} + +static void exec_data(void) +{ + execute_from_buffer(data_buf); + zassert_unreachable("Execute from data did not fault"); +} + +static void exec_stack(void) +{ + u8_t stack_buf[BUF_SIZE] __aligned(sizeof(int)); + + execute_from_buffer(stack_buf); + zassert_unreachable("Execute from stack did not fault"); +} + +#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) +static void exec_heap(void) +{ + u8_t *heap_buf = k_malloc(BUF_SIZE); + + execute_from_buffer(heap_buf); + k_free(heap_buf); + zassert_unreachable("Execute from heap did not fault"); +} +#endif + +void test_main(void *unused1, void *unused2, void *unused3) +{ + ztest_test_suite(test_protection, + ztest_unit_test(write_ro), + ztest_unit_test(write_text), + ztest_unit_test(exec_data), + ztest_unit_test(exec_stack) +#if (CONFIG_HEAP_MEM_POOL_SIZE > 0) + , ztest_unit_test(exec_heap) +#endif + ); + ztest_run_test_suite(test_protection); +} diff --git a/tests/kernel/protection/src/targets.c b/tests/kernel/protection/src/targets.c new file mode 100644 index 0000000000000..8aad7d700c0ff --- /dev/null +++ b/tests/kernel/protection/src/targets.c @@ -0,0 +1,14 @@ +#include +#include + +#include "targets.h" + +const u32_t rodata_var = RODATA_VALUE; + +u8_t data_buf[BUF_SIZE] __aligned(sizeof(int)); + +int overwrite_target(int i) +{ + printk("text not modified\n"); + return (i - 1); +} diff --git a/tests/kernel/protection/src/targets.h b/tests/kernel/protection/src/targets.h new file mode 100644 index 0000000000000..71b71c2dcf79b --- /dev/null +++ b/tests/kernel/protection/src/targets.h @@ -0,0 +1,12 @@ +#ifndef _PROT_TEST_TARGETS_H_ +#define _PROT_TEST_TARGETS_H_ + +#define RODATA_VALUE 0xF00FF00F +extern const u32_t rodata_var; + +#define BUF_SIZE 16 +extern u8_t data_buf[BUF_SIZE]; + +extern int overwrite_target(int i); + +#endif diff --git a/tests/kernel/protection/testcase.ini b/tests/kernel/protection/testcase.ini new file mode 100644 index 0000000000000..b9edbaa0a45bc --- /dev/null +++ b/tests/kernel/protection/testcase.ini @@ -0,0 +1,3 @@ +[test] +tags = core security ignore_faults +filter = CONFIG_CPU_HAS_MPU or CONFIG_X86_MMU diff --git a/tests/ztest/include/ztest_test.h b/tests/ztest/include/ztest_test.h index b9e4f976b0966..81f0cd7157dc7 100644 --- a/tests/ztest/include/ztest_test.h +++ b/tests/ztest/include/ztest_test.h @@ -40,6 +40,16 @@ void _ztest_run_test_suite(const char *name, struct unit_test *suite); */ void ztest_test_fail(void); +/** + * @brief Pass the currently running test. + * + * Normally a test passes just by returning without an assertion failure. + * However, if the success case for your test involves a fatal fault, + * you can call this function from _SysFatalErrorHandler to indicate that + * the test passed before aborting the thread. + */ +void ztest_test_pass(void); + /** * @brief Do nothing, successfully. * diff --git a/tests/ztest/src/ztest.c b/tests/ztest/src/ztest.c index 488ecab522df2..f3ddfd37501b9 100644 --- a/tests/ztest/src/ztest.c +++ b/tests/ztest/src/ztest.c @@ -56,6 +56,7 @@ static void run_test_functions(struct unit_test *test) #define FAIL_FAST 0 static jmp_buf test_fail; +static jmp_buf test_pass; static jmp_buf stack_fail; void ztest_test_fail(void) @@ -63,6 +64,11 @@ void ztest_test_fail(void) raise(SIGABRT); } +void ztest_test_pass(void) +{ + longjmp(test_pass, 1); +} + static void handle_signal(int sig) { static const char *const phase_str[] = { @@ -106,6 +112,11 @@ static int run_test(struct unit_test *test) goto out; } + if (setjmp(test_pass)) { + ret = TC_PASS; + goto out; + } + run_test_functions(test); out: ret |= cleanup_test(test); @@ -142,6 +153,13 @@ void ztest_test_fail(void) k_thread_abort(k_current_get()); } +void ztest_test_pass(void) +{ + test_result = 0; + k_sem_give(&test_end_signal); + k_thread_abort(k_current_get()); +} + static void init_testing(void) { k_sem_init(&test_end_signal, 0, 1);