From 95af7040303c29d4cdef2ee2bf7aaca21805d02a Mon Sep 17 00:00:00 2001
From: Michele Balistreri <opensource@bitgamma.com>
Date: Tue, 15 May 2018 12:28:32 +0300
Subject: [PATCH] zero memory before booting the firmware

---
 Inc/sys.h     | 10 ++++------
 README.md     |  5 ++---
 Src/main.c    | 17 +++++++++++------
 Src/memzero.s | 19 +++++++++++++++++++
 4 files changed, 36 insertions(+), 15 deletions(-)
 create mode 100644 Src/memzero.s

diff --git a/Inc/sys.h b/Inc/sys.h
index 32ce8f5..c9eb99f 100644
--- a/Inc/sys.h
+++ b/Inc/sys.h
@@ -12,6 +12,9 @@
 #define FLASH_START_BANK1 FLASH_BASE
 #define FLASH_START_BANK2 (FLASH_BASE + FLASH_BANK_SIZE)
 
+#define SRAM_START SRAM1_BASE
+#define SRAM_END (SRAM_START + SRAM1_SIZE_MAX)
+
 // Utility macros
 #define UINT32_PTR(n) ((uint32_t *) n)
 #define UINT8_PTR(n) ((uint8_t *) n)
@@ -29,11 +32,6 @@
        __x > __y ? __x : __y; })
 #endif
 
-static inline void memzero(volatile void *v, uint32_t len) {
-  if (len) {
-    memset((void *) v, 0, len);
-    (void) *((volatile uint8_t *) v);
-  }
-}
+void memzero(uintptr_t start, uintptr_t end);
 
 #endif /* SYS_H_ */
diff --git a/README.md b/README.md
index 6041f93..41ea152 100644
--- a/README.md
+++ b/README.md
@@ -31,7 +31,7 @@ This document describes the bootloader, the memory layout and upgrade mechanism
     |            USER DATA              |
     |                                   |
     |                                   |
-    |-----------------------------------|
+    |===================================|
     
 The STM32 chips divide flash memory into two banks. Reading one bank while writing on the other is possible, so the user data and the firmware will live in two different banks. The page size is 2k, so all boundaries will be aligned to a multiple of that size. The area allocated for the firmware is (BANK_SIZE - BOOTLOADER_SIZE) / 2, since two instances of the firmware will need to fit in the first bank. The user data area size will be FW_SIZE + BOOTLOADER_SIZE, as to allocate the entire memory. Absolute sizes cannot be provided yet. When we have a final bootloader (estimated to be in the 4-8k range) and a final firmware we will decide how much flash memory on chip we need and with this finally calculate the absolute size. The chip must be chosen by keeping in mind that the firmware might grow by up to 50% in size (although that is unlikely) with future upgrades.
 
@@ -54,8 +54,7 @@ The linker definition file of the firmware must set the flash start address at f
 
 ## Booting
 
-The bootloader checks the integrity of the firmware by verifying its signature. It substitutes its Interrupt Vector pointer with the one defined by the firmware by setting the corresponding registry. *(It clears the entire content of
-the SRAM1 for security reasons, is this needed? we only load trusted firmware)*. It restores the stack pointer and jumps to the reset handler of the firmware.
+The bootloader checks the integrity of the firmware by verifying its signatures. It substitutes its Interrupt Vector pointer with the one defined by the firmware by setting the corresponding registry. It clears the entire content of the SRAM1, then restores the stack pointer and jumps to the reset handler of the firmware.
 
 ## Upgrade procedure
 
diff --git a/Src/main.c b/Src/main.c
index fec9e5a..8e53d62 100644
--- a/Src/main.c
+++ b/Src/main.c
@@ -38,7 +38,7 @@ int check_firmware(uintptr_t addr) {
 }
 
 
-void _load_firmware(uintptr_t newfw, uint8_t newfw_bank, uint8_t newfw_page) {
+void _load_firmware(uintptr_t newfw, uint8_t newfw_bank, uint8_t newfw_page, uint8_t remove_newfw) {
   flash_unlock();
 
   do {
@@ -46,21 +46,26 @@ void _load_firmware(uintptr_t newfw, uint8_t newfw_bank, uint8_t newfw_page) {
     flash_copy(UINT32_PTR(newfw),  UINT32_PTR(FIRMWARE_START), UINT32_PTR(newfw)[1]);
   } while(check_firmware(FIRMWARE_START) != 0);
 
-  flash_erase(newfw_bank, newfw_page, FIRMWARE_PAGE_COUNT);
+  if (remove_newfw) {
+    flash_erase(newfw_bank, newfw_page, FIRMWARE_PAGE_COUNT);
+  }
+
   flash_lock();
 }
 
 void upgrade_firmware() {
-  _load_firmware(UPGRADE_FW_START, FLASH_BANK2, UPGRADE_FW_FIRST_PAGE);
+  _load_firmware(UPGRADE_FW_START, FLASH_BANK2, UPGRADE_FW_FIRST_PAGE, 1);
+}
+
+void factory_reset(void) {
+  _load_firmware(RECOVERY_FW_START, FLASH_BANK1, RECOVERY_FW_FIRST_PAGE, 0);
 }
 
 void run_firmware(void) {
+  memzero(SRAM_START, SRAM_END);
   uint32_t* fw_entry = UINT32_PTR(FIRMWARE_CODE_START);
   SCB->VTOR = (uint32_t) fw_entry;
   __set_MSP(fw_entry[0]);
   ((void (*)(void))fw_entry[1])();
 }
 
-void factory_reset(void) {
-  _load_firmware(RECOVERY_FW_START, FLASH_BANK1, RECOVERY_FW_FIRST_PAGE);
-}
diff --git a/Src/memzero.s b/Src/memzero.s
new file mode 100644
index 0000000..2efef43
--- /dev/null
+++ b/Src/memzero.s
@@ -0,0 +1,19 @@
+	.text
+	.syntax unified
+	.global memzero
+	.func memzero
+	.thumb_func
+
+memzero:
+	/* on entry
+	 *   r0 = start
+	 *   r1 = end
+	 * on exit
+	 *   r0 = end
+	 */
+    mov r2, #0
+ L_loop_begin:
+    str r2, [r0], 4
+    cmp r0, r1
+    bne L_loop_begin
+    bx lr