diff options
| -rw-r--r-- | docs/threat_model/threat_model.rst | 22 | ||||
| -rw-r--r-- | services/spd/opteed/opteed_main.c | 11 |
2 files changed, 20 insertions, 13 deletions
diff --git a/docs/threat_model/threat_model.rst b/docs/threat_model/threat_model.rst index 0e967baf0..940cad54f 100644 --- a/docs/threat_model/threat_model.rst +++ b/docs/threat_model/threat_model.rst @@ -921,16 +921,16 @@ These are highlighted in the ``Mitigations implemented?`` box. +------------------------+-----------------------------------------------------+ | ID | 14 | +========================+=====================================================+ -| Threat | | **Security vulnerabilities in the Non-secure OS | -| | can lead to secure world compromise if the option | -| | OPTEE_ALLOW_SMC_LOAD is enabled.** | +| Threat | | **Attacker wants to execute an arbitrary or | +| | untrusted binary as the secure OS.** | | | | -| | | This option trusts the non-secure world up until | -| | the point it issues the SMC call to load the | -| | Secure BL32 payload. If a compromise occurs | -| | before the SMC call is invoked, then arbitrary | -| | code execution in S-EL1 can occur or arbitrary | -| | memory in EL3 can be overwritten. | +| | | When the option OPTEE_ALLOW_SMC_LOAD is enabled, | +| | this trusts the non-secure world up until the | +| | point it issues the SMC call to load the Secure | +| | BL32 payload. If a compromise occurs before the | +| | SMC call is invoked, then arbitrary code execution| +| | in S-EL1 can occur or arbitrary memory in EL3 can | +| | be overwritten. | +------------------------+-----------------------------------------------------+ | Diagram Elements | DF5 | +------------------------+-----------------------------------------------------+ @@ -948,9 +948,9 @@ These are highlighted in the ``Mitigations implemented?`` box. +------------------------+-----------------+-----------------+-----------------+ | Impact | Critical (5) | Critical (5) | Critical (5) | +------------------------+-----------------+-----------------+-----------------+ -| Likelihood | Low (2) | Low (2) | Low (2) | +| Likelihood | High (4) | High (4) | High (4) | +------------------------+-----------------+-----------------+-----------------+ -| Total Risk Rating | Medium (10) | Medium (10) | Medium (10) | +| Total Risk Rating | Critical (20) | Critical (20) | Critical (20) | +------------------------+-----------------+-----------------+-----------------+ | Mitigations | When enabling the option OPTEE_ALLOW_SMC_LOAD, | | | the non-secure OS must be considered a closed | diff --git a/services/spd/opteed/opteed_main.c b/services/spd/opteed/opteed_main.c index ff2aee0c5..ff09e7e0f 100644 --- a/services/spd/opteed/opteed_main.c +++ b/services/spd/opteed/opteed_main.c @@ -168,7 +168,8 @@ static int32_t opteed_setup(void) * used. It also assumes that a valid non-secure context has been * initialised by PSCI so it does not need to save and restore any * non-secure state. This function performs a synchronous entry into - * OPTEE. OPTEE passes control back to this routine through a SMC. + * OPTEE. OPTEE passes control back to this routine through a SMC. This returns + * a non-zero value on success and zero on failure. ******************************************************************************/ static int32_t opteed_init_with_entry_point(entry_point_info_t *optee_entry_point) @@ -232,6 +233,10 @@ static int32_t opteed_handle_smc_load(uint64_t data_size, uint32_t data_pa) mapped_data_va = mapped_data_pa; data_map_size = page_align(data_size + (mapped_data_pa - data_pa), UP); + /* + * We do not validate the passed in address because we are trusting the + * non-secure world at this point still. + */ rc = mmap_add_dynamic_region(mapped_data_pa, mapped_data_va, data_map_size, MT_MEMORY | MT_RO | MT_NS); if (rc != 0) { @@ -290,7 +295,9 @@ static int32_t opteed_handle_smc_load(uint64_t data_size, uint32_t data_pa) 0, 0, &opteed_sp_context[linear_id]); - rc = opteed_init_with_entry_point(&optee_ep_info); + if (opteed_init_with_entry_point(&optee_ep_info) == 0) { + rc = -EFAULT; + } /* Restore non-secure state */ cm_el1_sysregs_context_restore(NON_SECURE); |