feat(fvp): update HW_CONFIG DT loading mechanism
Currently, HW-config is loaded into non-secure memory, which mean
a malicious NS-agent could tamper with it. Ideally, this shouldn't
be an issue since no software runs in non-secure world at this time
(non-secure world has not been started yet).
It does not provide a guarantee though since malicious external
NS-agents can take control of this memory region for update/corruption
after BL2 loads it and before BL31/BL32/SP_MIN consumes it. The threat
is mapped to Threat ID#3 (Bypass authentication scenario) in threat
model [1].
Hence modified the code as below -
1. BL2 loads the HW_CONFIG into secure memory
2. BL2 makes a copy of the HW_CONFIG in the non-secure memory at an
address provided by the newly added property(ns-load-address) in
the 'hw-config' node of the FW_CONFIG
3. SP_MIN receives the FW_CONFIG address from BL2 via arg1 so that
it can retrieve details (address and size) of HW_CONFIG from
FW_CONFIG
4. A secure and non-secure HW_CONFIG address will eventually be used
by BL31/SP_MIN/BL32 and BL33 components respectively
5. BL31/SP_MIN dynamically maps the Secure HW_CONFIG region and reads
information from it to local variables (structures) and then
unmaps it
6. Reduce HW_CONFIG maximum size from 16MB to 1MB; it appears
sufficient, and it will also create a free space for any future
components to be added to memory
[1]: https://trustedfirmware-a.readthedocs.io/en/latest/threat_model/threat_model.html
Change-Id: I1d431f3e640ded60616604b1c33aa638b9a1e55e
Signed-off-by: Manish V Badarkhe <Manish.Badarkhe@arm.com>
diff --git a/plat/arm/board/fvp/fvp_bl31_setup.c b/plat/arm/board/fvp/fvp_bl31_setup.c
index a94a4f4..dd90965 100644
--- a/plat/arm/board/fvp/fvp_bl31_setup.c
+++ b/plat/arm/board/fvp/fvp_bl31_setup.c
@@ -17,6 +17,8 @@
#include "fvp_private.h"
+static const struct dyn_cfg_dtb_info_t *hw_config_info __unused;
+
void __init bl31_early_platform_setup2(u_register_t arg0,
u_register_t arg1, u_register_t arg2, u_register_t arg3)
{
@@ -34,6 +36,17 @@
if (soc_fw_config_info != NULL) {
arg1 = soc_fw_config_info->config_addr;
}
+
+ /*
+ * arg2 is currently holding the 'secure' address of HW_CONFIG.
+ * But arm_bl31_early_platform_setup() below expects the 'non-secure'
+ * address of HW_CONFIG (which it will pass to BL33).
+ * This why we need to override arg2 here.
+ */
+ hw_config_info = FCONF_GET_PROPERTY(dyn_cfg, dtb, HW_CONFIG_ID);
+ assert(hw_config_info != NULL);
+ assert(hw_config_info->ns_config_addr != 0UL);
+ arg2 = hw_config_info->ns_config_addr;
#endif /* !RESET_TO_BL31 && !BL2_AT_EL3 */
arm_bl31_early_platform_setup((void *)arg0, arg1, arg2, (void *)arg3);
@@ -66,22 +79,57 @@
void __init bl31_plat_arch_setup(void)
{
+ int rc __unused;
+ uintptr_t hw_config_base_align __unused;
+ size_t mapped_size_align __unused;
+
arm_bl31_plat_arch_setup();
/*
* For RESET_TO_BL31 systems, BL31 is the first bootloader to run.
* So there is no BL2 to load the HW_CONFIG dtb into memory before
- * control is passed to BL31.
+ * control is passed to BL31. The code below relies on dynamic mapping
+ * capability, which is not supported by xlat tables lib V1.
+ * TODO: remove the ARM_XLAT_TABLES_LIB_V1 check when its support
+ * gets deprecated.
*/
-#if !RESET_TO_BL31 && !BL2_AT_EL3
- /* HW_CONFIG was also loaded by BL2 */
- const struct dyn_cfg_dtb_info_t *hw_config_info;
-
- hw_config_info = FCONF_GET_PROPERTY(dyn_cfg, dtb, HW_CONFIG_ID);
+#if !RESET_TO_BL31 && !BL2_AT_EL3 && !ARM_XLAT_TABLES_LIB_V1
assert(hw_config_info != NULL);
+ assert(hw_config_info->config_addr != 0UL);
+ /* Page aligned address and size if necessary */
+ hw_config_base_align = page_align(hw_config_info->config_addr, DOWN);
+ mapped_size_align = page_align(hw_config_info->config_max_size, UP);
+
+ if ((hw_config_info->config_addr != hw_config_base_align) &&
+ (hw_config_info->config_max_size == mapped_size_align)) {
+ mapped_size_align += PAGE_SIZE;
+ }
+
+ /*
+ * map dynamically HW config region with its aligned base address and
+ * size
+ */
+ rc = mmap_add_dynamic_region((unsigned long long)hw_config_base_align,
+ hw_config_base_align,
+ mapped_size_align,
+ MT_RO_DATA);
+ if (rc != 0) {
+ ERROR("Error while mapping HW_CONFIG device tree (%d).\n", rc);
+ panic();
+ }
+
+ /* Populate HW_CONFIG device tree with the mapped address */
fconf_populate("HW_CONFIG", hw_config_info->config_addr);
-#endif
+
+ /* unmap the HW_CONFIG memory region */
+ rc = mmap_remove_dynamic_region(hw_config_base_align, mapped_size_align);
+ if (rc != 0) {
+ ERROR("Error while unmapping HW_CONFIG device tree (%d).\n",
+ rc);
+ panic();
+ }
+#endif /* !RESET_TO_BL31 && !BL2_AT_EL3 && !ARM_XLAT_TABLES_LIB_V1 */
}
unsigned int plat_get_syscnt_freq2(void)