startup: use custom reset_handler

jhoenicke · prusnak · commit 98e617d8740b · 2017-08-16T13:52:01.000+02:00
+ group confidential data in one place
+ zero all SRAM where needed
diff --git a/Makefile b/Makefile
@@ -1,3 +1,4 @@
+OBJS += startup.o
 OBJS += buttons.o
 OBJS += layout.o
 OBJS += oled.o
diff --git a/Makefile.include b/Makefile.include
@@ -7,11 +7,14 @@ LD      = $(PREFIX)gcc
 OBJCOPY = $(PREFIX)objcopy
 OBJDUMP = $(PREFIX)objdump
 AR      = $(PREFIX)ar
+AS      = $(PREFIX)as
 FLASH   = st-flash
 OPENOCD = openocd
 
 OPTFLAGS ?= -O3
 DBGFLAGS ?= -g -DNDEBUG
+CPUFLAGS ?= -mcpu=cortex-m3 -mthumb
+FPUFLAGS ?= -msoft-float
 
 CFLAGS   += $(OPTFLAGS) \
             $(DBGFLAGS) \
@@ -40,10 +43,10 @@ CFLAGS   += $(OPTFLAGS) \
             -ffunction-sections \
             -fdata-sections \
             -fstack-protector-all \
-            -mcpu=cortex-m3 \
-            -mthumb \
-            -msoft-float \
+            $(CPUFLAGS) \
+            $(FPUFLAGS) \
             -DSTM32F2 \
+            -DCONFIDENTIAL='__attribute__((section("confidential")))' \
             -I$(TOOLCHAIN_DIR)/include \
             -I$(TOP_DIR) \
             -I$(TOP_DIR)gen \
@@ -83,9 +86,8 @@ LDFLAGS  += --static \
             -T$(LDSCRIPT) \
             -nostartfiles \
             -Wl,--gc-sections \
-            -mcpu=cortex-m3 \
-            -mthumb \
-            -msoft-float
+            $(CPUFLAGS) \
+            $(FPUFLAGS)
 
 all: $(NAME).bin
 
@@ -128,6 +130,9 @@ $(NAME).list: $(NAME).elf
 $(NAME).elf: $(OBJS) $(LDSCRIPT) $(TOOLCHAIN_DIR)/lib/libopencm3_stm32f2.a $(TOP_DIR)/libtrezor.a
 	$(LD) -o $(NAME).elf $(OBJS) -ltrezor -lopencm3_stm32f2 $(LDFLAGS)
 
+%.o: %.s Makefile
+	$(AS) $(CPUFLAGS) -o $@ $<
+
 %.o: %.c Makefile
 	$(CC) $(CFLAGS) -MMD -o $@ -c $<
 
diff --git a/bootloader/bootloader.c b/bootloader/bootloader.c
@@ -78,6 +78,9 @@ void show_unofficial_warning(const uint8_t *hash)
 
 void __attribute__((noreturn)) load_app(void)
 {
+	// zero out SRAM
+	memset_reg(_ram_start, _ram_end, 0);
+
 	load_vector_table((const vector_table_t *) FLASH_APP_START);
 }
 
diff --git a/bootloader/bootloader.h b/bootloader/bootloader.h
@@ -22,14 +22,14 @@
 
 #define VERSION_MAJOR 1
 #define VERSION_MINOR 3
-#define VERSION_PATCH 2
+#define VERSION_PATCH 3
 
 #define STR(X) #X
 #define VERSTR(X) STR(X)
 
 #define VERSION_MAJOR_CHAR "\x01"
 #define VERSION_MINOR_CHAR "\x03"
-#define VERSION_PATCH_CHAR "\x02"
+#define VERSION_PATCH_CHAR "\x03"
 
 #include <stdbool.h>
 #include "memory.h"
diff --git a/firmware/Makefile b/firmware/Makefile
@@ -1,10 +1,8 @@
-ifeq ($(FASTFLASH),1)
-APPVER = 2.0.0
+APPVER = 1.0.0
 
+ifeq ($(FASTFLASH),1)
 OBJS += fastflash.o
 OBJS += bootloader.o
