bootloader: Don't access R/W memory when bootloading

The RAM firmware image is loaded from the beginning of SRAM, which
overlaps read/write data used by the ROM firmware. This has the
potential to corrupt ROM runtime data that is still being loaded in and
out of memory while it is preparing to boot the image.

This patch removes the image booting logic from the `msys_rom` and
`juno_rom` modules and moves it to the `bootloader` module to ensure
that the transfer is properly and safely contained and controlled.

Other changes to reduce exploitability including updating the stack
pointer to the one expected by the RAM firmware rather than permitting
it to continue from where the ROM firmware left off, and relocating the
vector table to the one embedded in the RAM firmware image, which
prevents the core from using the exception handlers created during ROM
boot (which are in the heap and may have been corrupted while loading
the RAM firmware).

Change-Id: I4413c1cd058ca93ef04177424f1f29561b10872f
Signed-off-by: Chris Kay <chris.kay@arm.com>

10 files changed, 110 insertions, 103 deletions

diff --git a/Makefile b/Makefile
index 3506787a..cb9c1676 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,7 @@ export TOOLS_DIR := $(TOP_DIR)/tools
 export BS_DIR := $(TOOLS_DIR)/build_system
 export PRODUCTS_DIR := $(TOP_DIR)/product
 export MODULES_DIR := $(TOP_DIR)/module
+export CMSIS_DIR := $(TOP_DIR)/cmsis/CMSIS/Core
 export OS_DIR := $(TOP_DIR)/cmsis/CMSIS/RTOS2
 
 BUILD_STRING := $(shell $(TOOLS_DIR)/build_string.py 2>/dev/null)
diff --git a/module/bootloader/src/Makefile b/module/bootloader/src/Makefile
index 20397275..e3f44fdc 100644
--- a/module/bootloader/src/Makefile
+++ b/module/bootloader/src/Makefile
@@ -6,6 +6,8 @@
 #
 
 BS_LIB_NAME := Bootloader
-BS_LIB_SOURCES := mod_bootloader.c
+BS_LIB_SOURCES := \
+    mod_bootloader.c \
+    mod_bootloader_boot.S
 
 include $(BS_DIR)/lib.mk
diff --git a/module/bootloader/src/mod_bootloader.c b/module/bootloader/src/mod_bootloader.c
index ae3bebe3..1cbe3682 100644
--- a/module/bootloader/src/mod_bootloader.c
+++ b/module/bootloader/src/mod_bootloader.c
@@ -5,15 +5,21 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#include <stdint.h>
-#include <string.h>
+#include <mod_bootloader.h>
+#include <mod_sds.h>
+
 #include <fwk_element.h>
 #include <fwk_id.h>
+#include <fwk_interrupt.h>
 #include <fwk_module.h>
 #include <fwk_module_idx.h>
+#include <fwk_noreturn.h>
 #include <fwk_status.h>
-#include <mod_bootloader.h>
-#include <mod_sds.h>
+
+#include <fmw_cmsis.h>
+
+#include <stdint.h>
+#include <string.h>
 
 /* Offset within the SDS structure where the valid flag is located. */
 #define BOOTLOADER_STRUCT_VALID_POS           0
