| Message ID | 20200722073222.510135-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Accepted |
| Commit | 9b87d4429c145ebb66895c7e053e8d53192180e2 |
| Headers | show |
| Series | efi_loader: Check for the native OP-TEE result on mm_communicate calls | expand |
On 22.07.20 09:32, Ilias Apalodimas wrote: > Currently we only check for the return value of tee_invoke_func(). > Although OP-TEE and StMM will correctly set param[1].u.value.a and we'll > eventually return an error, the correct thing to do is check for the > OP_TEE return code as well. > So let's check for that and move tee_shm_free() and tee_close_session() > before exiting with an error to make sure we always clear the registered > memory. > > Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE") > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>> --- > lib/efi_loader/efi_variable_tee.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c > index c0423489388a..346f4c786f28 100644 > --- a/lib/efi_loader/efi_variable_tee.c > +++ b/lib/efi_loader/efi_variable_tee.c > @@ -100,10 +100,10 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize) > param[1].attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT; > > rc = tee_invoke_func(conn.tee, &arg, 2, param); > - if (rc) > - return EFI_INVALID_PARAMETER; > tee_shm_free(shm); > tee_close_session(conn.tee, conn.session); > + if (rc || arg.ret != TEE_SUCCESS) > + return EFI_INVALID_PARAMETER; I will use EFI_DEVICE_ERROR here when merging as this indicates an internal problem in contrast to a problem caused by the caller of the UEFI API. Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > switch (param[1].u.value.a) { > case ARM_SMC_MM_RET_SUCCESS: >
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index c0423489388a..346f4c786f28 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -100,10 +100,10 @@ static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize) param[1].attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT; rc = tee_invoke_func(conn.tee, &arg, 2, param); - if (rc) - return EFI_INVALID_PARAMETER; tee_shm_free(shm); tee_close_session(conn.tee, conn.session); + if (rc || arg.ret != TEE_SUCCESS) + return EFI_INVALID_PARAMETER; switch (param[1].u.value.a) { case ARM_SMC_MM_RET_SUCCESS:
Currently we only check for the return value of tee_invoke_func(). Although OP-TEE and StMM will correctly set param[1].u.value.a and we'll eventually return an error, the correct thing to do is check for the OP_TEE return code as well. So let's check for that and move tee_shm_free() and tee_close_session() before exiting with an error to make sure we always clear the registered memory. Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE") Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- lib/efi_loader/efi_variable_tee.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.28.0.rc1