-else
-APPVER = 1.0.0
 endif
 
 NAME  = trezor
@@ -93,6 +91,7 @@ CFLAGS += -DUSE_ETHEREUM=1
 
 bootloader.o: ../fastflash/bootloader.bin
 	$(OBJCOPY) -I binary -O elf32-littlearm -B arm \
+		--redefine-sym _binary_$(shell echo -n "$<" | tr -c "[:alnum:]" "_")_start=__bootloader_start__ \
 		--redefine-sym _binary_$(shell echo -n "$<" | tr -c "[:alnum:]" "_")_size=__bootloader_size__ \
-		--rename-section .data=.bootloader \
+		--rename-section .data=.rodata \
 		$< $@
diff --git a/firmware/ethereum.c b/firmware/ethereum.c
@@ -37,7 +37,7 @@
 static bool ethereum_signing = false;
 static uint32_t data_total, data_left;
 static EthereumTxRequest msg_tx_request;
-static uint8_t privkey[32];
+static CONFIDENTIAL uint8_t privkey[32];
 static uint8_t chain_id;
 struct SHA3_CTX keccak_ctx;
 
diff --git a/firmware/fastflash.c b/firmware/fastflash.c
@@ -24,16 +24,18 @@
 #include <stdint.h>
 #include <string.h>
 
-extern uint8_t __bootloader_loadaddr__[];
-extern uint8_t __bootloader_runaddr__[];
-extern uint8_t __bootloader_size__[];
-
-void load_bootloader(void)
-{
-	memcpy(__bootloader_runaddr__, __bootloader_loadaddr__, (size_t) __bootloader_size__);
-}
+#define bootloader_vec ((vector_table_t *) 0x20000000)
 
 void __attribute__((noreturn)) run_bootloader(void)
 {
-	load_vector_table((const vector_table_t *) __bootloader_runaddr__);
+	extern uint8_t __bootloader_start__[];
+	extern uint8_t __bootloader_size__[];
+
+	// zero out SRAM
+	memset_reg(_ram_start, _ram_end, 0);
+
+	// copy bootloader
+	memcpy(bootloader_vec, __bootloader_start__, (size_t) __bootloader_size__);
+
+	load_vector_table(bootloader_vec);
 }
diff --git a/firmware/fastflash.h b/firmware/fastflash.h
@@ -20,7 +20,6 @@
 #ifndef __FASTFLASH_H__
 #define __FASTFLASH_H__
 
-void load_bootloader(void);
 void __attribute__((noreturn)) run_bootloader(void);
 
 #endif
