Djam/arm-trusted-firmware
1
0
Fork 0
mirror of https://github.com/ARM-software/arm-trusted-firmware.git synced 2025-04-16 01:24:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(optee): address late comments and fix bad rc

Browse source 
There were some late comments to the prior change (18635) which are
address in this commit. There was also an invalid return value check
which was changed and the wrong result was being returned via the SMC
call for loading OP-TEE which is now fixed.

Signed-off-by: Jeffrey Kardatzke <jkardatzke@google.com>
Change-Id: I883ddf966662549a3ef9c801a2d4f47709422332
This commit is contained in:
Jeffrey Kardatzke 2023-02-09 11:03:17 -08:00
parent 6264643a07
commit 8d7c80fa4c
2 changed files with 20 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
docs/threat_model/threat_model.rst
View file

@ -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 |

11
services/spd/opteed/opteed_main.c
View file

@ -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);