@@ -38,6 +44,12 @@ static struct bootloader_ctx module_ctx;
 
 static int load_image(void)
 {
+    extern noreturn void mod_bootloader_boot(
+        uintptr_t destination,
+        void *source,
+        size_t size,
+        volatile uint32_t *vtor);
+
     int status;
     void *image_base;
     uint32_t image_flags;
@@ -96,10 +108,13 @@ static int load_image(void)
     image_base = (void *)((uint8_t *)module_ctx.module_config->source_base +
                           image_offset);
 
-    memcpy((void *)module_ctx.module_config->destination_base, image_base,
-           image_size);
+    fwk_interrupt_global_disable(); /* We are relocating the vector table */
 
-    return FWK_SUCCESS;
+    mod_bootloader_boot(
+        module_ctx.module_config->destination_base,
+        image_base,
+        image_size,
+        &SCB->VTOR);
 }
 
 static const struct mod_bootloader_api bootloader_api = {
diff --git a/module/bootloader/src/mod_bootloader_boot.S b/module/bootloader/src/mod_bootloader_boot.S
new file mode 100644
index 00000000..7f4a9d98
--- /dev/null
+++ b/module/bootloader/src/mod_bootloader_boot.S
@@ -0,0 +1,41 @@
+/*
+ * Arm SCP/MCP Software
+ * Copyright (c) 2020, Arm Limited and Contributors. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+    .syntax unified
+
+    .text
+
+/*
+ * noreturn void mod_bootloader_boot(
+ *     uint8_t *destination,
+ *     uint8_t *source,
+ *     uint32_t size,
+ *     uint32_t *vtor);
+ */
+    .thumb
+    .thumb_func
+    .global mod_bootloader_boot
+    .type mod_bootloader_boot, %function
+mod_bootloader_boot:
+    movs r4, r0 /* Save the destination - it soon points to the vector table */
+
+1:
+    ldrb r5, [r1], #1 /* Load next byte from source */
+    strb r5, [r0], #1 /* Store next byte at destination */
+
+    subs r2, #1 /* Decrement the size, which we use as the counter... */
+    bne 1b /* ... until it reaches zero */
+
+    str r4, [r3] /* Store vector table address in SCB->VTOR (if it exists) */
+
+    ldr r0, [r4] /* Grab new stack pointer from vector table... */
+    msr msp, r0 /* ... and update the main stack pointer with it */
+
+    ldr r0, [r4, #4] /* Load the reset address from the vector table... */
+    bx r0 /* ... and take a leap of faith */
+
+    .pool
diff --git a/module/msys_rom/include/mod_msys_rom.h b/module/msys_rom/include/mod_msys_rom.h
index 1506a2aa..ce682698 100644
--- a/module/msys_rom/include/mod_msys_rom.h
+++ b/module/msys_rom/include/mod_msys_rom.h
@@ -12,9 +12,11 @@
 #ifndef MOD_MSYS_ROM_H
 #define MOD_MSYS_ROM_H
 
+#include <fwk_id.h>
+#include <fwk_module_idx.h>
+
 #include <stddef.h>
 #include <stdint.h>
-#include <fwk_id.h>
 
 /*!
  * \ingroup GroupMSYSModule
@@ -32,9 +34,6 @@ struct msys_rom_config {
     /*! Size of the AP context area */
     const size_t ap_context_size;
 
-    /*! Base address of the RAM firmware image */
-    const uintptr_t ramfw_base;
-
     /*! Element ID of the primary cluster PPU */
     const fwk_id_t id_primary_cluster;
 
diff --git a/module/msys_rom/src/mod_msys_rom.c b/module/msys_rom/src/mod_msys_rom.c
index 5c6093cc..001bb4c2 100644
--- a/module/msys_rom/src/mod_msys_rom.c
+++ b/module/msys_rom/src/mod_msys_rom.c
@@ -9,22 +9,21 @@
  *     firmware.
  */
 
-#include <stdbool.h>
-#include <stdint.h>
-#include <string.h>
+#include <mod_bootloader.h>
+#include <mod_log.h>
+#include <mod_msys_rom.h>
+#include <mod_power_domain.h>
+#include <mod_ppu_v1.h>
+
 #include <fwk_assert.h>
-#include <fwk_interrupt.h>
 #include <fwk_module.h>
-#include <fwk_module_idx.h>
 #include <fwk_noreturn.h>
 #include <fwk_notification.h>
 #include <fwk_status.h>
 #include <fwk_thread.h>
-#include <mod_bootloader.h>
-#include <mod_log.h>
-#include <mod_msys_rom.h>
-#include <mod_power_domain.h>
-#include <mod_ppu_v1.h>
+
+#include <stdbool.h>
+#include <string.h>
 
 struct msys_rom_ctx {
     const struct msys_rom_config *rom_config;
@@ -39,34 +38,6 @@ enum rom_event {
     ROM_EVENT_COUNT
 };
 
-/*
- * This function assumes that the RAM firmware image is located at the beginning
- * of the SCP SRAM. The reset handler will be at offset 0x4.
- */
-static noreturn void msys_jump_to_ramfw(void)
-{
-    uintptr_t const *reset_base =
-        (uintptr_t *)(ctx.rom_config->ramfw_base + 0x4);
-    void (*ramfw_reset_handler)(void);
-
-    /*
-     * Disable interrupts for the duration of the ROM firmware to RAM firmware
-     * transition.
-     */
-    fwk_interrupt_global_disable();
-
-    ramfw_reset_handler = (void (*)(void))*reset_base;
-
-    /*
-     * Execute the RAM firmware's reset handler to pass control from ROM
-     * firmware to the RAM firmware.
-     */
-    ramfw_reset_handler();
-
-    while (true)
-        continue;
-}
-
 static int msys_deferred_setup(void)
 {
     int status;
@@ -83,15 +54,13 @@ static int msys_deferred_setup(void)
     ctx.log_api->log(MOD_LOG_GROUP_INFO, "[SYSTEM] Primary CPU powered\n");
 
     status = ctx.bootloader_api->load_image();
-    if (status != FWK_SUCCESS) {
-        ctx.log_api->log(MOD_LOG_GROUP_ERROR,
-                         "[SYSTEM] Failed to load RAM firmware image\n");
-        return FWK_E_DATA;
-    }
 
-    msys_jump_to_ramfw();
+    ctx.log_api->log(
+        MOD_LOG_GROUP_ERROR,
+        "[SYSTEM] Failed to load RAM firmware image: %d\n",
+        status);
 
-    return FWK_SUCCESS;
+    return FWK_E_DATA;
 }
 
 /*
diff --git a/product/juno/module/juno_rom/src/mod_juno_rom.c b/product/juno/module/juno_rom/src/mod_juno_rom.c
index 2d59ec4a..9bf5e058 100644
--- a/product/juno/module/juno_rom/src/mod_juno_rom.c
+++ b/product/juno/module/juno_rom/src/mod_juno_rom.c
@@ -5,20 +5,6 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#include <string.h>
-#include <fwk_assert.h>
-#include <fwk_id.h>
-#include <fwk_interrupt.h>
-#include <fwk_module.h>
-#include <fwk_module_idx.h>
-#include <fwk_notification.h>
-#include <fwk_status.h>
-#include <fwk_thread.h>
-#include <mod_bootloader.h>
-#include <mod_juno_ppu.h>
-#include <mod_juno_rom.h>
-#include <mod_log.h>
-#include <mod_power_domain.h>
 #include <juno_debug_rom.h>
 #include <juno_nic400.h>
 #include <juno_ppu_idx.h>
@@ -27,6 +13,22 @@
 #include <juno_wdog_rom.h>
 #include <scp_config.h>
 
+#include <mod_bootloader.h>
+#include <mod_juno_ppu.h>
+#include <mod_juno_rom.h>
+#include <mod_log.h>
+#include <mod_power_domain.h>
+
+#include <fwk_assert.h>
+#include <fwk_id.h>
+#include <fwk_module.h>
+#include <fwk_module_idx.h>
+#include <fwk_notification.h>
+#include <fwk_status.h>
+#include <fwk_thread.h>
+
+#include <string.h>
+
 /* Values for cluster configuration */
 #define CLUSTERCLK_CONTROL_CLKDIVSYS        UINT32_C(0x000000F0)
 #define CLUSTERCLK_CONTROL_CLKDIVEXT        UINT32_C(0x00000F00)
@@ -166,25 +168,6 @@ static void css_clock_cluster_sel_set(volatile uint32_t *clk,
     }
 }
 
-/*
- * This function assumes that the RAM firmware image is located at the beginning
- * of the SCP SRAM. The reset handler will be at offset 0x4.
- */
-static noreturn void jump_to_ramfw(void)
-{
-    uintptr_t *reset_base = NULL;
-    void (*ramfw_reset_handler)(void);
-
-    fwk_interrupt_global_disable();
-
-    reset_base = (uintptr_t *)(ctx.config->ramfw_base + 0x4);
-    ramfw_reset_handler = (void (*)(void))*reset_base;
-    ramfw_reset_handler();
-
-    while (true)
-        continue;
-}
-
 static int deferred_setup(void)
 {
     int status;
@@ -235,19 +218,13 @@ static int deferred_setup(void)
     #endif
 
     status = ctx.bootloader_api->load_image();
-    if (status != FWK_SUCCESS) {
-        ctx.log_api->log(MOD_LOG_GROUP_ERROR,
-                         "[ROM] ERROR: Failed to load RAM firmware image\n");
-        return FWK_E_DATA;
-    }
 
-    #ifndef BUILD_MODE_DEBUG
-    juno_wdog_rom_reload();
-    #endif
+    ctx.log_api->log(
+        MOD_LOG_GROUP_ERROR,
+        "[ROM] ERROR: Failed to load RAM firmware image: %d\n",
+        status);
 
-    jump_to_ramfw();
-
-    return FWK_SUCCESS;
+    return FWK_E_DATA;
 }
 
 /*
diff --git a/product/sgm775/scp_romfw/config_msys_rom.c b/product/sgm775/scp_romfw/config_msys_rom.c
index 1921b985..85b726a8 100644
--- a/product/sgm775/scp_romfw/config_msys_rom.c
+++ b/product/sgm775/scp_romfw/config_msys_rom.c
@@ -17,7 +17,6 @@ const struct fwk_module_config config_msys_rom = {
     .data = &((struct msys_rom_config) {
         .ap_context_base = AP_CONTEXT_BASE,
         .ap_context_size = AP_CONTEXT_SIZE,
-        .ramfw_base = SCP_RAM_BASE,
         .id_primary_cluster = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_PPU_V1, 0),
         .id_primary_core = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_PPU_V1, 1),
     })
diff --git a/product/sgm776/scp_romfw/config_msys_rom.c b/product/sgm776/scp_romfw/config_msys_rom.c
index 1e38f1b1..8da6fcc3 100644
--- a/product/sgm776/scp_romfw/config_msys_rom.c
+++ b/product/sgm776/scp_romfw/config_msys_rom.c
@@ -16,7 +16,6 @@ const struct fwk_module_config config_msys_rom = {
     .data = &((struct msys_rom_config) {
         .ap_context_base = AP_CONTEXT_BASE,
         .ap_context_size = AP_CONTEXT_SIZE,
-        .ramfw_base = SCP_RAM_BASE,
         .id_primary_cluster = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_PPU_V1, 2),
         .id_primary_core = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_PPU_V1, 3),
     })
diff --git a/tools/build_system/rules.mk b/tools/build_system/rules.mk
index 67bb655d..c7d78249 100644
--- a/tools/build_system/rules.mk
+++ b/tools/build_system/rules.mk
@@ -166,6 +166,11 @@ endif
 INCLUDES += $(FWK_DIR)/include
 
 #
+# Always include CMSIS
+#
+INCLUDES += $(CMSIS_DIR)/Include
+
+#
 # Toolchain-independent flags
 #
 CFLAGS += -O$(O)