diff --git a/firmware/fsm.c b/firmware/fsm.c
@@ -177,7 +177,7 @@ const CoinType *fsm_getCoin(bool has_name, const char *name)
 
 HDNode *fsm_getDerivedNode(const char *curve, uint32_t *address_n, size_t address_n_count)
 {
-	static HDNode node;
+	static CONFIDENTIAL HDNode node;
 	if (!storage_getRootNode(&node, curve, true)) {
 		fsm_sendFailure(FailureType_Failure_NotInitialized, _("Device not initialized or passphrase request cancelled or unsupported curve"));
 		layoutHome();
diff --git a/firmware/signing.c b/firmware/signing.c
@@ -32,7 +32,7 @@ static uint32_t inputs_count;
 static uint32_t outputs_count;
 static const CoinType *coin;
 static const HDNode *root;
-static HDNode node;
+static CONFIDENTIAL HDNode node;
 static bool signing = false;
 enum {
 	STAGE_REQUEST_1_INPUT,
@@ -54,7 +54,8 @@ static TxInputType input;
 static TxOutputBinType bin_output;
 static TxStruct to, tp, ti;
 static SHA256_CTX hashers[3];
-static uint8_t privkey[32], pubkey[33], sig[64];
+static uint8_t CONFIDENTIAL privkey[32];
+static uint8_t pubkey[33], sig[64];
 static uint8_t hash_prevouts[32], hash_sequence[32],hash_outputs[32];
 static uint8_t hash_check[32];
 static uint64_t to_spend, authorized_amount, spending, change_spend;
diff --git a/firmware/storage.c b/firmware/storage.c
@@ -42,7 +42,7 @@
 #include "usb.h"
 #include "gettext.h"
 
-Storage storage;
+Storage CONFIDENTIAL storage;
 
 uint32_t storage_uuid[12/sizeof(uint32_t)];
 char    storage_uuid_str[25];
@@ -95,12 +95,12 @@ static const uint32_t storage_magic = 0x726f7473;   // 'stor' as uint32_t
 
 static bool sessionSeedCached, sessionSeedUsesPassphrase;
 
-static uint8_t sessionSeed[64];
+static uint8_t CONFIDENTIAL sessionSeed[64];
 
 static bool sessionPinCached;
 
 static bool sessionPassphraseCached;
-static char sessionPassphrase[51];
+static char CONFIDENTIAL sessionPassphrase[51];
 
 #define STORAGE_VERSION 8
 
diff --git a/firmware/transaction.c b/firmware/transaction.c
@@ -152,7 +152,7 @@ int compile_output(const CoinType *coin, const HDNode *root, TxOutputType *in, T
 	}
 
 	if (in->address_n_count > 0) {
-		HDNode node;
+		static CONFIDENTIAL HDNode node;
 		InputScriptType input_script_type;
 
 		switch (in->script_type) {
diff --git a/firmware/trezor.c b/firmware/trezor.c
@@ -95,7 +95,6 @@ int main(void)
 #if FASTFLASH
 	uint16_t state = gpio_port_read(BTN_PORT);
 	if ((state & BTN_PIN_NO) == 0) {
-		load_bootloader();
 		run_bootloader();
 	}
 #endif
diff --git a/firmware/u2f.c b/firmware/u2f.c
@@ -450,7 +450,7 @@ void getReadableAppId(const uint8_t appid[U2F_APPID_SIZE], const char **appname,
 
 const HDNode *getDerivedNode(uint32_t *address_n, size_t address_n_count)
 {
-	static HDNode node;
+	static CONFIDENTIAL HDNode node;
 	if (!storage_getRootNode(&node, NIST256P1_NAME, false)) {
 		layoutHome();
 		debugLog(0, "", "ERR: Device not init");
diff --git a/memory.ld b/memory.ld
@@ -6,4 +6,17 @@ MEMORY
 	ram (rwx) : ORIGIN = 0x20000000, LENGTH = 128K
 }
 
+SECTIONS
+{
+	.confidential (NOLOAD) : {
+		*(confidential)
+		ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!");
+	} >ram
+}
+
 INCLUDE libopencm3_stm32f2.ld
+
+_ram_start = ORIGIN(ram);
+_ram_end = ORIGIN(ram) + LENGTH(ram);
+
+_data_size = SIZEOF(.data);
diff --git a/memory_app_1.0.0.ld b/memory_app_1.0.0.ld
@@ -6,4 +6,17 @@ MEMORY
 	ram (rwx) : ORIGIN = 0x20000000, LENGTH = 128K
 }
 
+SECTIONS
+{
+	.confidential (NOLOAD) : {
+		*(confidential)
+		ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!");
+	} >ram
+}
+
 INCLUDE libopencm3_stm32f2.ld
+
+_ram_start = ORIGIN(ram);
+_ram_end = ORIGIN(ram) + LENGTH(ram);
+
+_data_size = SIZEOF(.data);
diff --git a/memory_app_2.0.0.ld b/memory_app_2.0.0.ld
diff --git a/memory_app_fastflash.ld b/memory_app_fastflash.ld
@@ -6,4 +6,17 @@ MEMORY
 		    LENGTH = 128K - LENGTH(rom)
 }
 
+SECTIONS
+{
+	.confidential (NOLOAD) : {
+		*(confidential)
+		ASSERT ((SIZEOF(.confidential) <= 32K), "Error: Confidential section too big!");
+	} >ram
+}
+
 INCLUDE libopencm3_stm32f2.ld
+
+_ram_start = ORIGIN(ram);
+_ram_end = ORIGIN(ram) + LENGTH(ram);
+
+_data_size = SIZEOF(.data);
diff --git a/setup.c b/setup.c
@@ -17,6 +17,7 @@
  * along with this library.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <libopencm3/cm3/scb.h>
 #include <libopencm3/stm32/rcc.h>
 #include <libopencm3/stm32/gpio.h>
 #include <libopencm3/stm32/spi.h>
@@ -44,6 +45,14 @@ void nmi_handler(void)
 
 void setup(void)
 {
+	// set SCB_CCR STKALIGN bit to make sure 8-byte stack alignment on exception entry is in effect.
+	// This is not strictly necessary for the current TREZOR system.
+	// This is here to comply with guidance from section 3.3.3 "Binary compatibility with other Cortex processors"
+	// of the ARM Cortex-M3 Processor Technical Reference Manual.
+	// According to section 4.4.2 and 4.4.7 of the "STM32F10xxx/20xxx/21xxx/L1xxxx Cortex-M3 programming manual",
+	// STM32F2 series MCUs are r2p0 and always have this bit set on reset already.
+	SCB_CCR |= SCB_CCR_STKALIGN;
+
 	// setup clock
 	struct rcc_clock_scale clock = rcc_hse_8mhz_3v3[RCC_CLOCK_3V3_120MHZ];
 	rcc_clock_setup_hse_3v3(&clock);
diff --git a/startup.s b/startup.s
@@ -0,0 +1,39 @@
+  .syntax unified
+
+  .text
+
+  .global memset_reg
+  .type memset_reg, STT_FUNC
+memset_reg:
+  // call with the following (note that the arguments are not validated prior to use):
+  // r0 - address of first word to write (inclusive)
+  // r1 - address of first word following the address in r0 to NOT write (exclusive)
+  // r2 - word value to be written
+  // both addresses in r0 and r1 needs to be divisible by 4!
+  .L_loop_begin:
+    str r2, [r0], 4 // store the word in r2 to the address in r0, post-indexed
+    cmp r0, r1
+  bne .L_loop_begin
+  bx lr
+
+  .global reset_handler
+  .type reset_handler, STT_FUNC
+reset_handler:
+  ldr r0, =_ram_start // r0 - point to beginning of SRAM
+  ldr r1, =_ram_end   // r1 - point to byte after the end of SRAM
+  ldr r2, =0          // r2 - the byte-sized value to be written
+  bl memset_reg
+
+  // copy .data section from flash to SRAM
+  ldr r0, =_data          // dst addr
+  ldr r1, =_data_loadaddr // src addr
+  ldr r2, =_data_size     // length in bytes
+  bl memcpy
+
+  // enter the application code
+  bl main
+
+  // loop forever if the application code returns
+  b .
+
+  .end
diff --git a/util.h b/util.h

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+OBJS += startup.o`
`1`	`2`	`OBJS += buttons.o`
`2`	`3`	`OBJS += layout.o`
`3`	`4`	`OBJS += oled.o`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,9 @@ void show_unofficial_warning(const uint8_t *hash)`
`78`	`78`
`79`	`79`	`void __attribute__((noreturn)) load_app(void)`
`80`	`80`	`{`
	`81`	`+ // zero out SRAM`
	`82`	`+ memset_reg(_ram_start, _ram_end, 0);`
	`83`	`+`
`81`	`84`	`load_vector_table((const vector_table_t *) FLASH_APP_START);`
`82`	`85`	`}`
`83`	`86`
Original file line number	Diff line number	Diff line change
`@@ -177,7 +177,7 @@ const CoinType fsm_getCoin(bool has_name, const char name)`
`177`	`177`
`178`	`178`	`HDNode fsm_getDerivedNode(const char curve, uint32_t *address_n, size_t address_n_count)`
`179`	`179`	`{`
`180`		`- static HDNode node;`
	`180`	`+ static CONFIDENTIAL HDNode node;`
`181`	`181`	`if (!storage_getRootNode(&node, curve, true)) {`
`182`	`182`	`fsm_sendFailure(FailureType_Failure_NotInitialized, _("Device not initialized or passphrase request cancelled or unsupported curve"));`
`183`	`183`	`layoutHome();`