diff mbox series

efi_loader: Extra checks while opening an OPTEE session

Message ID 20201223104319.57141-1-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series efi_loader: Extra checks while opening an OPTEE session | expand

Commit Message

Ilias Apalodimas Dec. 23, 2020, 10:43 a.m. UTC
When opening an OP-TEE session we need to check the internal return
value of OP-TEE call arguments as well the return code of the
function itself.
The code was also ignoring to close the OP-TEE session in case the
shared memory registration failed.

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 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.30.0.rc1

Comments

Heinrich Schuchardt Dec. 23, 2020, 10:56 a.m. UTC | #1
Am 23. Dezember 2020 11:43:19 MEZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>When opening an OP-TEE session we need to check the internal return

>value of OP-TEE call arguments as well the return code of the

>function itself.

>The code was also ignoring to close the OP-TEE session in case the

>shared memory registration failed.

>

>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 | 10 ++++++----

> 1 file changed, 6 insertions(+), 4 deletions(-)

>

>diff --git a/lib/efi_loader/efi_variable_tee.c

>b/lib/efi_loader/efi_variable_tee.c

>index be6f3dfad469..717d0b45e7cd 100644

>--- a/lib/efi_loader/efi_variable_tee.c

>+++ b/lib/efi_loader/efi_variable_tee.c

>@@ -45,10 +45,11 @@ static int get_connection(struct mm_connection

>*conn)

> 	memset(&arg, 0, sizeof(arg));

> 	tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);

> 	rc = tee_open_session(tee, &arg, 0, NULL);

>-	if (!rc) {

>-		conn->tee = tee;

>-		conn->session = arg.session;

>-	}

>+	if (rc || arg.ret != TEE_SUCCESS)

>+		return rc == 0 ? -EIO : rc;


 Could you simplify this, please

if (rc)
     return rc;
if (arg.ret != TEE_SUCCESS)
    return -EIO;

>+

>+	conn->tee = tee;

>+	conn->session = arg.session;

> 

> 	return rc;


return 0;

Best regards

Heinrich

> }

>@@ -88,6 +89,7 @@ static efi_status_t optee_mm_communicate(void

>*comm_buf, ulong dsize)

> 

> 	if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {

> 		log_err("Unable to register shared memory\n");

>+		tee_close_session(conn.tee, conn.session);

> 		return EFI_UNSUPPORTED;

> 	}

>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index be6f3dfad469..717d0b45e7cd 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -45,10 +45,11 @@  static int get_connection(struct mm_connection *conn)
 	memset(&arg, 0, sizeof(arg));
 	tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
 	rc = tee_open_session(tee, &arg, 0, NULL);
-	if (!rc) {
-		conn->tee = tee;
-		conn->session = arg.session;
-	}
+	if (rc || arg.ret != TEE_SUCCESS)
+		return rc == 0 ? -EIO : rc;
+
+	conn->tee = tee;
+	conn->session = arg.session;
 
 	return rc;
 }
@@ -88,6 +89,7 @@  static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
 
 	if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, &shm)) {
 		log_err("Unable to register shared memory\n");
+		tee_close_session(conn.tee, conn.session);
 		return EFI_UNSUPPORTED;
 	}