startup: use custom reset_handler

+ group confidential data in one place
+ zero all SRAM where needed

Makefile

@@ -1,3 +1,4 @@
+OBJS += startup.o
 OBJS += buttons.o
 OBJS += layout.o
 OBJS += oled.o

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 \
+            $(CPUFLAGS) \
-            -mthumb \
+            $(FPUFLAGS) \
-            -msoft-float \
             -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 \
+            $(CPUFLAGS) \
-            -mthumb \
+            $(FPUFLAGS)
-            -msoft-float
 
 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 $<
 

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);
 }
 

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"

firmware/Makefile

@@ -1,10 +1,8 @@
-ifeq ($(FASTFLASH),1)
+APPVER = 1.0.0
-APPVER = 2.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 \
 		$< $@

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;
 

firmware/fastflash.c

@@ -24,16 +24,18 @@
 #include <stdint.h>
 #include <string.h>
 
-extern uint8_t __bootloader_loadaddr__[];
+#define bootloader_vec ((vector_table_t *) 0x20000000)
-extern uint8_t __bootloader_runaddr__[];
-extern uint8_t __bootloader_size__[];
-
-void load_bootloader(void)
-{
-	memcpy(__bootloader_runaddr__, __bootloader_loadaddr__, (size_t) __bootloader_size__);
-}
 
 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);
 }

firmware/fastflash.h

@@ -20,7 +20,6 @@
 #ifndef __FASTFLASH_H__
 #define __FASTFLASH_H__
 
-void load_bootloader(void);
 void __attribute__((noreturn)) run_bootloader(void);
 
 #endif

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();

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;

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
 

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) {

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

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");

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);

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);

memory_app_2.0.0.ld

@@ -1,21 +0,0 @@
-/* STM32F205RE - 512K Flash, 128K RAM */
-/* program starts at 0x08010000 */
-MEMORY
-{
-	rom (rx)        : ORIGIN = 0x08010000, LENGTH = 512K - 64K
-	bootloader (rx) : ORIGIN = 0x20000000, LENGTH = 32K
-	ram (rwx)       : ORIGIN = 0x20000000 + LENGTH(bootloader),
-			  LENGTH = 128K - LENGTH(bootloader)
-}
-
-INCLUDE libopencm3_stm32f2.ld
-
-SECTIONS
-{
-	.bootloader : {
-		__bootloader_runaddr__ = .;
-		KEEP (*(.bootloader*))
-	} >bootloader AT >rom
-
-	__bootloader_loadaddr__ = LOADADDR(.bootloader);
-}

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);

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);

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

util.h

@@ -41,18 +41,25 @@ void __attribute__((noreturn)) system_halt(void);
 // reset system
 void __attribute__((noreturn)) system_reset(void);
 
-static inline void __attribute__((noreturn)) load_vector_table(const vector_table_t *vector_table) {
+// defined in memory.ld
-    // Relocate vector table
+extern uint8_t _ram_start[], _ram_end[];
-    SCB_VTOR = (uint32_t) vector_table;
 
-    // Set stack pointer
+// defined in startup.s
-    __asm__ volatile("msr msp, %0" :: "r" (vector_table->initial_sp_value));
+extern void memset_reg(void *start, void *stop, uint32_t val);
 
-    // Jump to address
+static inline void __attribute__((noreturn)) load_vector_table(const vector_table_t *vector_table)
-    vector_table->reset();
+{
+	// Relocate vector table
+	SCB_VTOR = (uint32_t)vector_table;
 
-    // Prevent compiler from generating stack protector code (which causes CPU fault because the stack is moved)
+	// Set stack pointer
-    for (;;);
+	__asm__ volatile("msr msp, %0" :: "r" (vector_table->initial_sp_value));
+
+	// Jump to address
+	vector_table->reset();
+
+	// Prevent compiler from generating stack protector code (which causes CPU fault because the stack is moved)
+	for (;;);
 }
 
 #endif