From 3ec36e016082186da7ee43fe99e6166582c2e3b2 Mon Sep 17 00:00:00 2001 From: Daisuke Nojiri Date: Wed, 11 Dec 2013 11:39:55 -0800 Subject: Protect inactive EC image from code execution This change configures MPU to prevent instruction fetch from the flash image that is not running at the time system_disable_jump is called. Violating the protection causes instruction access violation, then the EC reboots. RO image protection is tested as follows: ... [6.255696 MPU type: 00000800] [6.255874 RAM locked. Exclusion 20005680-200056a0] [6.256168 RO image locked] ... > sysjump 0 Jumping to 0x00000000 === PROCESS EXCEPTION: 03 ====== xPSR: 60000000 === r0 :00000000 r1 :2000541c r2 :00001388 r3 :20007fe8 r4 :200032f0 r5 :00000000 r6 :20002b70 r7 :20002df4 r8 :0002d308 r9 :20002df4 r10:00000000 r11:00000000 r12:00000002 sp :20002358 lr :0002a1a7 pc :00000000 Instruction access violation, Forced hard fault mmfs = 1, shcsr = 70000, hfsr = 40000000, dfsr = 0 =========== Process Stack Contents =========== 200023c0: 00000098 00000000 00000000 0002a785 200023d0: 00000002 20002dfd 00000007 20002b70 200023e0: 00000002 00025777 00000000 20002dfd 200023f0: 20002df4 20002dfc 00000000 00000000 Rebooting... Memory management fault status register has bit0 set, indicating there was an instruction fetch volation. FYI, RAM protection is still working: > sysjump 0x20000000 Jumping to 0x20000000 === PROCESS EXCEPTION: 03 ====== xPSR: 60000000 === r0 :00000000 r1 :2000541c r2 :00001388 r3 :20007fe8 r4 :200032f0 r5 :20000000 r6 :20002b70 r7 :20002df4 r8 :0002d308 r9 :20002df4 r10:00000000 r11:00000000 r12:00000002 sp :20002358 lr :0002a1a7 pc :20000000 Instruction access violation, Forced hard fault mmfs = 1, shcsr = 70000, hfsr = 40000000, dfsr = 0 =========== Process Stack Contents =========== 200023c0: 00000098 00000000 20000000 0002a785 200023d0: 00000002 20002e06 00000007 20002b70 200023e0: 00000002 00025777 00000000 20002e06 200023f0: 20002df4 20002dfc 00000000 00000000 Rebooting... TEST=Booted Peppy. Tested lid close & open. Ran Flashrom from userspace to update main firmware then software-synched an EC image. BUG=chrome-os-partner:16904 BRANCH=none Signed-off-by: Daisuke Nojiri Change-Id: Id4f84d24325566a9f648194166bde0d94d1124dc Reviewed-on: https://chromium-review.googlesource.com/169050 Reviewed-by: Bill Richardson Reviewed-by: Randall Spangler Commit-Queue: Daisuke Nojiri Tested-by: Daisuke Nojiri --- core/cortex-m/include/mpu.h | 52 +++++++++++-------------- core/cortex-m/mpu.c | 95 +++++++++++++++++++++++++++++++++------------ 2 files changed, 93 insertions(+), 54 deletions(-) (limited to 'core') diff --git a/core/cortex-m/include/mpu.h b/core/cortex-m/include/mpu.h index c3a632aff7..c0c595a367 100644 --- a/core/cortex-m/include/mpu.h +++ b/core/cortex-m/include/mpu.h @@ -20,39 +20,27 @@ #define MPU_CTRL_PRIVDEFEN (1 << 2) #define MPU_CTRL_HFNMIENA (1 << 1) #define MPU_CTRL_ENABLE (1 << 0) -#define MPU_ATTR_NX (1 << 12) -#define MPU_ATTR_NOACCESS (0 << 8) -#define MPU_ATTR_FULLACCESS (3 << 8) -/* Suggested in table 3-6 of Stellaris LM4F232H5QC Datasheet and table 38 of - * STM32F10xxx Cortex-M3 programming manual for internal sram. */ -#define MPU_ATTR_INTERNALSRAM 6 -/** - * Configure a region - * - * region: Number of the region to update - * addr: Base address of the region - * size: Size of the region in bytes - * attr: Attribute of the region. Current value will be overwritten if enable - * is set. - * enable: Enables the region if non zero. Otherwise, disables the region. - * - * Returns EC_SUCCESS on success or EC_ERROR_INVAL if a parameter is invalid. +/* + * XN (execute never) bit. It's bit 12 if accessed by halfword. + * 0: XN off + * 1: XN on */ -int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, - uint16_t attr, uint8_t enable); +#define MPU_ATTR_XN (1 << 12) -/** - * Set a region non-executable. - * - * region: number of the region - * addr: base address of the region - * size: size of the region in bytes - */ -int mpu_nx_region(uint8_t region, uint32_t addr, uint32_t size); +/* AP bit. See table 3-5 of Stellaris LM4F232H5QC datasheet for details */ +#define MPU_ATTR_NO_NO (0 << 8) /* previleged no access, unprev no access */ +#define MPU_ATTR_RW_RW (3 << 8) /* previleged ReadWrite, unprev ReadWrite */ +#define MPU_ATTR_RO_NO (5 << 8) /* previleged Read-only, unprev no access */ + +/* Suggested value for TEX S/C/B bit. See table 3-6 of Stellaris LM4F232H5QC + * datasheet and table 38 of STM32F10xxx Cortex-M3 programming manual. */ +#define MPU_ATTR_INTERNAL_SRAM 6 /* for Internal SRAM */ +#define MPU_ATTR_FLASH_MEMORY 2 /* for flash memory */ /** - * Enable MPU */ + * Enable MPU + */ void mpu_enable(void); /** @@ -70,10 +58,16 @@ extern char __iram_text_start; extern char __iram_text_end; /** - * Lock down RAM + * Protect RAM from code execution */ int mpu_protect_ram(void); +/** + * Protect flash memory from code execution + */ +int mpu_lock_ro_flash(void); +int mpu_lock_rw_flash(void); + /** * Initialize MPU. * It disables all regions if MPU is implemented. Otherwise, returns diff --git a/core/cortex-m/mpu.c b/core/cortex-m/mpu.c index 23d434d7e6..b8f2ca8def 100644 --- a/core/cortex-m/mpu.c +++ b/core/cortex-m/mpu.c @@ -11,20 +11,28 @@ #include "task.h" #include "util.h" +/* Region assignment. 7 as the highest, a higher index has a higher priority. + * For example, using 7 for .iram.text allows us to mark entire RAM XN except + * .iram.text, which is used for hibernation. */ +enum mpu_region { + REGION_IRAM = 0, /* For internal RAM */ + REGION_FLASH_MEMORY = 1, /* For flash memory */ + REGION_IRAM_TEXT = 7 /* For *.(iram.text) */ +}; + /** * Update a memory region. * - * region: number of the region to update + * region: index of the region to update * addr: base address of the region * size_bit: size of the region in power of two. - * attr: attribute of the region. Current value will be overwritten if enable - * is set. + * attr: attribute bits. Current value will be overwritten if enable is true. * enable: enables the region if non zero. Otherwise, disables the region. * * Based on 3.1.4.1 'Updating an MPU Region' of Stellaris LM4F232H5QC Datasheet */ static void mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, - uint16_t attr, uint8_t enable) + uint16_t attr, uint8_t enable) { asm volatile("isb; dsb;"); @@ -39,8 +47,19 @@ static void mpu_update_region(uint8_t region, uint32_t addr, uint8_t size_bit, asm volatile("isb; dsb;"); } -int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, - uint16_t attr, uint8_t enable) +/** + * Configure a region + * + * region: index of the region to update + * addr: Base address of the region + * size: Size of the region in bytes + * attr: Attribute bits. Current value will be overwritten if enable is set. + * enable: Enables the region if non zero. Otherwise, disables the region. + * + * Returns EC_SUCCESS on success or EC_ERROR_INVAL if a parameter is invalid. + */ +static int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, + uint16_t attr, uint8_t enable) { int size_bit = 0; @@ -60,11 +79,34 @@ int mpu_config_region(uint8_t region, uint32_t addr, uint32_t size, return EC_SUCCESS; } -int mpu_nx_region(uint8_t region, uint32_t addr, uint32_t size) +/** + * Set a region non-executable and read-write. + * + * region: index of the region + * addr: base address of the region + * size: size of the region in bytes + * texscb: TEX and SCB bit field + */ +static int mpu_lock_region(uint8_t region, uint32_t addr, uint32_t size, + uint8_t texscb) { - return mpu_config_region( - region, addr, size, - MPU_ATTR_NX | MPU_ATTR_FULLACCESS | MPU_ATTR_INTERNALSRAM, 1); + return mpu_config_region(region, addr, size, + MPU_ATTR_XN | MPU_ATTR_RW_RW | texscb, 1); +} + +/** + * Set a region executable and read-write. + * + * region: index of the region + * addr: base address of the region + * size: size of the region in bytes + * texscb: TEX and SCB bit field + */ +static int mpu_unlock_region(uint8_t region, uint32_t addr, uint32_t size, + uint8_t texscb) +{ + return mpu_config_region(region, addr, size, + MPU_ATTR_RW_RW | texscb, 1); } void mpu_enable(void) @@ -82,27 +124,32 @@ uint32_t mpu_get_type(void) return MPU_TYPE; } -/** - * Prevent code from running on RAM. - * We need to allow execution from iram.text. Using higher region# allows us to - * do 'whitelisting' (lock down all then create exceptions). - */ int mpu_protect_ram(void) { int ret; - - ret = mpu_nx_region(0, CONFIG_RAM_BASE, CONFIG_RAM_SIZE); + ret = mpu_lock_region(REGION_IRAM, CONFIG_RAM_BASE, + CONFIG_RAM_SIZE, MPU_ATTR_INTERNAL_SRAM); if (ret != EC_SUCCESS) return ret; - - ret = mpu_config_region( - 7, (uint32_t)&__iram_text_start, + ret = mpu_unlock_region( + REGION_IRAM_TEXT, (uint32_t)&__iram_text_start, (uint32_t)(&__iram_text_end - &__iram_text_start), - MPU_ATTR_FULLACCESS | MPU_ATTR_INTERNALSRAM, 1); - + MPU_ATTR_INTERNAL_SRAM); return ret; } +int mpu_lock_ro_flash(void) +{ + return mpu_lock_region(REGION_FLASH_MEMORY, CONFIG_FW_RO_OFF, + CONFIG_FW_IMAGE_SIZE, MPU_ATTR_FLASH_MEMORY); +} + +int mpu_lock_rw_flash(void) +{ + return mpu_lock_region(REGION_FLASH_MEMORY, CONFIG_FW_RW_OFF, + CONFIG_FW_IMAGE_SIZE, MPU_ATTR_FLASH_MEMORY); +} + int mpu_pre_init(void) { int i; @@ -112,9 +159,7 @@ int mpu_pre_init(void) mpu_disable(); for (i = 0; i < 8; ++i) { - mpu_config_region( - i, CONFIG_RAM_BASE, CONFIG_RAM_SIZE, - MPU_ATTR_FULLACCESS | MPU_ATTR_INTERNALSRAM, 0); + mpu_config_region(i, CONFIG_RAM_BASE, CONFIG_RAM_SIZE, 0, 0); } return EC_SUCCESS; -- cgit v1.2.1