From 054344de6b4eb0dea5892650312d6f8c8bbbfc25 Mon Sep 17 00:00:00 2001
From: Robbe Derks <robbe.derks@gmail.com>
Date: Wed, 14 Jun 2023 15:28:49 +0200
Subject: [PATCH] Flash bounds checking outside of bootstub (#1459)

* add bounds checking on flash operations outside of bootstub

* fix test
---
 board/drivers/logging.h        |  2 +-
 board/fake_stm.h               |  1 +
 board/stm32fx/llflash.h        | 29 +++++++++++++++++++++++------
 board/stm32fx/stm32fx_config.h |  1 +
 board/stm32h7/llflash.h        | 31 ++++++++++++++++++++++++-------
 board/stm32h7/stm32h7_config.h |  1 +
 6 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/board/drivers/logging.h b/board/drivers/logging.h
index 6d21c87c..117de587 100644
--- a/board/drivers/logging.h
+++ b/board/drivers/logging.h
@@ -1,7 +1,7 @@
 
 #include "logging_definitions.h"
 
-#define BANK_SIZE 0x20000U
+#define BANK_SIZE LOGGING_FLASH_SECTOR_SIZE
 #define BANK_LOG_CAPACITY (BANK_SIZE / sizeof(log_t))
 #define TOTAL_LOG_CAPACITY (BANK_LOG_CAPACITY * 2U)
 
diff --git a/board/fake_stm.h b/board/fake_stm.h
index 66baa1d3..e6709c78 100644
--- a/board/fake_stm.h
+++ b/board/fake_stm.h
@@ -56,6 +56,7 @@ uint8_t fake_logging_bank[0x40000] __attribute__ ((aligned (4)));
 #define LOGGING_FLASH_BASE_B (&fake_logging_bank[0x20000])
 #define LOGGING_FLASH_SECTOR_A 5
 #define LOGGING_FLASH_SECTOR_B 6
+#define LOGGING_FLASH_SECTOR_SIZE 0x20000U
 
 bool flash_locked = true;
 void flash_unlock(void) {
diff --git a/board/stm32fx/llflash.h b/board/stm32fx/llflash.h
index 7ae14ca5..52f62848 100644
--- a/board/stm32fx/llflash.h
+++ b/board/stm32fx/llflash.h
@@ -12,9 +12,17 @@ void flash_lock(void) {
 }
 
 bool flash_erase_sector(uint16_t sector) {
+#ifdef BOOTSTUB
   // don't erase the bootloader(sector 0)
+  uint16_t min_sector = 1U;
+  uint16_t max_sector = 11U;
+#else
+  uint16_t min_sector = LOGGING_FLASH_SECTOR_A;
+  uint16_t max_sector = LOGGING_FLASH_SECTOR_B;
+#endif
+
   bool ret = false;
-  if ((sector != 0U) && (sector < 12U) && (!flash_is_locked())) {
+  if ((sector >= min_sector) && (sector <= max_sector) && (!flash_is_locked())) {
     FLASH->CR = (sector << 3) | FLASH_CR_SER;
     FLASH->CR |= FLASH_CR_STRT;
     while ((FLASH->SR & FLASH_SR_BSY) != 0U);
@@ -23,11 +31,20 @@ bool flash_erase_sector(uint16_t sector) {
   return ret;
 }
 
-void flash_write_word(void *prog_ptr, uint32_t data) {
-  uint32_t *pp = prog_ptr;
-  FLASH->CR = FLASH_CR_PSIZE_1 | FLASH_CR_PG;
-  *pp = data;
-  while ((FLASH->SR & FLASH_SR_BSY) != 0U);
+void flash_write_word(uint32_t *prog_ptr, uint32_t data) {
+  #ifndef BOOTSTUB
+  // don't write to any region besides the logging region
+  if ((prog_ptr >= (uint32_t *)LOGGING_FLASH_BASE_A) && (prog_ptr < (uint32_t *)(LOGGING_FLASH_BASE_B + LOGGING_FLASH_SECTOR_SIZE))) {
+  #endif
+
+    uint32_t *pp = prog_ptr;
+    FLASH->CR = FLASH_CR_PSIZE_1 | FLASH_CR_PG;
+    *pp = data;
+    while ((FLASH->SR & FLASH_SR_BSY) != 0U);
+
+  #ifndef BOOTSTUB
+  }
+  #endif
 }
 
 void flush_write_buffer(void) { }
diff --git a/board/stm32fx/stm32fx_config.h b/board/stm32fx/stm32fx_config.h
index 8bf54965..1573426d 100644
--- a/board/stm32fx/stm32fx_config.h
+++ b/board/stm32fx/stm32fx_config.h
@@ -42,6 +42,7 @@
 #define LOGGING_FLASH_SECTOR_B 11U
 #define LOGGING_FLASH_BASE_A 0x080C0000U
 #define LOGGING_FLASH_BASE_B 0x080E0000U
+#define LOGGING_FLASH_SECTOR_SIZE 0x20000U
 
 #include "can_definitions.h"
 #include "comms_definitions.h"
diff --git a/board/stm32h7/llflash.h b/board/stm32h7/llflash.h
index ece69c5c..800d9414 100644
--- a/board/stm32h7/llflash.h
+++ b/board/stm32h7/llflash.h
@@ -12,9 +12,17 @@ void flash_lock(void) {
 }
 
 bool flash_erase_sector(uint16_t sector) {
-  // don't erase the bootloader(sector 0)
+  #ifdef BOOTSTUB
+    // don't erase the bootloader(sector 0)
+    uint16_t min_sector = 1U;
+    uint16_t max_sector = 7U;
+  #else
+    uint16_t min_sector = LOGGING_FLASH_SECTOR_A;
+    uint16_t max_sector = LOGGING_FLASH_SECTOR_B;
+  #endif
+
   bool ret = false;
-  if ((sector != 0U) && (sector < 8U) && (!flash_is_locked())) {
+  if ((sector >= min_sector) && (sector <= max_sector) && (!flash_is_locked())) {
     FLASH->CR1 = (sector << 8) | FLASH_CR_SER;
     FLASH->CR1 |= FLASH_CR_START;
     while ((FLASH->SR1 & FLASH_SR_QW) != 0U);
@@ -23,11 +31,20 @@ bool flash_erase_sector(uint16_t sector) {
   return ret;
 }
 
-void flash_write_word(void *prog_ptr, uint32_t data) {
-  uint32_t *pp = prog_ptr;
-  FLASH->CR1 |= FLASH_CR_PG;
-  *pp = data;
-  while ((FLASH->SR1 & FLASH_SR_QW) != 0U);
+void flash_write_word(uint32_t *prog_ptr, uint32_t data) {
+  #ifndef BOOTSTUB
+  // don't write to any region besides the logging region
+  if ((prog_ptr >= (uint32_t *)LOGGING_FLASH_BASE_A) && (prog_ptr < (uint32_t *)(LOGGING_FLASH_BASE_B + LOGGING_FLASH_SECTOR_SIZE))) {
+  #endif
+
+    uint32_t *pp = prog_ptr;
+    FLASH->CR1 |= FLASH_CR_PG;
+    *pp = data;
+    while ((FLASH->SR1 & FLASH_SR_QW) != 0U);
+
+  #ifndef BOOTSTUB
+  }
+  #endif
 }
 
 void flush_write_buffer(void) {
diff --git a/board/stm32h7/stm32h7_config.h b/board/stm32h7/stm32h7_config.h
index 85e349e3..4e6f99bf 100644
--- a/board/stm32h7/stm32h7_config.h
+++ b/board/stm32h7/stm32h7_config.h
@@ -50,6 +50,7 @@ separate IRQs for RX and TX.
 #define LOGGING_FLASH_SECTOR_B 6U
 #define LOGGING_FLASH_BASE_A 0x080A0000U
 #define LOGGING_FLASH_BASE_B 0x080C0000U
+#define LOGGING_FLASH_SECTOR_SIZE 0x20000U
 
 #include "can_definitions.h"
 #include "comms_definitions.h"